| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I don't understand this well enough to review but just wanted to say thanks for all the work on these tasks 🙇 |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
|
Feel free to skip the tests again in the mean time |
Sorry, something went wrong.
|
The test that fails (negative-settimeout.any.js) is short and straightforward. It checks that a timer with a negative timeout value runs before a timer with a timeout value of 10. setup({ single_test: true });
setTimeout(done, -100);
setTimeout(assert_unreached, 10);While the ordering may be guaranteed in browsers, I don't think it is in Node.js due to the linked list stuff that underpins timers. But maybe I'm confused. /ping @nodejs/timers for confirmation. If the ordering in the above case is not in fact guaranteed in Node.js, then skipping the test is certainly the right thing to do. |
Sorry, something went wrong.
@Trott I think that there's something wrong with done(). Rewriting the test to the following is not flaky on my machine (while the original is): setup({ single_test: true });
setTimeout(() => { clearTimeout(y); done(); }, -100);
var y = setTimeout(assert_unreached, 10);I assume that done doesn't clear the timeout, and there's an edge-case where if there's a very small difference (or maybe if they're on the same tick) between the actual running time of done and assert_unreached it fails the test anyway. I think that the above fix is a slightly more correct way to test that the first runs before the second anyway. |
Sorry, something went wrong.
|
OK. The Parallel to Simple change fixed most of them. The one it didn't fix is now marked flaky in the status file. This is ready for review. |
Sorry, something went wrong.
Sorry, something went wrong.
|
Stress test against this PR: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/224/ ✅ |
Sorry, something went wrong.
|
Stress test and CI were both green on the first run. It would be great to get this frequent-CI-failure addressed. @nodejs/testing |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks!
Sorry, something went wrong.
Moving test harness from Parallel to Simple resolves most of the failures. negative-settimeout.any.js still needs to be marked flaky. Refs: nodejs#37672 PR-URL: nodejs#37691 Fixes: nodejs#37672 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
| Back | FazBrowse Home | New Git URL |
Change the wpt tests to not run in parallel in our Python test runner.
On my local machine, this changes the time needed to run the suite from
about 2 seconds to about 5 seconds, but it makes the test suite much
more reliable.
Fixes: #37672