factor-bundle icon indicating copy to clipboard operation
factor-bundle copied to clipboard

Fails on Windows (Tests and all)

Open justrhysism opened this issue 11 years ago • 14 comments

Sadly for me, working in Windows isn't something I have the luxury of choosing.

I do have access to OS X also, but it's the rest of the team (and build server) is on Windows. I was able to clone the repository on OS X, "npm install -D" and run the tests successfully.

When I did the same on Windows, all tests fail - and the module itself doesn't work.

I'm unsure if the tests are failing because of the plugin itself, or because Tape doesn't support Windows - but I've been unsuccessful in getting the plugin to work on Windows :(

justrhysism avatar Sep 01 '14 07:09 justrhysism

Just so I can reproduce exactly, how are you running the module, and with what versions of Windows, node and npm?

terinjokes avatar Sep 01 '14 15:09 terinjokes

For my own sanity check, I cloned the source from here (GitHub), installed all devDependencies and ran the tests with npm test. I followed the same procedure on OS X and all tests passed.

Environment

  • Windows 8.1 - [Version 6.3.9600]
  • Node v0.10.31
  • NPM 1.4.23

justrhysism avatar Sep 02 '14 03:09 justrhysism

I really don't understand how ALL the tests can fail on Windows. This really doesn't make much sense.

Any indication on when this might be fixed or even your priority on it? It's a show stopper and if it's not soon we're going to have to drop Browserify + Factor Bundle for something like WebPack (which I don't really want to do, but am without choice sadly).

justrhysism avatar Oct 13 '14 01:10 justrhysism

I'll spin up a Windows VM and try to figure this out. We aren't doing anything different than the Browserify repo for tests, and no one has any issues with tests on Windows.

terinjokes avatar Oct 14 '14 23:10 terinjokes

I just tried to run Browserify's tests on Windows and a whole heap failed. It's probably environment related (damn Windows).

On 15 October 2014 10:04, Terin Stock [email protected] wrote:

I'll spin up a Windows VM and try to figure this out. We aren't doing anything different than the Browserify repo for tests, and no one has any issues with tests on Windows.

— Reply to this email directly or view it on GitHub https://github.com/substack/factor-bundle/issues/41#issuecomment-59135362 .

justrhysism avatar Oct 16 '14 05:10 justrhysism

FYI, we've been using factor-bundle without any platform issues for months. We have ~30 devs, all on Windows.

Tests do fail on Windows, but it's not related to tape. I think it's related to usage of spawn - the tests use spawn('browserify', args), but Windows needs something like spawn('node', ['browserify'].concat(args)).

I also see failing tests in common-deps.js - not sure if that's Windows-specific.

jgoz avatar Oct 16 '14 23:10 jgoz

I built a windows VM to debug this today, but didn't find the common-deps issue before it was quitting time. Will look again in the morning.

@jgoz Yeah, that's what it looks like.

terinjokes avatar Oct 17 '14 01:10 terinjokes

@terinjokes common-deps fails due to path issues. When path.resolve is called on an absolute path on windows, you get something rooted at C:\. e.g., path.resolve('/b.js') -> 'C:\b.js'. Therefore, lookups in this._files will fail (here). Providing an rmap fixes this for me.

diff --git a/test/common-deps.js b/test/common-deps.js
index 9d1a8ba..5966ac8 100644
--- a/test/common-deps.js
+++ b/test/common-deps.js
@@ -1,3 +1,4 @@
+var path = require('path');
 var test = require('tape');
 var through = require('through');
 var factor = require('../');
