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

investigate flaky wpt/test-timers #37672

Closed
Trott opened this issue Mar 9, 2021 · 7 comments · Fixed by #37691
Closed

investigate flaky wpt/test-timers #37672

Trott opened this issue Mar 9, 2021 · 7 comments · Fixed by #37691
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Comments

@Trott
Copy link
Member

Trott commented Mar 9, 2021

  • Test: wpt/test-timers
  • Platform: freebsd11-x64
  • Console Output:
00:23:47 not ok 2832 wpt/test-timers
00:23:48   ---
00:23:48   duration_ms: 0.973
00:23:48   severity: fail
00:23:48   exitcode: 1
00:23:48   stack: |-
00:23:48     ---- type-long-settimeout.any.js ----
00:23:48     [PASS] Untitled
00:23:48     ---- negative-settimeout.any.js ----
00:23:48     [PASS] Untitled
00:23:48     [UNEXPECTED_FAILURE][UNCAUGHT] evaluation in WPTRunner.runJsTests()
00:23:48     assert_unreached: Reached unreachable code
00:23:48     {
00:23:48       message: 'assert_unreached: Reached unreachable code',
00:23:48       stack: '    at listOnTimeout (node:internal/timers:557:17)\n' +
00:23:48         '    at processTimers (node:internal/timers:500:7)'
00:23:48     }
00:23:48     Command: /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/out/Release/node  /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/wpt/test-timers.js type-long-setinterval.any.js
00:23:48     
00:23:48     ---- type-long-setinterval.any.js ----
00:23:48     [PASS] Untitled
00:23:48     ---- missing-timeout-setinterval.any.js ----
00:23:48     [PASS] Calling setInterval with no interval should be the same as if called with 0 interval
00:23:48     ---- missing-timeout-setinterval.any.js ----
00:23:48     [PASS] Calling setInterval with undefined interval should be the same as if called with 0 interval
00:23:48     ---- negative-setinterval.any.js ----
00:23:48     [PASS] Untitled
00:23:48     ---- cleartimeout-clearinterval.any.js ----
00:23:48     [PASS] Clear timeout with clearInterval
00:23:48     ---- cleartimeout-clearinterval.any.js ----
00:23:48     [PASS] Clear interval with clearTimeout
00:23:48     {
00:23:48       'type-long-setinterval.any.js': {
00:23:48         fail: { unexpected: [ 'assert_unreached: Reached unreachable code' ] }
00:23:48       }
00:23:48     }
00:23:48     Ran 6/6 tests, 0 skipped, 5 passed, 0 expected failures, 1 unexpected failures
00:23:48     (node:22536) TimeoutOverflowWarning: 4294967296 does not fit into a 32-bit signed integer.
00:23:48     Timeout duration was set to 1.
00:23:48     (Use `node --trace-warnings ...` to show where the warning was created)
00:23:48     (node:22536) TimeoutOverflowWarning: 4294967296 does not fit into a 32-bit signed integer.
00:23:48     Timeout duration was set to 1.
00:23:48     (Use `node --trace-warnings ...` to show where the warning was created)
00:23:48     /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/common/wpt.js:445
00:23:48             throw new Error(
00:23:48             ^
00:23:48     
00:23:48     Error: Found 1 unexpected failures. Consider updating test/wpt/status/html/webappapis/timers.json for these files:
00:23:48     type-long-setinterval.any.js
00:23:48         at process.<anonymous> (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/common/wpt.js:445:15)
00:23:48         at process.emit (node:events:390:22)
00:23:48   ...
@Trott Trott added flaky-test Issues and PRs related to the tests with unstable failures on the CI. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Mar 9, 2021
@Trott
Copy link
Member Author

Trott commented Mar 9, 2021

@targos @joyeecheung

@targos
Copy link
Member

targos commented Mar 9, 2021

I thought maybe it could be a race condition under load after #37626 and #37627, but I cannot reproduce it on my computer:

$ tools/test.py --repeat 1000 -j 96 wpt/test-url 
[01:52|% 100|+ 1000|-   0]: Done 

@targos
Copy link
Member

targos commented Mar 9, 2021

🤦🏻 I ran the wrong test...
I reproduce easily with test-timers

@targos
Copy link
Member

targos commented Mar 9, 2021

It's definitely a timing issue. We get the worker's error event before the message that indicates that the test passed.

@targos
Copy link
Member

targos commented Mar 9, 2021

I thought about calling receiveMessageOnPort in the error handler to synchronously get the pending message(s) but it doesn't seem possible to access the worker's port?

@targos
Copy link
Member

targos commented Mar 9, 2021

I think the main issue is that we have double parallelization with the WPT.

  1. The test categories are run in parallel:
    return testpy.ParallelTestConfiguration(context, root, 'wpt')
  2. Each test within a category is run in parallel with the others (all workers are running simultaneously)

@Trott
Copy link
Member Author

Trott commented Mar 10, 2021

Happened again today: https://ci.nodejs.org/job/node-test-commit-osx/39393/nodes=osx1014/console

Seems like a good solution would be to remove the parallelization in testcfg.py?

Trott added a commit to Trott/io.js that referenced this issue Mar 10, 2021
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: nodejs#37672
Trott added a commit to Trott/io.js that referenced this issue Mar 13, 2021
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
@Trott Trott closed this as completed in 66f0eb7 Mar 14, 2021
danielleadams pushed a commit that referenced this issue Mar 16, 2021
Moving test harness from Parallel to Simple resolves most of the
failures. negative-settimeout.any.js still needs to be marked flaky.

Refs: #37672

PR-URL: #37691
Fixes: #37672
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams pushed a commit that referenced this issue Mar 16, 2021
Moving test harness from Parallel to Simple resolves most of the
failures. negative-settimeout.any.js still needs to be marked flaky.

Refs: #37672

PR-URL: #37691
Fixes: #37672
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants