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

test/parallel/test-npm-install.js fails (Node 4.4.4) #6648

Closed
phw opened this issue May 9, 2016 · 12 comments
Closed

test/parallel/test-npm-install.js fails (Node 4.4.4) #6648

phw opened this issue May 9, 2016 · 12 comments
Assignees
Labels
npm Issues and PRs related to the npm client dependency or the npm registry. test Issues and PRs related to the tests.

Comments

@phw
Copy link

phw commented May 9, 2016

  • Version: 4.4.4
  • Platform: Linux
  • Subsystem: unknown

Running make testthe test test/parallel/test-npm-install.js fails.

=== release test-npm-install ===                                               
Path: parallel/test-npm-install
assert.js:90
  throw new assert.AssertionError({
  ^
AssertionError: npm install should run without an error
    at ChildProcess.handleExit (/home/phw/devel/nodejs4/src/node-v4.4.4/test/parallel/test-npm-install.js:40:10)
    at ChildProcess.<anonymous> (/home/phw/devel/nodejs4/src/node-v4.4.4/test/common.js:385:15)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:172:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)
Command: out/Release/node /home/phw/devel/nodejs4/src/node-v4.4.4/test/parallel/test-npm-install.js

Here is the npm-debug.log generated.

I see multiple issues with this test:

  1. Obviously npm is expecting a package.json in the build directory, which isn't there.
  2. This npm call uses the previously installed node 4.4.3 for testing and not the newly compiled version in my source tree.
  3. This test even runs if I configured the build with the --without-npm flag (not sure whether this is indeed an issue or not).
@addaleax addaleax added test Issues and PRs related to the tests. npm Issues and PRs related to the npm client dependency or the npm registry. labels May 9, 2016
@scorpp
Copy link

scorpp commented May 9, 2016

Similar issue here with fresh ArchLinux. More over I tried to downgrade to 4.4.3 (which i had installed and working fine beforehand) and faced similar issue.

Even more to add - node binary segfaults on almost any operation. E.g. I cannot install npm.

I have another ArchLinux PC which doesn't have update installed for few days and 4.4.4 has been built on that PC without this issue. And it doesn't segfault. So I built it on 'older' PC and transferred and installed package on my laptop (which is suffering the topic issue). And 4.4.4 build on 'older' linux box works just fine on a 'newer' one.

Here is list of packages which are newer on suffering PC

[2016-05-09 13:49] [ALPM] upgraded linux-api-headers (4.4.1-1 -> 4.5.2-1)
[2016-05-09 13:49] [ALPM] upgraded glibc (2.23-1 -> 2.23-2)
[2016-05-09 13:49] [ALPM] upgraded lib32-glibc (2.23-1 -> 2.23-2)
[2016-05-09 13:49] [ALPM] upgraded lib32-gcc-libs (5.3.0-5 -> 6.1.1-1)
[2016-05-09 13:49] [ALPM] upgraded gcc-libs-multilib (5.3.0-5 -> 6.1.1-1)
[2016-05-09 13:49] [ALPM] upgraded gmp (6.1.0-3 -> 6.1.0-4)
[2016-05-09 13:49] [ALPM] upgraded perl (5.22.1-2 -> 5.22.2-1)
[2016-05-09 13:49] [ALPM] upgraded mpfr (3.1.4-1 -> 3.1.4.p1-1)
[2016-05-09 13:49] [ALPM] upgraded linux (4.5.1-1 -> 4.5.2-1)
[2016-05-09 13:49] [ALPM] upgraded acpi_call (1.1.0-44 -> 1.1.0-45)
[2016-05-09 13:49] [ALPM] upgraded binutils (2.26-3 -> 2.26-4)
[2016-05-09 13:49] [ALPM] upgraded libmpc (1.0.3-1 -> 1.0.3-2)
[2016-05-09 13:49] [ALPM] upgraded gcc-multilib (5.3.0-5 -> 6.1.1-1)
[2016-05-09 13:49] [ALPM] upgraded glib-networking (2.48.0-1 -> 2.48.1-1)
[2016-05-09 13:49] [ALPM] upgraded libassuan (2.4.2-1 -> 2.4.2-2)
[2016-05-09 13:49] [ALPM] upgraded pinentry (0.9.7-1 -> 0.9.7-2)
[2016-05-09 13:49] [ALPM] upgraded gnupg (2.1.11-1 -> 2.1.12-1)
[2016-05-09 13:49] [ALPM] upgraded harfbuzz (1.2.6-1 -> 1.2.7-1)
[2016-05-09 13:49] [ALPM] upgraded harfbuzz-icu (1.2.6-1 -> 1.2.7-1)
[2016-05-09 13:49] [ALPM] upgraded lib32-harfbuzz (1.2.6-1 -> 1.2.7-1)
[2016-05-09 13:49] [ALPM] upgraded lib32-libgpg-error (1.21-1 -> 1.22-1)
[2016-05-09 13:49] [ALPM] upgraded libbluray (0.9.2-1 -> 0.9.2-2)
[2016-05-09 13:49] [ALPM] upgraded libtool (2.4.6-4 -> 2.4.6-5)
[2016-05-09 13:49] [ALPM] upgraded linux-headers (4.5.1-1 -> 4.5.2-1)
[2016-05-09 13:49] [ALPM] upgraded make (4.1-3 -> 4.1-4)
[2016-05-09 13:49] [ALPM] upgraded pacman (5.0.1-2 -> 5.0.1-3)
[2016-05-09 13:49] [ALPM] upgraded qt5-base (5.6.0-4 -> 5.6.0-5)
[2016-05-09 13:49] [ALPM] upgraded skype (4.3.0.37-6 -> 4.3.0.37-7)

@MylesBorins
Copy link
Contributor

This is an interesting edge case... it looks like two things are happening that should

  1. npm is using the system node
  2. the cwd that is being set in spawn is not being resepcted

I'm going to dig into this and see what is going on.

@MylesBorins MylesBorins self-assigned this May 9, 2016
@MylesBorins
Copy link
Contributor

MylesBorins commented May 9, 2016

OK, so here is where I am at, and perhaps you can tell me a bit more about your systems because this is a bit odd.

  1. Obviously npm is expecting a package.json in the build directory, which isn't there.

This is being created and written into the common.tmpdir here. The cwd of the spawned process is set to common.tmpdir, so it is very odd that is not being found

  1. This npm call uses the previously installed node 4.4.3 for testing and not the newly compiled version in my source tree.

I have a PR submitted to make sure PATH is explicitly including the build directory which should fix this

  1. This test even runs if I configured the build with the --without-npm flag (not sure whether this is indeed an issue or not).

Is that flag related to make install? AFAIK we don't touch or change anything in the npm directory, this test should be completely self contained to the repo, and that flag should have no affect on it's ability to pass

@phw
Copy link
Author

phw commented May 10, 2016

Thanks for looking into this. I have as a quick test now tried building both 4.4.3 and 4.4.4 again, both fail with the same error while previously make test did run without any error when I installed 4.4.3 some time ago. Also it is not only the npm install test but also a few other tests failing. So I think there is something wrong with my system (Arch Linux also, just like @scorpp ), I will investigate what changed since then and report back.

@phw
Copy link
Author

phw commented May 10, 2016

I am clueless. Both version 4.4.3 and 4.4.4 fail on a couple of tests. The resulting binary is very unstable and core dumps in many cases. The last time I compiled and installed 4.4.3 was May 3, and that build went fine. But I don't see any critical update since then, and I even reverted some updates to try whether they had any effect. Just for having it here below is the output of make test. But ultimately I think this is probably not really a node issue but something broken on the Arch side.

~/devel/nodejs/node-v4.4.4 $ make test
make -C out BUILDTYPE=Release V=1
make[1]: Verzeichnis „/home/phw/devel/nodejs/node-v4.4.4/out“ wird betreten
make[1]: Für das Ziel „all“ ist nichts zu tun.
make[1]: Verzeichnis „/home/phw/devel/nodejs/node-v4.4.4/out“ wird verlassen
ln -fs out/Release/node node
Running main() from gtest_main.cc
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from UtilTest
[ RUN      ] UtilTest.ListHead
[       OK ] UtilTest.ListHead (0 ms)
[----------] 1 test from UtilTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 1 test.
/usr/bin/python2 tools/test.py --mode=release message parallel sequential -J
=== release test-timers-args ===                                               
Path: parallel/test-timers-args
Command: out/Release/node /home/phw/devel/nodejs/node-v4.4.4/test/parallel/test-timers-args.js
--- CRASHED ---
=== release test-npm-install ===                                 
Path: parallel/test-npm-install
assert.js:90
  throw new assert.AssertionError({
  ^
AssertionError: npm install should run without an error
    at ChildProcess.handleExit (/home/phw/devel/nodejs/node-v4.4.4/test/parallel/test-npm-install.js:40:10)
    at ChildProcess.<anonymous> (/home/phw/devel/nodejs/node-v4.4.4/test/common.js:385:15)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:172:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)
Command: out/Release/node /home/phw/devel/nodejs/node-v4.4.4/test/parallel/test-npm-install.js
=== release test-tick-processor ===                                            
Path: parallel/test-tick-processor
assert.js:90
  throw new assert.AssertionError({
  ^
AssertionError: false == true
    at runTest (/home/phw/devel/nodejs/node-v4.4.4/test/parallel/test-tick-processor.js:58:3)
    at Object.<anonymous> (/home/phw/devel/nodejs/node-v4.4.4/test/parallel/test-tick-processor.js:21:1)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3
Command: out/Release/node /home/phw/devel/nodejs/node-v4.4.4/test/parallel/test-tick-processor.js
[01:43|% 100|+ 1007|-   3]: Done                                               
Makefile:91: die Regel für Ziel „test“ scheiterte
make: *** [test] Fehler 1

@addaleax
Copy link
Member

test-tick-processor is a known flaky test (#4427), so you can probably ignore that.

@bnoordhuis
Copy link
Member

@pwh Are you compiling with g++ v6.x by any chance? If yes, see #6272 - it's going to be fixed in the next release but in the mean time try building with make -j8 CXX="g++ -fno-delete-null-pointer-checks".

@scorpp
Copy link

scorpp commented May 10, 2016

I'm compiling with g++ v6.1.1, can confirm that adding -fno-delete-null-pointer-checks fixes the issue. Both tests and segfaults. I'm on ArchLinux.
Many thanks @bnoordhuis for pointing the right thread

@MylesBorins
Copy link
Contributor

@bnoordhuis I dug through #6272
Looks like the v8 changes are being backported in #6669

@scorpp if all is well this should be fixed in v4.4.5

@pwh
Copy link

pwh commented May 10, 2016

Hey, I'm pwh!
I'm not phw, and I know not node
so I'm unsubscribing the thread.

(Programmers sweat the details!)

@MylesBorins
Copy link
Contributor

off by one

@phw
Copy link
Author

phw commented May 10, 2016

The real @phw here ;) I still don't quite understand what happened that test-npm-install failed the way it did, but compiling with -fno-delete-null-pointer-checks fixes the issue (and all other build errors). Thanks to all for helping out. Hope #6669 will go through for v4.4.5. At least this issue report led to the small patch by @thealphanerd to fix the node version used by the test :)

Otherwise I think this issue can be closed if nobody objects.

MylesBorins pushed a commit that referenced this issue May 11, 2016
Currently it is possible that the shelled out instance of npm will use
the system copy of node. This PR changes the test to shim the build
directory into the path. This will ensure that npm will use the correct
version of node.

fixes: #6648

PR-URL: #6658
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
evanlucas pushed a commit that referenced this issue May 17, 2016
Currently it is possible that the shelled out instance of npm will use
the system copy of node. This PR changes the test to shim the build
directory into the path. This will ensure that npm will use the correct
version of node.

fixes: #6648

PR-URL: #6658
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue May 18, 2016
Currently it is possible that the shelled out instance of npm will use
the system copy of node. This PR changes the test to shim the build
directory into the path. This will ensure that npm will use the correct
version of node.

fixes: #6648

PR-URL: #6658
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Issues and PRs related to the npm client dependency or the npm registry. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

6 participants