Skip to content

Commit 487b82f

Browse files
sirozhaclaude
andcommitted
test(e2e): unit-test the mock matcher instead of narrating it, drop dead comments
The mock world's matchers carried five explanatory comments and no test. Cover them directly — order-independent variable matching, nested-object subset, sequenced-then-repeated entries, a mismatched REST body staying unmatched, and flag-gated visibility — and delete the prose they replace. The order test is built key-by-key on purpose: a literal would be re-sorted by the linter, which silently defeated the first version (a stripped key-sort still passed until the value was moved inside an array, where stableStringify actually normalises it). Sweep the rest of e2e for comments that only narrate the test or restate the code — step labels, "happy path", cassette descriptions, field docstrings — and remove them. Kept: framework gotchas whose violation is silent (a probe reading a colour mid-transition, react-hook-form's disabled Submit, a pre-ack frame that storms graphql-ws), security notes, magic-value and source-of-truth pointers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a69c9bf commit 487b82f

15 files changed

Lines changed: 90 additions & 74 deletions

frontend/e2e/helpers/terminal.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ interface XtermHost extends HTMLElement {
1414
};
1515
}
1616

17-
/** Reads the visible xterm buffer as plain text — the WebGL canvas has no DOM text to locate. */
1817
export const readTerminalBuffer = async (page: Page): Promise<string> =>
1918
page
2019
.locator('.xterm')

