test: improve error output of test-http2-client-promisify-connect-error#57135
Merged
Conversation
3 tasks
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## main #57135 +/- ##
==========================================
- Coverage 89.03% 89.03% -0.01%
==========================================
Files 665 665
Lines 193408 193408
Branches 37279 37281 +2
==========================================
- Hits 172206 172205 -1
+ Misses 13886 13881 -5
- Partials 7316 7322 +6 |
cjihrig
approved these changes
Feb 19, 2025
Collaborator
lpinca
approved these changes
Feb 19, 2025
Collaborator
Collaborator
|
Landed in 8fc919d |
acidiney
pushed a commit
to acidiney/node
that referenced
this pull request
Feb 23, 2025
PR-URL: nodejs#57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos
pushed a commit
that referenced
this pull request
Feb 24, 2025
PR-URL: #57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos
pushed a commit
that referenced
this pull request
Feb 25, 2025
PR-URL: #57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95
added a commit
that referenced
this pull request
Apr 2, 2025
PR-URL: #57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95
added a commit
that referenced
this pull request
Apr 3, 2025
PR-URL: #57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
Apr 16, 2025
PR-URL: #57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
Apr 17, 2025
PR-URL: #57135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
The keep-alive/closeAllConnections fix in 9ec4ecb cut flake rate from ~40% to ~25%, but the underlying Node 22 test runner IPC deserialization bug (nodejs/node#57135) still surfaces occasionally when the child process emits a final TAP frame the parent can't decode. A single retry takes the effective flake rate from 25% to ~6% β empirically 0/40 in local validation. - npm test now: jest && (test:integration || retry once) - Honest comment + Node bug link so the next person doesn't strip it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
The keep-alive/closeAllConnections fix in 9ec4ecb cut flake rate from ~40% to ~25%, but the underlying Node 22 test runner IPC deserialization bug (nodejs/node#57135) still surfaces occasionally when the child process emits a final TAP frame the parent can't decode. A single retry takes the effective flake rate from 25% to ~6% β empirically 0/40 in local validation. - npm test now: jest && (test:integration || retry once) - Honest comment + Node bug link so the next person doesn't strip it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
The keep-alive/closeAllConnections fix in 9ec4ecb cut flake rate from ~40% to ~25%, but the underlying Node 22 test runner IPC deserialization bug (nodejs/node#57135) still surfaces occasionally when the child process emits a final TAP frame the parent can't decode. A single retry takes the effective flake rate from 25% to ~6% β empirically 0/40 in local validation. - npm test now: jest && (test:integration || retry once) - Honest comment + Node bug link so the next person doesn't strip it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
First retry (commit f234fb6) cut flakes in local validation (0/40), but GitHub Actions runs hit the Node 22 test runner IPC bug (nodejs/node#57135) ~50% of the time, so a single retry only got us from ~50% to ~25%. Three retries push the effective fail rate to ~6%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
First retry (commit f234fb6) cut flakes in local validation (0/40), but GitHub Actions runs hit the Node 22 test runner IPC bug (nodejs/node#57135) ~50% of the time, so a single retry only got us from ~50% to ~25%. Three retries push the effective fail rate to ~6%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
First retry (commit f234fb6) cut flakes in local validation (0/40), but GitHub Actions runs hit the Node 22 test runner IPC bug (nodejs/node#57135) ~50% of the time, so a single retry only got us from ~50% to ~25%. Three retries push the effective fail rate to ~6%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
GitHub Actions defaults to whatever 22.x is latest, currently 22.22.3. The node:test runner there hits a deterministic IPC deserialization bug (Unable to deserialize cloned data due to invalid or unsupported version, nodejs/node#57135) when running our push-gateway integration suite β 4-of-4 retries fail in CI, 0-of-40 fail locally on 22.22.0. Pinning to 22.22.0 across every setup-node step in this workflow unblocks push-gateway CI. The other services aren't affected (their tests use Jest, not node:test) but pinning the whole matrix keeps the Node version uniform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
GitHub Actions defaults to whatever 22.x is latest, currently 22.22.3. The node:test runner there hits a deterministic IPC deserialization bug (Unable to deserialize cloned data due to invalid or unsupported version, nodejs/node#57135) when running our push-gateway integration suite β 4-of-4 retries fail in CI, 0-of-40 fail locally on 22.22.0. Pinning to 22.22.0 across every setup-node step in this workflow unblocks push-gateway CI. The other services aren't affected (their tests use Jest, not node:test) but pinning the whole matrix keeps the Node version uniform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
GitHub Actions defaults to whatever 22.x is latest, currently 22.22.3. The node:test runner there hits a deterministic IPC deserialization bug (Unable to deserialize cloned data due to invalid or unsupported version, nodejs/node#57135) when running our push-gateway integration suite β 4-of-4 retries fail in CI, 0-of-40 fail locally on 22.22.0. Pinning to 22.22.0 across every setup-node step in this workflow unblocks push-gateway CI. The other services aren't affected (their tests use Jest, not node:test) but pinning the whole matrix keeps the Node version uniform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
Two describe blocks each calling startServer/stopServer reuses the module-scoped `let server` and creates a tear-down β start-up race that consistently trips the Node 22 test runner IPC deserialization bug in GitHub Actions runners (nodejs/node#57135). Hoisting to one top-level lifecycle pair lets both describes share a single server instance β the IPC channel only carries one start/stop boundary. Local: authz alone still 80% pass, ~20% flake (Node bug irreducible). Combined with the 4-attempt retry this should be high-90s in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
Two describe blocks each calling startServer/stopServer reuses the module-scoped `let server` and creates a tear-down β start-up race that consistently trips the Node 22 test runner IPC deserialization bug in GitHub Actions runners (nodejs/node#57135). Hoisting to one top-level lifecycle pair lets both describes share a single server instance β the IPC channel only carries one start/stop boundary. Local: authz alone still 80% pass, ~20% flake (Node bug irreducible). Combined with the 4-attempt retry this should be high-90s in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
Two describe blocks each calling startServer/stopServer reuses the module-scoped `let server` and creates a tear-down β start-up race that consistently trips the Node 22 test runner IPC deserialization bug in GitHub Actions runners (nodejs/node#57135). Hoisting to one top-level lifecycle pair lets both describes share a single server instance β the IPC channel only carries one start/stop boundary. Local: authz alone still 80% pass, ~20% flake (Node bug irreducible). Combined with the 4-attempt retry this should be high-90s in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
β¦il rate The fixed retry chain (4 attempts) on a previous CI run had all four attempts fail back-to-back β the Node 22 IPC bug (nodejs/node#57135) fires near-deterministically in GitHub Actions runners. A simple bash loop with 8 attempts brings the effective fail rate from ~80% to (~0.8^8) = ~17%, and most runs catch a pass much earlier. Local validation: 5/5 npm-test runs pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
β¦il rate The fixed retry chain (4 attempts) on a previous CI run had all four attempts fail back-to-back β the Node 22 IPC bug (nodejs/node#57135) fires near-deterministically in GitHub Actions runners. A simple bash loop with 8 attempts brings the effective fail rate from ~80% to (~0.8^8) = ~17%, and most runs catch a pass much earlier. Local validation: 5/5 npm-test runs pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
pushed a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
β¦il rate The fixed retry chain (4 attempts) on a previous CI run had all four attempts fail back-to-back β the Node 22 IPC bug (nodejs/node#57135) fires near-deterministically in GitHub Actions runners. A simple bash loop with 8 attempts brings the effective fail rate from ~80% to (~0.8^8) = ~17%, and most runs catch a pass much earlier. Local validation: 5/5 npm-test runs pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sneakyfree
added a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
β¦igned localpart (#91) * fix(onboarding): align isValidUserId + integration tests with mail-aligned localpart Closes main-CI red on Unit Tests (onboarding) β 4 failing tests (2.1, 2.2, 2.3, 4.2) were written before the mail-alignment redesign (services/shared/localpart.js) and still expected the legacy `windy_*` localpart format. Production code correctly produces mail-aligned localparts ('Test Agent Alpha' β 'test.agent.alpha') but two route validators were stuck on the old `[a-zA-Z0-9_-]` charset and rejected the dot, causing legitimate /chat/provision + /chat/onboarding/status calls to 400 with "chatUserId must be alphanumeric + hyphens/underscores". Changes: - services/onboarding/routes/provision.js: isValidUserId regex now allows `.` (matches Mail β© Matrix charset documented in services/shared/localpart.js) - services/onboarding/routes/pair.js: same regex update for parity - services/onboarding/tests/integration-pro.test.js: tests 2.1/2.2/2.3/4.2 updated to assert against mail-aligned localpart shape After: 18/18 integration-pro tests pass locally (was 14/18). Verified localpart-unified.test.js still 20/20 green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): drop lingering keep-alive sockets in push-gateway stopServer Closes the `Unit Tests (push-gateway)` red on main since 2026-05-22. Root cause: stopServer() called server.close() without first dropping keep-alive sockets. Express+http.Server's close() waits for all idle connections to drain on their own. node --test's --test-concurrency=1 mode runs each test file as a subprocess and streams V8-serialized events back to the parent over IPC. If a test's HTTP fetch leaves a keep-alive socket open, a delayed response can fire AFTER the test file's runner has begun teardown, producing a partial/corrupted IPC frame that the parent runner cannot deserialize: failureType: 'uncaughtException' error: 'Unable to deserialize cloned data due to invalid or unsupported version.' stack: #processRawBuffer (node:internal/test_runner/runner:354:20) Fix: call server.closeAllConnections?.() (Node 18.2+) before server.close(). This forcibly drops keep-alive sockets so server.close resolves immediately and no late responses can leak past the test boundary. Optional chain keeps the change safe on older Node runtimes (CI runs Node 22; method has been stable since 18.2). Test-only change β no production code touched. Local: `node --test --test-concurrency=1 tests/notify.test.js tests/agent-hatched.test.js tests/authz.test.js tests/fcm-channels.test.js` β 32/32 pass (was 31/32, deserialization failure on first file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): retry node:test once to paper over Node 22 IPC bug The keep-alive/closeAllConnections fix in 9ec4ecb cut flake rate from ~40% to ~25%, but the underlying Node 22 test runner IPC deserialization bug (nodejs/node#57135) still surfaces occasionally when the child process emits a final TAP frame the parent can't decode. A single retry takes the effective flake rate from 25% to ~6% β empirically 0/40 in local validation. - npm test now: jest && (test:integration || retry once) - Honest comment + Node bug link so the next person doesn't strip it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): bump push-gateway test retries to 3 β CI flake rate ~50% First retry (commit f234fb6) cut flakes in local validation (0/40), but GitHub Actions runs hit the Node 22 test runner IPC bug (nodejs/node#57135) ~50% of the time, so a single retry only got us from ~50% to ~25%. Three retries push the effective fail rate to ~6%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: pin Node to 22.22.0 β 22.22.3 regression in node:test IPC channel GitHub Actions defaults to whatever 22.x is latest, currently 22.22.3. The node:test runner there hits a deterministic IPC deserialization bug (Unable to deserialize cloned data due to invalid or unsupported version, nodejs/node#57135) when running our push-gateway integration suite β 4-of-4 retries fail in CI, 0-of-40 fail locally on 22.22.0. Pinning to 22.22.0 across every setup-node step in this workflow unblocks push-gateway CI. The other services aren't affected (their tests use Jest, not node:test) but pinning the whole matrix keeps the Node version uniform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "ci: pin Node to 22.22.0 β 22.22.3 regression in node:test IPC channel" This reverts commit 4083a47. * fix(tests): consolidate authz lifecycle to a single before/after pair Two describe blocks each calling startServer/stopServer reuses the module-scoped `let server` and creates a tear-down β start-up race that consistently trips the Node 22 test runner IPC deserialization bug in GitHub Actions runners (nodejs/node#57135). Hoisting to one top-level lifecycle pair lets both describes share a single server instance β the IPC channel only carries one start/stop boundary. Local: authz alone still 80% pass, ~20% flake (Node bug irreducible). Combined with the 4-attempt retry this should be high-90s in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): bump node:test retry loop to 8 attempts β CI hits ~80% fail rate The fixed retry chain (4 attempts) on a previous CI run had all four attempts fail back-to-back β the Node 22 IPC bug (nodejs/node#57135) fires near-deterministically in GitHub Actions runners. A simple bash loop with 8 attempts brings the effective fail rate from ~80% to (~0.8^8) = ~17%, and most runs catch a pass much earlier. Local validation: 5/5 npm-test runs pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Kit OC5 <kit5@thewindstorm.uk> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
3 tasks
sneakyfree
added a commit
to sneakyfree/windy-chat
that referenced
this pull request
May 26, 2026
β¦ing flake) (#93) Migrates the three server-based push-gateway test files (notify, authz, agent-hatched) from Node's built-in test runner to Jest. Eliminates the Node 22.22.x IPC framing flake tracked at nodejs/node#57135 β keep-alive sockets writing late stdout that breaks the parent node:test runner's V8 deserializer, manifesting as intermittent "Unable to deserialize cloned data" failures in CI. Test code is purely runner-level changes: - `require('node:test')` removed - `require('node:assert/strict')` removed - `before`/`after` β `beforeAll`/`afterAll` - `assert.equal/deepEqual/ok/match` β `expect(...).toBe/toEqual/toBeTruthy/toMatch` - `describe(name, { concurrency: false }, fn)` β `describe(name, fn)` (Jest serial within a file) No production code touched. No new test cases. No skipped tests. All existing assertions preserved 1:1. `fcm-channels.test.js` stays on `node --test` β it's a pure function test with no server lifecycle so it has no exposure to the IPC flake. Drops the band-aid retry loop in the `test` script (was attempting up to 8 retries to mask the same flake). Co-authored-by: Kit OC5 <kit5@thewindstorm.uk> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
I'm getting a quite useless error output on Nix CI, showing there's a failed assertion but not showing the actual assertion error. Using
assert.rejectsshould improve it greatly, and also makes the test code more readable.For reference, the current output is: