mirror of
https://github.com/block/buzz.git
synced 2026-10-29 21:41:33 +00:00
Make the thread-head E2E proof deterministic (P1 on #6887)
Address Wes/Carl's P1: the spec delayed get_thread_replies by 4s but both assertions used Playwright's 5s auto-retry window. On the buggy code the panel first shows the stale body, then the delayed relay response (which carries the root edit) lands inside the retry window and heals it, so the positive assertion can false-green. Navigation time eats into the 4s, making the false-green path more likely under load. The delay is a timer that self-heals; it does not prove the head opens with the channel-window overlay rather than healing later. Replace the timer with a manual gate. New `deferThreadReplies` mock option holds every get_thread_replies response open until the test calls `__BUZZ_E2E_RELEASE_THREAD_REPLIES__()` — mirroring the existing deferLinkPreviewMetadata / RELEASE_GET_EVENT queue pattern. The thread-aux backfill provably cannot land before the assertion, so: - head-edited assertion runs while the fetch is verifiably held (`__BUZZ_E2E_THREAD_REPLIES_PENDING__` poll guards against the fetch never having been dispatched); - release the gate and re-assert the head stays edited (dedup path). Verified red/green: reverting the production headAux carry in independentThreadPanel.ts makes the spec fail deterministically at the head assertion (received the stale "these two PRs?"); with the fix it passes. tsc clean; 8/8 independentThreadPanel unit cases pass. Signed-off-by: Leo <5faf251baee50ee6bcde338aef6acdd70bb3e60115664c2cd490d94a55dfc488@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -370,6 +370,13 @@ type E2eConfig = {
|
||||
/** Delay (ms) after snapshotting a thread-replies page so E2E tests can
|
||||
* deliver live reply/aux events while an older response is in flight. */
|
||||
threadRepliesDelayMs?: number;
|
||||
/** Hold every `get_thread_replies` response until
|
||||
* `__BUZZ_E2E_RELEASE_THREAD_REPLIES__()` is called. Unlike
|
||||
* `threadRepliesDelayMs` (a timer that self-heals inside Playwright's
|
||||
* auto-retry window), this is a manual gate: the thread-aux backfill
|
||||
* provably never lands until the test releases it, so a spec can assert
|
||||
* the panel's head state before any backfill can heal it. */
|
||||
deferThreadReplies?: boolean;
|
||||
usersBatchDelayMs?: number;
|
||||
/** Delay (ms) applied to continuation channel-window requests so e2e
|
||||
* tests can observe the in-flight prepend window. 0/undefined = instant. */
|
||||
@@ -1440,6 +1447,12 @@ declare global {
|
||||
__BUZZ_E2E_RELEASE_LINK_PREVIEW_METADATA__?: () => number;
|
||||
/** Release link-preview uploads held before mock-native registration. */
|
||||
__BUZZ_E2E_RELEASE_LINK_PREVIEW_UPLOADS__?: () => number;
|
||||
/** Flush every `get_thread_replies` call held by `deferThreadReplies`.
|
||||
* Returns the number of held requests released. */
|
||||
__BUZZ_E2E_RELEASE_THREAD_REPLIES__?: () => number;
|
||||
/** Number of `get_thread_replies` calls currently held by
|
||||
* `deferThreadReplies`. */
|
||||
__BUZZ_E2E_THREAD_REPLIES_PENDING__?: () => number;
|
||||
/** Uploads that passed mock-native registration and began relay work. */
|
||||
__BUZZ_E2E_LINK_PREVIEW_UPLOAD_STARTS__?: number;
|
||||
}
|
||||
@@ -1548,6 +1561,7 @@ type DeferredGetEvent = {
|
||||
let deferredGetEventQueue: DeferredGetEvent[] = [];
|
||||
let deferredLinkPreviewMetadataQueue: Array<() => void> = [];
|
||||
let deferredLinkPreviewUploadQueue: Array<() => void> = [];
|
||||
let deferredThreadRepliesQueue: Array<() => void> = [];
|
||||
let cancelledMediaUploadIds = new Set<string>();
|
||||
let deferNextChannelsRead = false;
|
||||
let deferredChannelsReadResolve: (() => void) | null = null;
|
||||
@@ -5161,6 +5175,11 @@ async function handleGetThreadReplies(
|
||||
if (delayMs > 0) {
|
||||
await new Promise((resolve) => window.setTimeout(resolve, delayMs));
|
||||
}
|
||||
if (config?.mock?.deferThreadReplies) {
|
||||
await new Promise<void>((resolve) => {
|
||||
deferredThreadRepliesQueue.push(resolve);
|
||||
});
|
||||
}
|
||||
|
||||
return { events: page, next_cursor: nextCursor };
|
||||
}
|
||||
@@ -10709,6 +10728,7 @@ export function maybeInstallE2eTauriMocks() {
|
||||
deferredSendMessageLiveEchoes.length = 0;
|
||||
deferredLinkPreviewMetadataQueue = [];
|
||||
deferredLinkPreviewUploadQueue = [];
|
||||
deferredThreadRepliesQueue = [];
|
||||
cancelledMediaUploadIds = new Set<string>();
|
||||
window.__BUZZ_E2E_LINK_PREVIEW_UPLOAD_STARTS__ = 0;
|
||||
window.__BUZZ_E2E_RELEASE_LINK_PREVIEW_METADATA__ = () => {
|
||||
@@ -10721,6 +10741,13 @@ export function maybeInstallE2eTauriMocks() {
|
||||
for (const release of queued) release();
|
||||
return queued.length;
|
||||
};
|
||||
window.__BUZZ_E2E_RELEASE_THREAD_REPLIES__ = () => {
|
||||
const queued = deferredThreadRepliesQueue.splice(0);
|
||||
for (const release of queued) release();
|
||||
return queued.length;
|
||||
};
|
||||
window.__BUZZ_E2E_THREAD_REPLIES_PENDING__ = () =>
|
||||
deferredThreadRepliesQueue.length;
|
||||
mockGlobalAgentConfig = config.mock?.globalAgentConfig
|
||||
? { ...config.mock.globalAgentConfig }
|
||||
: null;
|
||||
|
||||
@@ -9,9 +9,12 @@ import { waitForAnimations } from "../helpers/animations";
|
||||
// edit) but its overlay was previously dropped, relying solely on the async
|
||||
// thread-reply aux backfill — so before that fetch lands the head renders stale.
|
||||
//
|
||||
// We reproduce the divergence deterministically by delaying the thread-replies
|
||||
// fetch (`threadRepliesDelayMs`): the channel window already holds the edit, but
|
||||
// the thread-aux response is still in flight when the panel first renders.
|
||||
// We reproduce the divergence deterministically by *gating* the thread-replies
|
||||
// fetch (`deferThreadReplies`): the channel window already holds the edit, but
|
||||
// the thread-aux response is held open — it provably cannot land until the test
|
||||
// releases it. This is stronger than a timed delay: a 4s timer self-heals
|
||||
// inside Playwright's auto-retry window, so on the buggy code the delayed aux
|
||||
// backfill could arrive mid-assertion and false-green. A held gate cannot.
|
||||
|
||||
const CHANNEL = "general";
|
||||
const SHOT = "test-results/thread-head-stale-edit";
|
||||
@@ -52,9 +55,9 @@ async function waitForMockLiveSubscription(
|
||||
test("thread head reflects the channel-window edit even before thread aux loads", async ({
|
||||
page,
|
||||
}) => {
|
||||
// Long delay so the thread-replies (and its aux edit backfill) response is
|
||||
// still in flight when the thread panel first renders the head.
|
||||
await installMockBridge(page, { threadRepliesDelayMs: 4000 });
|
||||
// Hold the thread-replies (and its aux edit backfill) response open so it
|
||||
// provably cannot land until we release it after asserting the head.
|
||||
await installMockBridge(page, { deferThreadReplies: true });
|
||||
await page.goto("/");
|
||||
await page.getByTestId(`channel-${CHANNEL}`).click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText(CHANNEL);
|
||||
@@ -124,14 +127,44 @@ test("thread head reflects the channel-window edit even before thread aux loads"
|
||||
await expect(threadPanel).toBeVisible();
|
||||
|
||||
// 4. The thread head must show the EDITED body immediately — while the
|
||||
// thread-aux backfill is still delayed in flight. Pre-fix this rendered
|
||||
// the stale original ("these two PRs?").
|
||||
// thread-aux backfill is still gated (provably not yet delivered). Pre-fix
|
||||
// this rendered the stale original ("these two PRs?"), and because the gate
|
||||
// is held (not merely delayed) no backfill can arrive to heal it.
|
||||
const headBody = threadPanel
|
||||
.locator(`[data-testid="message-row"][data-message-id="${rootId}"]`)
|
||||
.getByTestId("message-body");
|
||||
await expect(headBody).toContainText("these PRs?");
|
||||
await expect(headBody).not.toContainText("these two PRs?");
|
||||
|
||||
// The thread-aux fetch must actually be held: this proves the edited head
|
||||
// above came purely from the channel-window overlay, not from a backfill that
|
||||
// slipped in. (Guards against the fetch never having been dispatched.)
|
||||
await expect
|
||||
.poll(() =>
|
||||
page.evaluate(
|
||||
() =>
|
||||
(
|
||||
window as Window & {
|
||||
__BUZZ_E2E_THREAD_REPLIES_PENDING__?: () => number;
|
||||
}
|
||||
).__BUZZ_E2E_THREAD_REPLIES_PENDING__?.() ?? 0,
|
||||
),
|
||||
)
|
||||
.toBeGreaterThan(0);
|
||||
|
||||
await waitForAnimations(page);
|
||||
await page.screenshot({ path: `${SHOT}/thread-head-edited.png` });
|
||||
|
||||
// 5. Release the gate; the held thread-aux backfill now lands and the head
|
||||
// stays edited (dedup against relay-provided aux, no regression).
|
||||
await page.evaluate(
|
||||
() =>
|
||||
(
|
||||
window as Window & {
|
||||
__BUZZ_E2E_RELEASE_THREAD_REPLIES__?: () => number;
|
||||
}
|
||||
).__BUZZ_E2E_RELEASE_THREAD_REPLIES__?.(),
|
||||
);
|
||||
await expect(headBody).toContainText("these PRs?");
|
||||
await expect(headBody).not.toContainText("these two PRs?");
|
||||
});
|
||||
|
||||
@@ -301,6 +301,11 @@ type MockBridgeOptions = {
|
||||
/** Delay (ms) after snapshotting a thread-replies page so E2E tests can
|
||||
* deliver live reply/aux events while an older response is in flight. */
|
||||
threadRepliesDelayMs?: number;
|
||||
/** Hold every `get_thread_replies` response until
|
||||
* `__BUZZ_E2E_RELEASE_THREAD_REPLIES__()` is called — a manual gate the test
|
||||
* releases explicitly, so the thread-aux backfill provably cannot land (and
|
||||
* heal a stale head) before assertions run. See e2eBridge mock config. */
|
||||
deferThreadReplies?: boolean;
|
||||
usersBatchDelayMs?: number;
|
||||
/** Delay (ms) for older-history fetches; see e2eBridge mock config. */
|
||||
channelWindowDelayMs?: number;
|
||||
|
||||
Reference in New Issue
Block a user