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-vm-timeout-escape-nexttick #24251

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 8, 2018

Increase the VM timeout. If it is too small, the VM does not exit before
the code has a chance to create the problematic condition that causes
the timeout to be ignored.

Fixes: #24120

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Increase the VM timeout. If it is too small, the VM does not exit before
the code has a chance to create the problematic condition that causes
the timeout to be ignored.

Fixes: nodejs#24120
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 8, 2018
@Trott
Copy link
Member Author

Trott commented Nov 8, 2018

@nodejs/build Is there an easy way to run a stress test on a Pi2 device?

@refack
Copy link
Contributor

refack commented Nov 8, 2018

Baseline stress master - https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/93/
stress this PR - https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/94/

Is there an easy way to run a stress test on a Pi2 device?

Not ATM, well not simple. You could patch Makefile to do what you want (ATT, the test-ci-js target is the one used by the PIs)

node/Makefile

Lines 444 to 447 in 1f6c4ba

test-ci-js: | clear-stalled
$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
--mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \
$(TEST_CI_ARGS) $(CI_JS_SUITES) --skip-tests=sequential/test-benchmark-napi

I'll see if I can make it simpler.

@Trott
Copy link
Member Author

Trott commented Nov 8, 2018

Baseline stress master - https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/93/
stress this PR - https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/94/

@refack Compilation or something failed for both...

@refack
Copy link
Contributor

refack commented Nov 8, 2018

@Trott
Copy link
Member Author

Trott commented Nov 8, 2018

@refack Getting closer. The second one seemed to fail on git somewhere/somehow?

@refack
Copy link
Contributor

refack commented Nov 8, 2018

PR: https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/98/

But we don't have a repro with master...

More intense stress on master - https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/99/parameters/

@Trott
Copy link
Member Author

Trott commented Nov 8, 2018

FWIW, we were seeing it on the Pi2 devices, which might have a different (lower) common.platform() multiplier, so maybe testing on Pi1 is not the way to go? But I don't see an easy way to test on Pi2 or Pi3, unfortunately....

@refack
Copy link
Contributor

refack commented Nov 9, 2018

FWIW, we were seeing it on the Pi2 devices

Trying to get the job to run on all 3 PI versions:
https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/100/

@refack
Copy link
Contributor

refack commented Nov 9, 2018

Job 100 fails indeed on PI2
Running this PR: https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/101/

@Trott
Copy link
Member Author

Trott commented Nov 9, 2018

Results are looking promising so far. Thanks for handling the CI finessing, @refack!

@Trott
Copy link
Member Author

Trott commented Nov 9, 2018

@Trott
Copy link
Member Author

Trott commented Nov 9, 2018

Resume Build: https://ci.nodejs.org/job/node-test-pull-request/18445/

(Failure on FreeBSD is a known and unrelated flaky.)

@Trott
Copy link
Member Author

Trott commented Nov 9, 2018

This changes a 24% failure on Pi2 to a 0% failure. Fixing CI so proposing to fast-track. Collaborators, please 👍 here to approve.

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 9, 2018
@Trott Trott added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. flaky-test Issues and PRs related to the tests with unstable failures on the CI. labels Nov 9, 2018
Trott added a commit to Trott/io.js that referenced this pull request Nov 9, 2018
Increase the VM timeout. If it is too small, the VM does not exit before
the code has a chance to create the problematic condition that causes
the timeout to be ignored.

Fixes: nodejs#24120

PR-URL: nodejs#24251
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@Trott
Copy link
Member Author

Trott commented Nov 9, 2018

Landed in a4a2e9f

@Trott Trott closed this Nov 9, 2018
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Increase the VM timeout. If it is too small, the VM does not exit before
the code has a chance to create the problematic condition that causes
the timeout to be ignored.

Fixes: #24120

PR-URL: #24251
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Increase the VM timeout. If it is too small, the VM does not exit before
the code has a chance to create the problematic condition that causes
the timeout to be ignored.

Fixes: nodejs#24120

PR-URL: nodejs#24251
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
codebytere pushed a commit that referenced this pull request Jan 12, 2019
Increase the VM timeout. If it is too small, the VM does not exit before
the code has a chance to create the problematic condition that causes
the timeout to be ignored.

Fixes: #24120

PR-URL: #24251
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@codebytere codebytere mentioned this pull request Jan 15, 2019
@Trott Trott deleted the fix-24120-moar branch January 13, 2022 22:50
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. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

investigate flaky known_issues/test-vm-timeout-escape-nexttick
5 participants