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: fix flaky test-worker-debug #28307

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Address a race condition in the test; the Worker’s exit events
may have been not recorded because the Worker exited before
the listeners were attached.

Fix the by attaching the event listeners before telling the Worker
to exit.

Fixes: #28299
Fixes: #28106

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Address a race condition in the test; the Worker’s exit events
may have been not recorded because the Worker exited before
the listeners were attached.

Fix the by attaching the event listeners before telling the Worker
to exit.

Fixes: nodejs#28299
Fixes: nodejs#28106
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 19, 2019
@addaleax addaleax added flaky-test Issues and PRs related to the tests with unstable failures on the CI. fast-track PRs that do not need to wait for 48 hours to land. worker Issues and PRs related to Worker support. inspector Issues and PRs related to the V8 inspector protocol labels Jun 19, 2019
@nodejs-github-bot
Copy link
Collaborator

@addaleax
Copy link
Member Author

addaleax commented Jun 19, 2019

Stress test: https://ci.nodejs.org/job/node-stress-single-test/2232/
Stress test: https://ci.nodejs.org/job/node-stress-single-test/2233/

Please 👍 this comment to approve fast-tracking.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 19, 2019
@danbev
Copy link
Contributor

danbev commented Jun 20, 2019

Landed in 2dec0f9.

@danbev danbev closed this Jun 20, 2019
danbev pushed a commit that referenced this pull request Jun 20, 2019
Address a race condition in the test; the Worker’s exit events
may have been not recorded because the Worker exited before
the listeners were attached.

Fix the by attaching the event listeners before telling the Worker
to exit.

PR-URL: #28307
Fixes: #28299
Fixes: #28106
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
@addaleax addaleax deleted the test-worker-debug-fix branch June 20, 2019 20:38
targos pushed a commit that referenced this pull request Jul 2, 2019
Address a race condition in the test; the Worker’s exit events
may have been not recorded because the Worker exited before
the listeners were attached.

Fix the by attaching the event listeners before telling the Worker
to exit.

PR-URL: #28307
Fixes: #28299
Fixes: #28106
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
@targos targos mentioned this pull request Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. flaky-test Issues and PRs related to the tests with unstable failures on the CI. inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. worker Issues and PRs related to Worker support.
Projects
None yet
6 participants