test(e2e): wait for the loaded channel list in the #1468 tests - #181
Merged
Merged
Conversation
…ests Both Kpa-clawbot#1468 tests snapshotted window._channelsGetStateForTest() as soon as the WS test hook existed. channels.js sets that hook synchronously in init(), before loadChannels() has an /api/channels answer, so `before` could read 0 channels. When the response then landed between the two evaluate() calls, `after` read the full list and the orphan test failed with before=0, after=5 although the orphan message was dropped (CI run 37002943213). With a delay on /api/channels the same race fails on #153's head too, and it also hits the control test: a WS-born channel pushed while the request is in flight is not in the snapshot, and mergeClientChannelState() only enriches rows the snapshot has. waitForChannelListSettled() waits until #chList has rendered the list (no .ch-loading), the count is > 0 and unchanged for 300 ms; both tests take their `before` snapshot only after that. Test-only change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 403075e)
dborup
marked this pull request as ready for review
October 3, 2026 07:02
This was referenced Oct 4, 2026
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.
Relates to #166.
What
This is a cherry-pick (
git cherry-pick -x) of403075e8from #166, with no other changes. Onlytest-e2e-playwright.jschanges (+28, test-only). #166 can only merge after #153, so the fix gets its own PR against master.The race
The two
#1468E2E tests ("live WS CHAN message with no payload.channel is dropped" and its control) snapshotwindow._channelsGetStateForTest()as soon as_channelsProcessWSBatchForTestexists.channels.jssets these WS test hooks synchronously ininit(), beforeloadChannels()has resolved/api/channels. So:beforecan read 0 channels. If the response lands between thebeforeandafterevaluates,afterreads the full list and the test fails withbefore=0, after=5, although the orphan message was dropped.loadChannels()installs.mergeClientChannelState()only enriches rows that the snapshot has.The flake has been seen on #166 and on #178 (which does not touch channels), both with
before=0, after=5.waitForChannelListSettled()waits until#chListhas rendered the list (no.ch-loading) and the channel count is > 0 and unchanged for 300 ms. Both tests take theirbeforesnapshot only after that.Evidence
The setup is a local Go server built from this branch on port 13581. It uses CI's fixture prep from
deploy.ymlon a copy oftest-fixtures/e2e-fixture.db: freshen, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed rows, migrate, andseed-2073-route-adverts.sql. The server serves plainpublic/, not the instrumented copy. Playwright 1.58.2 runs headless Chromium.The two test blocks were taken verbatim from each version of
test-e2e-playwright.jsand run in a loop. Each iteration starts on#/homeso#/channelsis a hash navigation, as in the full suite./api/channelswas delayed withpage.route./api/channelsdelayMaster's failures reproduce the CI message exactly:
channel count unchanged after orphan WS msg — before=0, after=5.A fixed 400 ms delay does not trigger it. With 400 ms, both evaluates run before the response arrives, so
before=0, after=0. The failure needs the response to land in the few milliseconds between the two evaluates. A probe confirms the window: the hook exists ~18 ms after navigation with 0 channels, and the list holds 5 channels once the response is in.On this branch, the two tests passed 180 runs in a row across all delays.
Mutants (from the cloud review)
Each mutant was served as a mutated
channels.jsviapage.route. No file was edited.if (!payload.channel) payload.channel = 'unknown';continue;)Merge note
403075e8stays on #166's branch, and #166's history is unchanged. Once this PR is merged, the same change arrives in master twice: here, and via #166's own commit. Merging #166 afterwards is a no-op for this hunk, because both sides made identical changes, so there is no conflict and no diff.deploy.ymlis unchanged (9 ×github.repository == 'Kpa-clawbot/CoreScope').🤖 Generated with Claude Code