Fails on Windows (Tests and all)

justrhysism commented 10 years ago

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 :(

terinjokes commented 10 years ago

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

justrhysism commented 10 years ago

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 commented 9 years ago

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).

terinjokes commented 9 years ago

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.

justrhysism commented 9 years ago

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 notifications@github.com 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 .

jgoz commented 9 years ago

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.

terinjokes commented 9 years ago

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.

jgoz commented 9 years ago

@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 -----');

terinjokes commented 9 years ago

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.

jgoz commented 9 years ago

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

terinjokes commented 9 years ago

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.

cuth commented 9 years ago

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

justrhysism commented 9 years ago

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 commented 9 years ago

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

> factor-bundle@2.3.0 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

browserify / factor-bundle

Fails on Windows (Tests and all) #41

Environment