test: fix six recurring CI flakes - #65780
Closed
codebytere wants to merge 6 commits into
Closed
Conversation
Collaborator
|
Review requested:
|
This comment has been minimized.
This comment has been minimized.
panva
approved these changes
Sep 4, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65780 +/- ##
=======================================
Coverage 90.19% 90.20%
=======================================
Files 770 770
Lines 264410 264410
Branches 50243 50236 -7
=======================================
+ Hits 238490 238516 +26
+ Misses 16924 16907 -17
+ Partials 8996 8987 -9 🚀 New features to boost your workflow:
|
Member
|
cc @nodejs/build this jenkins run started by @codebytere does not have Resume CI |
Contributor
|
It's similar to the problem I also reported in nodejs/build#4453 17:15:25 FATAL: Channel "hudson.remoting.Channel@39c0b532:JNLP4-connect connection from 67.158.54.159/67.158.54.159:54594": Remote call on JNLP4-connect connection from 67.158.54.159/67.158.54.159:54594 failed. The channel is closing down or has closed down |
benjamingr
approved these changes
Sep 4, 2026
This comment has been minimized.
This comment has been minimized.
jasnell
approved these changes
Sep 4, 2026
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
|
There is another PR (#65770) for test/parallel/test-child-process-fork-closed-channel-segfault.js` that was opened before this one. |
test-https-proxy-request-invalid-char-in-url is the only client-proxy test that asserts the proxy logged no socket errors at all. Once the last response has been read the client destroys its tunnel, and if the proxy is still relaying the upstream's TLS close_notify at that point the client answers with a reset, which the proxy records as ECONNRESET on the CONNECT socket. That has been failing the test on macOS even though every request was routed to the sanitized URL. Keep asserting on other errors but leave connection resets out. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
test-external-memory-reasonable-size makes a child allocate 1.2 GB of external memory so that V8's --external-memory-max-reasonable-size check fires and the process aborts. The abort raises SIGABRT with all of that memory resident, and on hosts that write core files (the SmartOS CI machines in particular) the dump takes longer than the test timeout, so the test has been timing out there since it was added. Run the child under `ulimit -c 0` on POSIX, the same way test-abort-fatal-error and common.childShouldThrowAndAbort() handle their aborting children. Refs: nodejs#65589 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
test-run-watch-emit-restarted expected exactly one test:watch:restarted
event, but it starts run({ watch: true }) right after writing the
fixtures into the watched directory. Watch backends that deliver events
with some latency, FSEvents on macOS most visibly, can still report
those setup writes once the first run is under way, which restarts it
and makes the later, intentional write the second restart. The test has
been marked flaky on macOS x64 for that reason.
Wait for the first drain, then require that the write is followed by a
restart and a drain, ignoring whatever the setup produced before it, and
drop the flaky marker.
Refs: nodejs#54534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: nodejs#64226 Refs: nodejs#59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
Member
Author
codebytere
force-pushed
the
test-deflake
branch
from
September 5, 2026 18:01
ce80b32 to
8783b15
Compare
Member
|
i'm suspecting a number of cc @jasnell off just these two deflake PRs that ran its CI today: |
Most node:bench tests record a sample with `b.start();
process.hrtime.bigint(); b.end(1)`, counting on the extra clock read to
make end() see a later timestamp than start(). On a debian12-x64 CI
host the monotonic clock is coarse enough that all three reads returned
the same value, so end() threw ERR_INVALID_STATE ("insufficient clock
precision for benchmark sample") and test-bench-harness-errors failed;
every test using the idiom can fail the same way there.
Add test/common/bench.js with completeSample(), which spins until
process.hrtime.bigint() moves between start() and end(), and use it at
all of those sites, bench-runner fixtures included.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
nodejs-github-bot
pushed a commit
that referenced
this pull request
Sep 6, 2026
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: #64226 Refs: #59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
nodejs-github-bot
pushed a commit
that referenced
this pull request
Sep 6, 2026
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
nodejs-github-bot
pushed a commit
that referenced
this pull request
Sep 6, 2026
Most node:bench tests record a sample with `b.start();
process.hrtime.bigint(); b.end(1)`, counting on the extra clock read to
make end() see a later timestamp than start(). On a debian12-x64 CI
host the monotonic clock is coarse enough that all three reads returned
the same value, so end() threw ERR_INVALID_STATE ("insufficient clock
precision for benchmark sample") and test-bench-harness-errors failed;
every test using the idiom can fail the same way there.
Add test/common/bench.js with completeSample(), which spins until
process.hrtime.bigint() moves between start() and end(), and use it at
all of those sites, bench-runner fixtures included.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-https-proxy-request-invalid-char-in-url is the only client-proxy test that asserts the proxy logged no socket errors at all. Once the last response has been read the client destroys its tunnel, and if the proxy is still relaying the upstream's TLS close_notify at that point the client answers with a reset, which the proxy records as ECONNRESET on the CONNECT socket. That has been failing the test on macOS even though every request was routed to the sanitized URL. Keep asserting on other errors but leave connection resets out. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-external-memory-reasonable-size makes a child allocate 1.2 GB of external memory so that V8's --external-memory-max-reasonable-size check fires and the process aborts. The abort raises SIGABRT with all of that memory resident, and on hosts that write core files (the SmartOS CI machines in particular) the dump takes longer than the test timeout, so the test has been timing out there since it was added. Run the child under `ulimit -c 0` on POSIX, the same way test-abort-fatal-error and common.childShouldThrowAndAbort() handle their aborting children. Refs: #65589 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-run-watch-emit-restarted expected exactly one test:watch:restarted
event, but it starts run({ watch: true }) right after writing the
fixtures into the watched directory. Watch backends that deliver events
with some latency, FSEvents on macOS most visibly, can still report
those setup writes once the first run is under way, which restarts it
and makes the later, intentional write the second restart. The test has
been marked flaky on macOS x64 for that reason.
Wait for the first drain, then require that the write is followed by a
restart and a drain, ignoring whatever the setup produced before it, and
drop the flaky marker.
Refs: #54534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: #64226 Refs: #59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
Most node:bench tests record a sample with `b.start();
process.hrtime.bigint(); b.end(1)`, counting on the extra clock read to
make end() see a later timestamp than start(). On a debian12-x64 CI
host the monotonic clock is coarse enough that all three reads returned
the same value, so end() threw ERR_INVALID_STATE ("insufficient clock
precision for benchmark sample") and test-bench-harness-errors failed;
every test using the idiom can fail the same way there.
Add test/common/bench.js with completeSample(), which spins until
process.hrtime.bigint() moves between start() and end(), and use it at
all of those sites, bench-runner fixtures included.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-https-proxy-request-invalid-char-in-url is the only client-proxy test that asserts the proxy logged no socket errors at all. Once the last response has been read the client destroys its tunnel, and if the proxy is still relaying the upstream's TLS close_notify at that point the client answers with a reset, which the proxy records as ECONNRESET on the CONNECT socket. That has been failing the test on macOS even though every request was routed to the sanitized URL. Keep asserting on other errors but leave connection resets out. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-external-memory-reasonable-size makes a child allocate 1.2 GB of external memory so that V8's --external-memory-max-reasonable-size check fires and the process aborts. The abort raises SIGABRT with all of that memory resident, and on hosts that write core files (the SmartOS CI machines in particular) the dump takes longer than the test timeout, so the test has been timing out there since it was added. Run the child under `ulimit -c 0` on POSIX, the same way test-abort-fatal-error and common.childShouldThrowAndAbort() handle their aborting children. Refs: #65589 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-run-watch-emit-restarted expected exactly one test:watch:restarted
event, but it starts run({ watch: true }) right after writing the
fixtures into the watched directory. Watch backends that deliver events
with some latency, FSEvents on macOS most visibly, can still report
those setup writes once the first run is under way, which restarts it
and makes the later, intentional write the second restart. The test has
been marked flaky on macOS x64 for that reason.
Wait for the first drain, then require that the write is followed by a
restart and a drain, ignoring whatever the setup produced before it, and
drop the flaky marker.
Refs: #54534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: #64226 Refs: #59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
Most node:bench tests record a sample with `b.start();
process.hrtime.bigint(); b.end(1)`, counting on the extra clock read to
make end() see a later timestamp than start(). On a debian12-x64 CI
host the monotonic clock is coarse enough that all three reads returned
the same value, so end() threw ERR_INVALID_STATE ("insufficient clock
precision for benchmark sample") and test-bench-harness-errors failed;
every test using the idiom can fail the same way there.
Add test/common/bench.js with completeSample(), which spins until
process.hrtime.bigint() moves between start() and end(), and use it at
all of those sites, bench-runner fixtures included.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
R31K4G3
pushed a commit
to R31K4G3/node
that referenced
this pull request
Sep 9, 2026
test-https-proxy-request-invalid-char-in-url is the only client-proxy test that asserts the proxy logged no socket errors at all. Once the last response has been read the client destroys its tunnel, and if the proxy is still relaying the upstream's TLS close_notify at that point the client answers with a reset, which the proxy records as ECONNRESET on the CONNECT socket. That has been failing the test on macOS even though every request was routed to the sanitized URL. Keep asserting on other errors but leave connection resets out. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: nodejs#65780 Fixes: nodejs#64226 Refs: nodejs#59146 Refs: nodejs#54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
R31K4G3
pushed a commit
to R31K4G3/node
that referenced
this pull request
Sep 9, 2026
test-run-watch-emit-restarted expected exactly one test:watch:restarted
event, but it starts run({ watch: true }) right after writing the
fixtures into the watched directory. Watch backends that deliver events
with some latency, FSEvents on macOS most visibly, can still report
those setup writes once the first run is under way, which restarts it
and makes the later, intentional write the second restart. The test has
been marked flaky on macOS x64 for that reason.
Wait for the first drain, then require that the write is followed by a
restart and a drain, ignoring whatever the setup produced before it, and
drop the flaky marker.
Refs: nodejs#54534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: nodejs#65780
Fixes: nodejs#64226
Refs: nodejs#59146
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
R31K4G3
pushed a commit
to R31K4G3/node
that referenced
this pull request
Sep 9, 2026
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: nodejs#64226 Refs: nodejs#59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: nodejs#65780 Refs: nodejs#54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
R31K4G3
pushed a commit
to R31K4G3/node
that referenced
this pull request
Sep 9, 2026
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: nodejs#65780
Fixes: nodejs#64226
Refs: nodejs#59146
Refs: nodejs#54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-https-proxy-request-invalid-char-in-url is the only client-proxy test that asserts the proxy logged no socket errors at all. Once the last response has been read the client destroys its tunnel, and if the proxy is still relaying the upstream's TLS close_notify at that point the client answers with a reset, which the proxy records as ECONNRESET on the CONNECT socket. That has been failing the test on macOS even though every request was routed to the sanitized URL. Keep asserting on other errors but leave connection resets out. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-run-watch-emit-restarted expected exactly one test:watch:restarted
event, but it starts run({ watch: true }) right after writing the
fixtures into the watched directory. Watch backends that deliver events
with some latency, FSEvents on macOS most visibly, can still report
those setup writes once the first run is under way, which restarts it
and makes the later, intentional write the second restart. The test has
been marked flaky on macOS x64 for that reason.
Wait for the first drain, then require that the write is followed by a
restart and a drain, ignoring whatever the setup produced before it, and
drop the flaky marker.
Refs: #54534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: #64226 Refs: #59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-https-proxy-request-invalid-char-in-url is the only client-proxy test that asserts the proxy logged no socket errors at all. Once the last response has been read the client destroys its tunnel, and if the proxy is still relaying the upstream's TLS close_notify at that point the client answers with a reset, which the proxy records as ECONNRESET on the CONNECT socket. That has been failing the test on macOS even though every request was routed to the sanitized URL. Keep asserting on other errors but leave connection resets out. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-run-watch-emit-restarted expected exactly one test:watch:restarted
event, but it starts run({ watch: true }) right after writing the
fixtures into the watched directory. Watch backends that deliver events
with some latency, FSEvents on macOS most visibly, can still report
those setup writes once the first run is under way, which restarts it
and makes the later, intentional write the second restart. The test has
been marked flaky on macOS x64 for that reason.
Wait for the first drain, then require that the write is followed by a
restart and a drain, ignoring whatever the setup produced before it, and
drop the flaky marker.
Refs: #54534
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-wasi-pthread fails now and then on every platform with "Assertion failed: r == 0 (c/pthread.c: main: 17)", i.e. pthread_create() itself reporting an error. The fixture implements `thread-spawn` by starting a Worker and blocking in Atomics.wait(result, 0, 0, 1000) until the worker signals that it has instantiated the module. Two things go wrong there: the worker signals success by storing 0, the value the main thread is already waiting on, so when the worker is quicker than the main thread its notify is lost and the wait runs into the timeout; and one second is not always enough for a Worker to start and instantiate a threads build on the slower CI hosts (arm debug, Windows, macOS). Either way spawn() returns -6 and wasi-libc turns that into a pthread_create() failure. Wait on a sentinel value that neither outcome writes, and give the worker a platform-scaled 30 seconds. Drop the flaky markers. Fixes: #64226 Refs: #59146 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95
pushed a commit
that referenced
this pull request
Sep 9, 2026
test-dns-resolver-max-timeout times a query with `{ timeout: 500,
tries: 3 }` against one that also sets `maxTimeout: 500` and asserts
the first took longer. c-ares only expires a try when cares_wrap's
timer fires, every `timeout` ms, so each try costs one or two ticks
depending on sub-millisecond ordering, and uncapped retries also get
0.5-1x jitter. That leaves the capped run anywhere in 1500-3000 ms and
the uncapped one in 3000-4500 ms; on a busy rhel10-ppc64le host both
came out at 3005 ms. With `timeout: 100, tries: 5` the ranges (about
600-1000 ms and 5000-7500 ms measured under load) cannot meet.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65780
Fixes: #64226
Refs: #59146
Refs: #54534
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six CI flakes on
main, each root-caused and fixed in its own commit. Four keep showing up in the nodejs/reliability reports over the last two weeks (picked by counting distinctmain-targeting PRs they failed in since 2026-08-21, dropping everything since fixed onmainor already in review in #65755, #65767, #65770); the last two (test-dns-resolver-max-timeout, thenode:benchtests) failed this PR's own CI runs this week and will be in the next reports.client-proxy/test-https-proxy-request-invalid-char-in-urlECONNRESETparallel/test-external-memory-reasonable-sizeulimit -c 0like the other abort teststest-runner/test-run-watch-emit-restartedwasi/test-wasi-pthreadthread-spawnwaits on the same value the worker stores on success, so an early notify is lost, and gives the worker only 1 s to instantiate; either waypthread_create()fails. Flaky marks droppedparallel/test-dns-resolver-max-timeout{ timeout: 500, tries: 3 }with and withoutmaxTimeout: 500, but c-ares only expires a try on cares_wrap'stimeoutms timer tick (one or two ticks per try) and jitters uncapped retries by 0.5-1x, so the two ranges meet at 3000 ms; nowtimeout: 100, tries: 5, where they are ~600-1000 ms vs ~5000-7500 msparallel/test-bench-*b.start(); process.hrtime.bigint(); b.end(1)and rely on the middle read to move the clock; on that host all three reads return the same value andend()throwsinsufficient clock precision. Now acompleteSample()helper intest/common/bench.jsspins until hrtime moves, used at every site and in the bench-runner fixturesLooked at and left alone:
parallel/test-runner-run("should support timeout" reportinguncaughtExceptionon macos15-x64) doesn't reproduce under load on Linux and I couldn't pin it down from the code;sequential/test-debugger-pidandtest-run-watch-cwd-isolation-none*need a Windows/macOS box;pummel/test-fs-watch-non-recursiveon AIX is the host running out of AHAFS watchers.Tests: each changed test passes locally (Linux x64), 16-48 repeats at
-j16for the watch and WASI ones, 40 concurrent runs beside 24 busy-loop processes for the dns one, alltest-bench-*under a preload that quantizesprocess.hrtime.bigint()to 100 µs (six of them fail that way before the change), and theparallel/ones also undertools/test.py --worker.Fixes: #64226
Refs: #59146
Refs: #54534
Disclosure: the code and this description were written by Claude Code, directed and reviewed by @codebytere.