Repository navigation
parallel/test-assert-esm-cjs-message-verify is flaky #53962
Description
Activity
- addedflaky-testIssues and PRs involving tests that fail intermittently in CI.Issues and PRs involving tests that fail intermittently in CI.
on Jul 20, 2024 - addedppcIssues and PRs related to the Power architecture.Issues and PRs related to the Power architecture.s390xIssues and PRs related to the s390x architecture.Issues and PRs related to the s390x architecture.linuxIssues and PRs related to the Linux platform.Issues and PRs related to the Linux platform.
on Jul 20, 2024 I ran a
git bisect runwith#!/usr/bin/env bash ./configure --verbose make -j 88 tools/test.py --repeat=1000 -J parallel/test-assert-esm-cjs-message-verify$ git bisect log git bisect start # status: waiting for both good and bad commits # bad: [cf8e5356d9fbbdca40d9b9fe062b3c292b7e91e3] lib: improve error message when index not found on cjs git bisect bad cf8e5356d9fbbdca40d9b9fe062b3c292b7e91e3 # status: waiting for good commit(s), bad commit known # good: [0b1ff6965e3bb3cedfe3563f4ddefc33e7fa69d2] src: fix potential segmentation fault in SQLite git bisect good 0b1ff6965e3bb3cedfe3563f4ddefc33e7fa69d2 # good: [3d019ce6af35399d78617d4df57ce248d3d37d83] cli: document `--inspect` port `0` behavior git bisect good 3d019ce6af35399d78617d4df57ce248d3d37d83 # bad: [a523c345b153616a8265f8691ebfa8547273f27a] inspector: add initial support for network inspection git bisect bad a523c345b153616a8265f8691ebfa8547273f27a # bad: [7168295e7a94e6cfef69e43d26d7d5b57d3e1cf9] fs: move `rmSync` implementation to c++ git bisect bad 7168295e7a94e6cfef69e43d26d7d5b57d3e1cf9 # good: [cafd44dc7eff20cfa6c36b289e24efd793d4422a] esm: refactor `get_format` git bisect good cafd44dc7eff20cfa6c36b289e24efd793d4422a # first bad commit: [7168295e7a94e6cfef69e43d26d7d5b57d3e1cf9] fs: move `rmSync` implementation to c++ $Also seen on Actions
Yagiz and I discussed this privately, Yagiz investigation led him to think the flakiness was introduced because the old implementation was using
setTimeout(not IO blocking), and the new one is usingsleep(IO blocking). IMO that's a valid change, having a sync method not IO blocking was a weird design (according to Yagiz, it was due tormandrmSyncsharing the same code), so my recommendation would be to keep the IO blocking implementation – but not land it on any release line, to not risk break our users that would rely on that behavior, like this test was.Reacted by Richard Lau- added a commit that references this issue
on Jul 21, 2024 - added a commit that references this issue
on Jul 28, 2024 According to https://github.com/nodejs/node/pull/53617/files#diff-102db77122b5d23f42d497abfc7d626e60723d911caa6293fd16a5ec92b0d653L220, the old implementation was using
sleep(I don't see howsetTimeoutcould be used correctly in a synchronous function)- added a commit that references this issue
on Aug 5, 2024
Test
parallel/test-assert-esm-cjs-message-verify
Platform
Linux PPC64LE, Linux s390x, Linux x64
Console output
Build links
Additional information
This shows up in today's Reliability report https://github.com/nodejs/reliability/actions/runs/10015836224 (issue creation failed because the "body is too long (maximum is 65536 characters"):
It looks like the test sporadically failed on Windows previously (according to past reliability reports) but the Linux failures are only showing up in today's report.
I can also reproduce this locally on Linux x64 (RHEL 9) with
Failures rates vary (seeing anything between 4-10 failures per 1000 runs).