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

gc tests don't run properly #5442

Closed
mhdawson opened this issue Feb 26, 2016 · 2 comments
Closed

gc tests don't run properly #5442

mhdawson opened this issue Feb 26, 2016 · 2 comments
Labels
test Issues and PRs related to the tests.

Comments

@mhdawson
Copy link
Member

This commit(#5376) seems to have broken the gc tests as they seem to run missing --expose-gc

to run

out/Release/node deps/npm/node_modules/node-gyp/bin/node-gyp rebuild --directory="$WORKSPACE/node/test/gc/node_modules/weak" --nodedir="$WORKSPACE/node"
tools/test.py --mode=release --progress=tap gc | tee gc.tests.tap

If I revert that change(with git checkout 1170b26 tools/test.py) the tests pass, otherwise I get errors like:

not ok 5 test-net-timeout.js
# /home/mhdawson/pulls/land/fips2/node/test/gc/test-net-timeout.js:46
#       gc();
#       ^
#
# ReferenceError: gc is not defined
#     at Socket. (/home/mhdawson/pulls/land/fips2/node/test/gc/test-net-timeout.js:46:7)
#     at Socket.g (events.js:277:16)
#     at emitNone (events.js:81:13)
#     at Socket.emit (events.js:180:7)
#     at Socket._onTimeout (net.js:324:8)
#     at _runOnTimeout (timers.js:537:11)
#     at _makeTimerTimeout (timers.js:528:3)
#     at Timer.unrefTimeout (timers.js:597:5)
# We should do 500 requests
  ---
  duration_ms: 0.234

Don't believe the gc tests are run in the CI and that's why we did not see in the CI runs. I noticed they started failing in our internal jobs and that's how I noticed it.

@stefanmb can you please take a look

@mhdawson mhdawson changed the title gc tests don' gc tests don't run properly Feb 26, 2016
@mhdawson mhdawson added the test Issues and PRs related to the tests. label Feb 26, 2016
@stefanmb
Copy link
Contributor

@mhdawson Thanks for reporting this, it's fixed in #5446.

@mhdawson
Copy link
Member Author

Just to confirm, launched our internal tests that had caught this and they are now running ok.

rvagg pushed a commit that referenced this issue Feb 27, 2016
Append --node-args to existing list, don't overwrite arg list.

Fixes: #5442
PR-URL: #5446
Reviewed-By: Ben Noorhduis <[email protected]>
Reviewed-by: Michael Dawson <[email protected]>
rvagg pushed a commit that referenced this issue Feb 27, 2016
Append --node-args to existing list, don't overwrite arg list.

Fixes: #5442
PR-URL: #5446
Reviewed-By: Ben Noorhduis <[email protected]>
Reviewed-by: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

2 participants