frontend/e2e/mocks/cassette.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ export interface CassetteFrame {
2020
*/
2121
export const entity = <T extends object>(typename: string, value: T): T => ({ __typename: typename, ...value });
2222

23-
/** Section-wise merge: an override replaces the base entry list per operation/path key. */
2423
export const mergeCassettes = (base: Cassette, override: Cassette): Cassette => ({
2524
mutations: { ...base.mutations, ...override.mutations },
2625
queries: { ...base.queries, ...override.queries },

frontend/e2e/mocks/cassettes/base.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ const settingsUser: ResultOf<typeof SettingsUserDocument> = {
4343
}),
4444
};
4545

46-
/** Everything the authenticated app shell requests on any page. */
4746
export const baseQueries = (): NonNullable<Cassette['queries']> => ({
4847
flows: [{ data: flows }],
4948
flowTemplates: [{ data: flowTemplates }],

frontend/e2e/mocks/cassettes/flows.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,6 @@ export const makeMessage = (
6969
...overrides,
7070
});
7171

72-
// One message per distinct render path in flow-message.tsx (all Answer-typed
73-
// cassettes never exercise these): thinking-toggle needs thinking+message,
74-
// report auto-expands its details, a Terminal resultFormat mounts the xterm
75-
// renderer, and Input right-aligns.
7672
export const VARIED_MESSAGES = [
7773
makeMessage('501', '5', { message: 'Planning the run', thinking: 'internal reasoning about the plan' }),
7874
makeMessage('502', '5', {
@@ -81,7 +77,6 @@ export const VARIED_MESSAGES = [
8177
resultFormat: ResultFormat.Markdown,
8278
type: MessageLogType.Report,
8379
}),
84-
// Report type auto-expands, so the Terminal renderer mounts on load.
8580
makeMessage('503', '5', {
8681
message: '',
8782
result: 'e2e-terminal-marker\nexit 0',
@@ -143,11 +138,6 @@ const addedFrame = (message: MessageLogFragmentFragment, delayMs: number) => ({
143138
payload: { data: { messageLogAdded: message } },
144139
});
145140

146-
/**
147-
* Two concurrent flows with disjoint message streams. The second `flow` entry
148-
* for flow 5 is the post-reconnect reconcile response: everything delivered so
149-
* far plus the message "missed" while the socket was down.
150-
*/
151141
export const flowsCassette = (override: Cassette = {}): Cassette =>
152142
mergeCassettes(
153143
{
@@ -188,7 +178,6 @@ export const flowsCassette = (override: Cassette = {}): Cassette =>
188178
override,
189179
);
190180

191-
/** Flow 5 pre-loaded with one message per render path, no streaming. */
192181
export const variedMessagesCassette = (): Cassette =>
193182
flowsCassette({
194183
queries: { flow: [{ data: flowQueryData(FLOW_A, VARIED_MESSAGES), variables: { id: '5' } }] },
@@ -315,7 +304,6 @@ const flowTabsData: ResultOf<typeof FlowDocument> = {
315304
vectorStoreLogs: [TABS_VECTOR_LOG],
316305
};
317306

318-
/** Flow 5 with one populated entry per detail tab, including the REST-fed screenshot image. */
319307
export const flowTabsCassette = (): Cassette =>
320308
flowsCassette({
321309
queries: {

frontend/e2e/mocks/cassettes/smoke.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ import type { Cassette } from '../cassette.ts';
33
import { authenticatedInfoEntry, guestInfoEntry, SEEDED_USER } from '../../fixtures/auth.ts';
44
import { baseQueries, baseRest } from './base.ts';
55

6-
/** Seeded-session smoke: every request answers as an authenticated user. */
76
export const smokeCassette: Cassette = {
87
queries: baseQueries(),
98
rest: baseRest(),
109
};
1110

12-
/** Login-form journey: /info serves guest until the login mutation raises the flag. */
1311
export const loginJourneyCassette: Cassette = {
1412
queries: baseQueries(),
1513
rest: {

frontend/e2e/mocks/world.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import type {
88
WorldFlagged,
99
} from './cassette.ts';
1010

11-
// Key-sorted stringify: the app builds variables objects at runtime (spreads,
12-
// conditional assignment), so property order must not affect matching.
1311
const stableStringify = (value: unknown): string =>
1412
JSON.stringify(value, (_key, node: unknown) =>
1513
node && typeof node === 'object' && !Array.isArray(node)
@@ -20,9 +18,6 @@ const stableStringify = (value: unknown): string =>
2018
const isPlainObject = (value: unknown): value is Record<string, unknown> =>
2119
typeof value === 'object' && value !== null && !Array.isArray(value);
2220

23-
// Recurses so a nested pin stays a subset — `{ input: { name } }` must not demand
24-
// that the whole `input` match, or any sibling the app also sends (a time-derived
25-
// TTL, a default) would make the entry unmatchable.
2621
const isSubsetMatch = (expected?: Record<string, unknown>, actual?: Record<string, unknown>): boolean =>
2722
!expected ||
2823
Object.entries(expected).every(([key, value]) => {
@@ -75,7 +70,6 @@ export class MockWorld {
7570
}
7671
}
7772

78-
/** Resolves once the flag is raised (immediately if it already is). */
7973
flagRaised(flag: string): Promise<void> {
8074
if (this.flags.has(flag)) {
8175
return Promise.resolve();
@@ -111,8 +105,6 @@ export class MockWorld {
111105

112106
return candidateMethod === method && candidatePath.replace(/\/+$/, '') === normalized;
113107
});
114-
// A path hit whose bodySubset does not match stays unmatched (501): a
115-
// broken request payload must not be answered with a canned success.
116108
const matching = (key ? this.cassette.rest?.[key] : undefined)?.filter((entry) =>
117109
isSubsetMatch(entry.bodySubset, body),
118110
);
@@ -148,13 +140,6 @@ export class MockWorld {
148140
this.unmatchedSubscriptions.push(description);
149141
}
150142

151-
/**
152-
* One driver per stream broadcasts each frame once to every current
153-
* subscriber: duplicate subscribers (e.g. flowUpdated is opened by both the
154-
* list and the detail provider) each get the frame, while a re-subscribe
155-
* after a reconnect joins past the cursor — delta-only, like the real
156-
* server. Returns an unsubscribe function.
157-
*/
158143
subscribeStream(streamKey: string, entry: SubscriptionCassetteEntry, subscriber: StreamSubscriber): () => void {
159144
const state = this.streams.get(streamKey) ?? { cursor: 0, isDriving: false, subscribers: new Set() };
160145

@@ -194,7 +179,6 @@ export class MockWorld {
194179
}
195180
}
196181

197-
/** Entries gated on an unraised flag are invisible; raised-flag entries outrank unflagged ones. */
198182
private eligible<T extends WorldFlagged>(candidates?: T[]): T[] | undefined {
199183
const open = candidates?.filter((entry) => !entry.whenFlag || this.flags.has(entry.whenFlag));
200184

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
3+
import type { SubscriptionCassetteEntry } from './cassette.ts';
4+
import type { StreamSubscriber } from './world.ts';
5+
6+
import { MockWorld } from './world.ts';
7+
8+
const collect = (into: unknown[]): StreamSubscriber => ({ complete: () => {}, next: (payload) => into.push(payload) });
9+
10+
describe('MockWorld matching', () => {
11+
it('matches variables regardless of property order inside arrays', () => {
12+
const world = new MockWorld({
13+
mutations: { save: [{ data: { ok: true }, variables: { items: [{ a: 1, b: 2 }] } }] },
14+
});
15+
16+
// Built key-by-key: a reversed object literal would be re-sorted by the linter,
17+
// hiding that stableStringify must normalise key order inside array values.
18+
const item: Record<string, number> = {};
19+
item.b = 2;
20+
item.a = 1;
21+
22+
expect(world.matchGraphQL('save', { items: [item] })?.data).toEqual({ ok: true });
23+
});
24+
25+
it('treats a pinned object as a subset — extra siblings match, a wrong pinned value does not', () => {
26+
const world = new MockWorld({
27+
mutations: { create: [{ data: { ok: true }, variables: { input: { name: 'x' } } }] },
28+
});
29+
30+
expect(world.matchGraphQL('create', { input: { name: 'x', ttl: 99 } })?.data).toEqual({ ok: true });
31+
expect(world.matchGraphQL('create', { input: { name: 'y', ttl: 99 } })).toBeUndefined();
32+
});
33+
34+
it('serves same-key entries in order and repeats the last', () => {
35+
const world = new MockWorld({
36+
queries: {
37+
flow: [
38+
{ data: { n: 1 }, variables: { id: '5' } },
39+
{ data: { n: 2 }, variables: { id: '5' } },
40+
],
41+
},
42+
});
43+
44+
expect(world.matchGraphQL('flow', { id: '5' })?.data).toEqual({ n: 1 });
45+
expect(world.matchGraphQL('flow', { id: '5' })?.data).toEqual({ n: 2 });
46+
expect(world.matchGraphQL('flow', { id: '5' })?.data).toEqual({ n: 2 });
47+
});
48+
49+
it('leaves a REST path hit with a mismatched body unmatched instead of answering success', () => {
50+
const world = new MockWorld({
51+
rest: { 'POST /api/v1/resources/mkdir': [{ body: { ok: true }, bodySubset: { path: 'new-folder' } }] },
52+
});
53+
54+
expect(world.matchRest('POST', '/api/v1/resources/mkdir', { path: 'new-folder' })).toBeDefined();
55+
expect(world.matchRest('POST', '/api/v1/resources/mkdir', { path: 'wrong' })).toBeUndefined();
56+
expect(world.matchRest('POST', '/api/v1/resources/mkdir/', { path: 'new-folder' })).toBeDefined();
57+
});
58+
59+
it('hides a flag-gated entry until the flag is raised, then lets it outrank the unflagged one', () => {
60+
const world = new MockWorld({
61+
mutations: { login: [{ data: { ok: true }, setFlag: 'logged-in' }] },
62+
queries: { info: [{ data: { role: 'guest' } }, { data: { role: 'user' }, whenFlag: 'logged-in' }] },
63+
});
64+
65+
expect(world.matchGraphQL('info', {})?.data).toEqual({ role: 'guest' });
66+
world.matchGraphQL('login', {});
67+
expect(world.matchGraphQL('info', {})?.data).toEqual({ role: 'user' });
68+
});
69+
});
70+
71+
describe('MockWorld streams', () => {
72+
it('broadcasts each frame once to every current subscriber; a later subscriber joins delta-only', async () => {
73+
const world = new MockWorld({ mutations: { trigger: [{ data: {}, setFlag: 'go' }] } });
74+
const entry: SubscriptionCassetteEntry = {
75+
frames: [{ payload: { data: { n: 1 } } }, { payload: { data: { n: 2 } }, whenFlag: 'go' }],
76+
};
77+
const first: unknown[] = [];
78+
const late: unknown[] = [];
79+
80+
world.subscribeStream('k', entry, collect(first));
81+
await vi.waitFor(() => expect(first).toHaveLength(1));
82+
83+
world.subscribeStream('k', entry, collect(late));
84+
world.matchGraphQL('trigger', {});
85+
await vi.waitFor(() => expect(first).toHaveLength(2));
86+
87+
expect(late).toEqual([{ data: { n: 2 } }]);
88+
});
89+
});

frontend/e2e/mocks/ws-graphql.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ interface SubscribePayload {
2020
variables?: Record<string, unknown>;
2121
}
2222

23-
/**
24-
* Transport-agnostic graphql-ws server half: Playwright routes and the vitest
25-
* protocol-contract suite (ws-graphql.unit.test.ts) both drive this exact
26-
* code, so the protocol invariants are pinned outside full browser runs.
27-
*/
2823
export const handleWsConnection = (
2924
world: MockWorld,
3025
transport: WsTransport,

frontend/e2e/playwright.config.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@ import type { BackendOptions, BackendTier } from './fixtures/backend.ts';
55

66
const rawTier = process.env.E2E_TIER ?? 'mock';
77
const isCI = Boolean(process.env.CI);
8-
// Visual snapshots only ever run inside the pinned Playwright Linux container
9-
// (e2e/tools/run-visual.sh) — a darwin run would generate parallel baselines
10-
// that never match CI pixels.
118
const isVisual = process.env.E2E_VISUAL === '1';
129

1310
// Baselines are linux-suffixed, so a host run writes `*-visual-darwin.png` beside
@@ -21,8 +18,6 @@ if (isVisual && process.platform !== 'linux') {
2118
// `vite preview` listens on VITE_PORT + 100 and reuses the dev proxy config.
2219
const PREVIEW_PORT = 8100;
2320

24-
// Reporter paths resolve against the process cwd, not the config file — pin them
25-
// so CI artifact uploads have one deterministic location.
2621
const here = (relative: string) => fileURLToPath(new URL(relative, import.meta.url));
2722

2823
export const AUTH_STATE_PATH = here('./.auth/user.json');
@@ -54,9 +49,6 @@ export default defineConfig<BackendOptions>({
5449
fullyParallel: true,
5550
globalTimeout: isCI ? 10 * 60_000 : undefined,
5651
outputDir: './test-results',
57-
// Cassette specs run only on the mock tier; specs/real/** run only against a
58-
// live backend, which authenticates once in the setup project and reuses
59-
// storageState.
6052
projects:
6153
tier === 'mock'
6254
? [
@@ -96,8 +88,6 @@ export default defineConfig<BackendOptions>({
9688
['list'],
9789
]
9890
: [['html', { open: 'never', outputFolder: here('./playwright-report') }], ['list']],
99-
// Retries and trace policy are intentionally identical local vs CI so a red CI
100-
// run reproduces byte-identically with `CI=1 pnpm e2e`.
10191
retries: 1,
10292
testDir: './specs',
10393
use: {

frontend/e2e/routes.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,9 @@ import { settingsProvidersCassette } from './mocks/cassettes/settings-providers.
1515
import { templatesCassette } from './mocks/cassettes/templates.ts';
1616

1717
export interface RouteManifestEntry {
18-
/** Known accessibility debt on this route, waived node-by-node by the axe sweep. */
1918
a11yWaivers?: A11yWaiver[];
2019
cassette: () => Cassette;
2120
path: string;
22-
/** The route counts as rendered when this locator is visible. */
2321
ready: (page: Page) => Locator;
2422
/**
2523
* Owning src/ areas — the substrate for changed-files → affected-routes
@@ -58,7 +56,6 @@ export const ROUTE_MANIFEST: RouteManifestEntry[] = [
5856
'src/features/flows',
5957
'src/providers/flow-provider.tsx',
6058
'src/providers/flows-provider.tsx',
61-
// Rendered inside the detail page's tabs alongside their owning routes.
6259
'src/components/shared/file-manager',
6360
'src/components/dashboard',
6461
'src/features/resources',

0 commit comments

Comments
 (0)