@@ -10,13 +11,17 @@ var ROWS = {

 var expected = {};
 expected.common = [ ROWS.A, ROWS.C ].sort(cmp);
-expected['/b.js'] = [ ROWS.B ].sort(cmp);
+expected[path.resolve('/b.js')] = [ ROWS.B ].sort(cmp);

 test('lift singly-shared dependencies', function (t) {
     t.plan(2);

     var files = [ '/b.js' ];
-    var fr = factor(files, { objectMode: true, raw: true });
+    var rmap = Object.keys(ROWS).reduce(function (acc, r) {
+        acc[ROWS[r].id] = path.resolve(ROWS[r].id);
+        return acc;
+    }, {});
+    var fr = factor(files, { objectMode: true, raw: true, rmap: rmap });
     fr.on('stream', function (bundle) {
         bundle.pipe(rowsOf(function (rows) {
             console.log('----- ' + bundle.file + ' ROWS -----');

jgoz avatar Oct 17 '14 17:10 jgoz

Ah yes. This is was what I was working on before I got sucked into the rabbit hole of mixins to support multiple versions of Flexbox…

I think I'll work on this. :wink:

Edit: I edited the comment above link to a specific commit, not master.

terinjokes avatar Oct 17 '14 19:10 terinjokes

Yeah, I'd probably make that same choice :)

jgoz avatar Oct 17 '14 19:10 jgoz

I've resolved two issues on my "windowz" branch:

  • Spawn browserify from unit tests in a Windows compatible way. This resolves the "plugin.js" and "plugin-dedupe.js" tests.
  • Since we utilize the 'file' property of a module-deps row, add a handler to the 'deps' pipeline event and filter for entry modules. Because we may now get them in the wrong order, I filter based on the "order" property. I'm not entirely sure why this was working on unix systems.

terinjokes avatar Oct 18 '14 03:10 terinjokes

@justrhysism were you able to get this working on Windows?

cuth avatar Dec 18 '14 21:12 cuth

Ran it on Win 8.1 VM and all tests are failing still - but I'm not convinced I'm on the right branch. Will check properly tomorrow on my Windows dev machine.

justrhysism avatar Dec 21 '14 07:12 justrhysism

Just tested this again on my dev machine at work from terinjokes\factor-bundle on the windowz branch. Output as follows:

D:\Projects\SanityCheck\factor-bundle (windowz)
λ npm test

> [email protected] test D:\Projects\SanityCheck\factor-bundle
> tape test/*.js

TAP version 13
# lift singly-shared dependencies
----- COMMON ROWS -----
[ { id: '/b.js', deps: { '/c.js': '/c.js' } },
  { id: '/a.js', deps: { '/c.js': '/c.js' } },
  { id: '/c.js', deps: {} } ]
not ok 1 should be equivalent
  ---
    operator: deepEqual
    expected:
      [ { deps: { '/c.js': '/c.js' }, id: '/a.js' }, { deps: {}, id: '/c.js' } ]
    actual:
      [ { deps: { '/c.js': '/c.js' }, id: '/a.js' }, { deps: { '/c.js': '/c.js' }, id: '/b.js' }, { deps: {}, id: '/c.js' } ]
  ...
not ok 2 plan != count
  ---
    operator: fail
    expected: 2
    actual:   1
  ...
not ok 3 test exited without ending
  ---
    operator: fail
  ...
not ok 4 test exited without ending
  ---
    operator: fail
  ...
not ok 5 test exited without ending
  ---
    operator: fail
  ...
not ok 6 test exited without ending
  ---
    operator: fail
  ...
not ok 7 test exited without ending
  ---
    operator: fail
  ...
not ok 8 test exited without ending
  ---
    operator: fail
  ...
not ok 9 test exited without ending
  ---
    operator: fail
  ...
not ok 10 test exited without ending
  ---
    operator: fail
  ...
not ok 11 test exited without ending
  ---
    operator: fail
  ...
not ok 12 test exited without ending
  ---
    operator: fail
  ...
not ok 13 test exited without ending
  ---
    operator: fail
  ...
not ok 14 test exited without ending
  ---
    operator: fail
  ...

1..14
# tests 14
# pass  0
# fail  14

npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

justrhysism avatar Jan 14 '15 07:01 justrhysism