Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails on Windows (Tests and all) #41

Open
justrhysism opened this issue Sep 1, 2014 · 14 comments
Open

Fails on Windows (Tests and all) #41

justrhysism opened this issue Sep 1, 2014 · 14 comments

Comments

@justrhysism
Copy link

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
Copy link
Contributor

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

@justrhysism
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
#41 (comment)
.

@jgoz
Copy link
Contributor

jgoz commented Oct 16, 2014

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
Copy link
Contributor

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
Copy link
Contributor

jgoz commented Oct 17, 2014

@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
Copy link
Contributor

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

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

@jgoz
Copy link
Contributor

jgoz commented Oct 17, 2014

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

@terinjokes
Copy link
Contributor

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
Copy link

cuth commented Dec 18, 2014

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

@justrhysism
Copy link
Author

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants