| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I ran that test as: while true; do ./node test/parallel/test-domain-top-level-error-handler-throw.js; done and I haven't been able to see it fail. Did you manage to reproduce the flakiness of this test? A minor nit to the commit message: the first line of the description is longer than 72 characters, but that can be changed when landing it, so no worries. Other than that, LGTM and we don't necessarily have to reproduce the flakiness to land this change, as it's definitely more correct to wait for 'close' to be emitted to check for stderr output. Added lts-watch-* labels as this test is present in all LTS branches with the same issue. |
Sorry, something went wrong.
Sorry, something went wrong.
Check the stderr output in the `close` event as it's not guaranteed to be fully available when the `exit` event is fired.
|
@misterdjules I could reproduce in OS X the issue by running a bunch of the tests in parallel. First I made multiple copies of the test: test/parallel/test-domain-top-level-error-handler-throw-1.js, etc. and then: while true; do /usr/bin/python tools/test.py --mode=release parallel/test-domain-top-level-error-handler-throw* -J; done I've already fixed the description. Thanks! |
Sorry, something went wrong.
|
@santigimeno I couldn't reproduce the problem on my OSX setup even when running multiple instances of test/parallel/test-domain-top-level-error-handler-throw.js in parallel. Nevertheless, the change in this PR is a good fix, and CI tests pass. Landing asap. Thank you! |
Sorry, something went wrong.
|
It appears that the code in this PR is breaking CI. While CI was run for this PR, it looks like it was not run on the final version of the commit. It was run on an initial version. Maybe something broke that would have been detected by a CI run with the commit rebased against master? Not sure what happened. Still trying to figure it out. The specific test broken is in the addons. Here's an example from https://ci.nodejs.org/job/node-test-commit-osx/1553/nodes=osx1010/console: not ok 918 hello.js # module.js:433 # return process.dlopen(module, path._makeLong(filename)); # ^ # # Error: Module did not self-register. # at Error (native) # at Object.Module._extensions..node (module.js:433:18) # at Module.load (module.js:354:32) # at Function.Module._load (module.js:311:12) # at Module.require (module.js:364:17) # at require (internal/module.js:12:17) # at Object.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/addons/doc-1/hello.js:2:15) # at Module._compile (module.js:408:26) # at Object.Module._extensions..js (module.js:415:10) # at Module.load (module.js:354:32) |
Sorry, something went wrong.
|
I'll have a PR in just a moment to revert this |
Sorry, something went wrong.
PR-URL: nodejs#4410 As @Trott points out in nodejs#4364 commit cf50305 is currently breaking CI. This commit reverts the changes.
|
I'm reopening this now that the new PR is in |
Sorry, something went wrong.
|
This does not appear to be breaking things |
Sorry, something went wrong.
|
Apologies for the bad diagnosis! |
Sorry, something went wrong.
Check the stderr output in the `close` event as it's not guaranteed to be fully available when the `exit` event is fired. PR: nodejs#4364 PR-URL: nodejs#4364 Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
Check the stderr output in the `close` event as it's not guaranteed to be fully available when the `exit` event is fired. PR: nodejs#4364 PR-URL: nodejs#4364 Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
| Back | FazBrowse Home | New Git URL |
Check the stderr output in the close event as it's not guaranteed to be fully
available when the exit event is fired.
It tries to fix #4206.