-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
flaky: parallel/test-timers-immediate-queue #24497
Comments
/CC @apapirovski might this be related to #22842? |
Attempting to replicate with a stress test: https://ci.nodejs.org/job/node-stress-single-test/2087/ Worker: test-azure_msft-win2016-x64-6 Non-default parameters: RUN_TESTS: -J --repeat 10 parallel/test-timers-immediate-queue |
@refack so this is because of timer precision on Windows. I can fix later today. Or someone else could... just needs the timeout and while loop to be bigger than like 16ms or whatever the platform granularity is (can't recall the exact number). |
FTR stress failed with 49% - Seems like it's fixable via Anyway, I'm trying to create a test that 100% reproduces so we can add it to |
@refack Meaning that it can be fixed in libuv but currently is not? |
Yes, exactly. |
For example the above mentioned |
The interwebs say > 15ms, but I'm not sure I grok the test, and know how to change it without eliminating it's essence. BTW: It doesn't repro on my computer (Windows 10 1809 build 18282) |
The test featured a timer for the purpose of measuring event loop progess. Replacing it with a deterministic feature, such as a `MessagePort`, removes the dependency on OS-specific timing behaviour and thus makes the test no longer flaky. Fixes: nodejs#24497
https://ci.nodejs.org/job/node-test-commit-arm/nodes=centos7-arm64-gcc8/36235/console
|
https://ci.nodejs.org/job/node-test-commit-arm/36447/nodes=centos7-arm64-gcc8/console
|
@nodejs/timers @nodejs/libuv |
I'd say revert recent timers commits but everything looks pretty mundane |
Replace a flaky immediates event loop test that hasn't truly made sense for a while, since it has gone through so many different iterations, with a new test that is verifying the same behavior but without so much potential for flakiness. Fixes: nodejs#24497
Closing per #48575 |
master
timers
Test: test file - parallel/test-timers-immediate-queue
Job: https://ci.nodejs.org/job/node-test-binary-windows/21717/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/
Worker: https://ci.nodejs.org/computer/test-azure_msft-win2016-x64-6/
Output:
The text was updated successfully, but these errors were encountered: