Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.4k
test: improve debug-break-on-uncaught reliability#6793
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Trott commented May 16, 2016
jasnell commented May 16, 2016
LGTM if CI is green. |
claudiorodriguez commented May 18, 2016
Test is failing on several platforms with this: |
Trott commented May 20, 2016
CI again to see if platforms are failing consistently or not: https://ci.nodejs.org/job/node-test-pull-request/2718/ |
Trott commented May 21, 2016
The use of https://ci.nodejs.org/job/node-test-commit-linux/3463/nodes=ubuntu1404-64/console |
Trott commented May 21, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
The latest change makes stuff better but still lots of fail. I'm going to move it back to |
Trott commented May 21, 2016
Squashed and force pushed. PTAL. |
Trott commented May 21, 2016
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assumes that lines correspond with data events. I'd concatenate the output and scan that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnoordhuis True. Fixed.
bnoordhuis commented May 22, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
LGTM with a suggestion. |
claudiorodriguez commented May 22, 2016
LGTM |
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it.
Trott commented May 24, 2016
implemented @bnoordhuis suggestion...onward to CI: https://ci.nodejs.org/job/node-test-pull-request/2755/ |
bnoordhuis commented May 24, 2016
Still LGTM. Another /cc @nodejs/build - known issue? Happened several times yesterday, too. |
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: nodejs#6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Trott commented May 24, 2016
Landed in ff00a48 |
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: nodejs#6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: #6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
MylesBorins commented Jul 11, 2016
@Trott lts? |
Trott commented Jul 11, 2016
@thealphanerd Yes, if it lands cleanly. |
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: #6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: #6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: #6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: #6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Running the test through CI reveals unreliability due to a race condition. These changes mitigate the race condition, but do not eliminate it. PR-URL: #6793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]>
Checklist
Affected core subsystem(s)
test debugger
Description of change
Move test-debug-break-on-uncaught from
debuggerdirectory tosequentialso that it gets exercised bymake testand via thecontinuous integration server for the project.
Removed unnecessary port number modification that is probable source of
unreliability on CI.