Skip to content

Commit 95f838f

Browse files
authored
test: stop pinning settle capture counts against a wall-clock loop (#1307)
* test: model the UI in settle-observation fixtures instead of a capture count settle-observation's "contention flakes" were a zero-margin comparison meeting a 1ms clock skew, not generic load. runStableCaptureLoop derives pollMs = min(300, max(25, quietMs)), so the test's settleQuietMs: 25 made pollMs === quietMs, and the settle check (one sleep(25) plus capture time, against >= 25) a 0ms margin. Node's setTimeout(25) advances Date.now() by only 24ms in 0.13% of calls idle and 0.63% under load, because libuv's timers and Date.now() read different clocks. On a 24 the loop takes a third capture that the transcript never scripted, and settle's best-effort catch reports the resulting throw as settled: false. The fixtures now model the surface rather than the runner's speed: a quiet UI serves the settled tree to every capture, a busy one a fresh tree per capture, via new transcript `repeat` entries and result factories. The snapshot-floor economy guard survives as a bound (2-3 captures), and the follow-up's "no fresh capture" cost — previously implied by the exact transcript — is now asserted directly. Production is untouched: the same zero margin only costs a wasted extra capture and poll there, filed separately as #1306. * test: stop pinning the settle capture count in the iOS contract scenario too A sweep for the same bug class found direct-ios-selector's settleObservation scenario carrying the identical 0ms margin: settleQuietMs: 25 (so pollMs === quietMs) against a consume-once transcript scripting exactly two settle captures, with no injected clock. It has not lost the coin flip in CI yet, but it fails the same way when it does — a third capture finds no entry and settle's best-effort catch reports settled: false. Same fix: the fixture models a quiet UI (every settle capture sees the same tree) instead of scripting how many captures fit in a wall-clock window. The rest of the sweep was clean. The other contract scenarios and the interaction runtime tests already use clamped mocks that repeat the last snapshot, so any capture count is tolerated; the fake-clock tests are correct to pin exact counts. * style: oxfmt quietRunnerSnapshotEntry signature * test: make one-shot-outranks-repeat a real transcript rule (P2 review) The review is right: `one-shot entries still outrank a repeat entry` asserted a guarantee the lookup did not provide. It passed only because the one-shot happened to be declared first — unordered lookup took the first match, so a repeat declared ahead of a matching one-shot shadowed it forever and left it permanently unconsumed. Both failure modes reproduce; the reverse-order test added here fails on the previous implementation. Unordered lookup now searches matching one-shots before repeats, so outranking holds whatever the declaration order. A repeat is documented as its command's fallback. Ordered transcripts now reject repeats at construction: ordered lookup only ever reads the head, so a repeat there never advances and strands every entry behind it. Refusing the combination beats failing later as a confusing "Provider command mismatch". Coverage added for both: reverse declaration order, and ordered + repeat.
1 parent 117f781 commit 95f838f

5 files changed

Lines changed: 207 additions & 27 deletions

File tree

test/integration/interaction-contract/daemon-harness.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ export function runnerSnapshotEntry(nodes: readonly unknown[]): ProviderScenario
6969
};
7070
}
7171

72+
// A UI that has gone quiet: every settle capture sees the same tree. How many
73+
// captures the loop spends reaching that verdict is wall-clock, so the count is
74+
// not the contract and is never scripted.
75+
export function quietRunnerSnapshotEntry(nodes: readonly unknown[]): ProviderScenarioProviderEntry {
76+
return { ...runnerSnapshotEntry(nodes), repeat: true };
77+
}
78+
7279
export function runnerTapEntry(
7380
result: Record<string, unknown>,
7481
request?: Record<string, unknown>,

test/integration/interaction-contract/direct-ios-selector.contract.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
RUNNER_NON_HITTABLE_NODES,
1414
} from './fixtures.ts';
1515
import {
16+
quietRunnerSnapshotEntry,
1617
runnerSnapshotEntry,
1718
runnerTapEntry,
1819
runnerTapErrorEntry,
@@ -188,11 +189,10 @@ test(scenario('settleObservation'), async () => {
188189
await withIosContractDaemon(
189190
[
190191
// --settle disables the direct path: runtime tree capture, coordinate
191-
// tap, then the settle loop's two stable captures of the changed tree.
192+
// tap, then the settle loop captures the changed tree until it goes quiet.
192193
runnerSnapshotEntry(RUNNER_CONTINUE_NODES),
193194
runnerTapEntry({ x: 200, y: 322 }),
194-
runnerSnapshotEntry(RUNNER_CHANGED_NODES),
195-
runnerSnapshotEntry(RUNNER_CHANGED_NODES),
195+
quietRunnerSnapshotEntry(RUNNER_CHANGED_NODES),
196196
],
197197
async (daemon, transcript) => {
198198
const click = await daemon.callCommand('click', ['label=Continue'], {

test/integration/provider-scenarios/settle-observation.test.ts

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,30 @@ function snapshotEntry(nodes: readonly unknown[]): ProviderScenarioProviderEntry
130130
};
131131
}
132132

133+
// A UI that has gone quiet: every settle capture sees the same tree. How many
134+
// captures the loop spends reaching that verdict is wall-clock — it polls on a
135+
// 25ms floor and settles once two identical captures span the quiet window — so
136+
// the count is not the contract and is never scripted here.
137+
function quietSnapshotEntry(nodes: readonly unknown[]): ProviderScenarioProviderEntry {
138+
return { ...snapshotEntry(nodes), repeat: true };
139+
}
140+
141+
// A UI that never goes quiet: every capture returns a fresh tree, so the loop
142+
// can never settle no matter how many captures fit in the budget.
143+
function changingSnapshotEntry(nodes: (label: string) => unknown[]): ProviderScenarioProviderEntry {
144+
let capture = 0;
145+
return {
146+
command: 'ios.runner.snapshot',
147+
deviceId: DEVICE_ID,
148+
platform: 'apple',
149+
repeat: true,
150+
result: () => {
151+
capture += 1;
152+
return { nodes: nodes(`Loading ${capture}`), truncated: false };
153+
},
154+
};
155+
}
156+
133157
function typeEntry(x: number, y: number): ProviderScenarioProviderEntry {
134158
return {
135159
command: 'ios.runner.type',
@@ -155,8 +179,7 @@ test('Provider-backed integration press --settle returns the settled diff and fr
155179
// press label=Continue --settle: resolution capture, tap, settle captures
156180
snapshotEntry(BEFORE_NODES),
157181
tapEntry(200, 322),
158-
snapshotEntry(SETTLED_NODES),
159-
snapshotEntry(SETTLED_NODES),
182+
quietSnapshotEntry(SETTLED_NODES),
160183
// press @e2 (the Done ref from the settled diff): tap on the stored tree
161184
tapEntry(200, 522),
162185
]);
@@ -212,7 +235,14 @@ test('Provider-backed integration press --settle returns the settled diff and fr
212235
};
213236
assert.ok(settle, 'press --settle must return a settle observation');
214237
assert.equal(settle.settled, true);
215-
assert.equal(settle.captures, 2);
238+
// Snapshot-floor economy: a quiet UI settles on the two identical
239+
// captures the verdict needs, or three when the second lands a hair short
240+
// of the quiet window. Anything above that is a regression in what settle
241+
// spends; pinning a single number would assert the runner's speed.
242+
assert.ok(
243+
settle.captures >= 2 && settle.captures <= 3,
244+
`settle should cost 2-3 captures, got ${settle.captures}`,
245+
);
216246
assert.equal(typeof settle.refsGeneration, 'number');
217247
assert.deepEqual(settle.diff?.summary, { additions: 1, removals: 2, unchanged: 1 });
218248
const added = settle.diff?.lines.find((line) => line.kind === 'added');
@@ -229,11 +259,19 @@ test('Provider-backed integration press --settle returns the settled diff and fr
229259
// directly on the stored settled tree with no fresh snapshot round trip
230260
// and no stale-refs warning. The plain `@e2` would require a complete
231261
// frame.
262+
const callsBeforeFollowUp = runnerTranscript.calls.length;
232263
const followUp = await daemon.callCommand('press', [`@e2~s${settle.refsGeneration}`], {});
233264
const followUpData = assertRpcOk(followUp);
234265
assert.equal(followUpData.warning, undefined);
235266
assert.equal(followUpData.x, 200);
236267
assert.equal(followUpData.y, 522);
268+
// The round trip settle exists to remove: the follow-up spends a tap and
269+
// nothing else. Asserted directly, since a quiet-UI entry would happily
270+
// serve a stray capture.
271+
assert.deepEqual(
272+
runnerTranscript.calls.slice(callsBeforeFollowUp).map((call) => call.command),
273+
['ios.runner.tap'],
274+
);
237275

238276
runnerTranscript.assertComplete();
239277
},
@@ -258,13 +296,11 @@ test('Provider-backed integration never-settled press --settle does not issue di
258296
},
259297
];
260298
const runnerTranscript = createProviderTranscript([
261-
// press label=Continue --settle: resolution capture, tap, then a changing
262-
// settle stream. The loop times out; the final capture is not actionable.
299+
// press label=Continue --settle: resolution capture, tap, then a settle
300+
// stream that never repeats itself, so the loop can only ever time out.
263301
snapshotEntry(BEFORE_NODES),
264302
tapEntry(200, 322),
265-
snapshotEntry(loadingNodes('Loading 1')),
266-
snapshotEntry(loadingNodes('Loading 2')),
267-
snapshotEntry(SETTLED_NODES),
303+
changingSnapshotEntry(loadingNodes),
268304
]);
269305
const appleRunnerProvider = createAppleRunnerProviderFromTranscript(
270306
runnerTranscript,
@@ -293,7 +329,10 @@ test('Provider-backed integration never-settled press --settle does not issue di
293329
const press = await daemon.callCommand('press', ['label=Continue'], {
294330
settle: true,
295331
settleQuietMs: 25,
296-
timeoutMs: 60,
332+
// Budget the loop spends without settling. Wide enough that a loaded
333+
// runner's capture cannot outlast it — a capture that overruns the
334+
// budget reports the stalled hint instead of this one.
335+
timeoutMs: 300,
297336
});
298337
const pressData = assertRpcOk(press);
299338
const settle = pressData.settle as {
@@ -324,8 +363,7 @@ test('Provider-backed integration modal-dismiss press --settle attaches the unch
324363
// no added refs, so the tail is the only actionable-target payload.
325364
snapshotEntry(MODAL_BEFORE_NODES),
326365
tapEntry(200, 422),
327-
snapshotEntry(MODAL_DISMISSED_NODES),
328-
snapshotEntry(MODAL_DISMISSED_NODES),
366+
quietSnapshotEntry(MODAL_DISMISSED_NODES),
329367
// press @e3 (the Continue ref from the tail): tap on the stored tree.
330368
tapEntry(200, 322),
331369
]);
@@ -392,11 +430,16 @@ test('Provider-backed integration modal-dismiss press --settle attaches the unch
392430
// The tail's ref acts directly on the stored settled tree, same as an
393431
// added-line ref would — consumed in pinned form from the partial frame
394432
// (ADR 0014).
433+
const callsBeforeFollowUp = runnerTranscript.calls.length;
395434
const followUp = await daemon.callCommand('press', [`@e3~s${settle.refsGeneration}`], {});
396435
const followUpData = assertRpcOk(followUp);
397436
assert.equal(followUpData.warning, undefined);
398437
assert.equal(followUpData.x, 200);
399438
assert.equal(followUpData.y, 322);
439+
assert.deepEqual(
440+
runnerTranscript.calls.slice(callsBeforeFollowUp).map((call) => call.command),
441+
['ios.runner.tap'],
442+
);
400443

401444
runnerTranscript.assertComplete();
402445
},
@@ -651,8 +694,7 @@ test('Provider-backed integration fill --settle summoning the keyboard still att
651694
// fresh capture), the runner types, then the settle loop captures. The
652695
// keyboard window appears and the field re-labels itself.
653696
typeEntry(201, 149),
654-
snapshotEntry(FILL_SETTLED_NODES),
655-
snapshotEntry(FILL_SETTLED_NODES),
697+
quietSnapshotEntry(FILL_SETTLED_NODES),
656698
// press @e17 (the "Discard and go back" ref from the tail): tap on the
657699
// stored settled tree.
658700
tapEntry(201, 202),
@@ -724,11 +766,16 @@ test('Provider-backed integration fill --settle summoning the keyboard still att
724766
assert.equal(settle.tailTruncated, undefined);
725767
assert.equal(typeof settle.refsGeneration, 'number');
726768

769+
const callsBeforeFollowUp = runnerTranscript.calls.length;
727770
const followUp = await daemon.callCommand('press', [`@e17~s${settle.refsGeneration}`], {});
728771
const followUpData = assertRpcOk(followUp);
729772
assert.equal(followUpData.warning, undefined);
730773
assert.equal(followUpData.x, 201);
731774
assert.equal(followUpData.y, 202);
775+
assert.deepEqual(
776+
runnerTranscript.calls.slice(callsBeforeFollowUp).map((call) => call.command),
777+
['ios.runner.tap'],
778+
);
732779

733780
runnerTranscript.assertComplete();
734781
},

test/integration/provider-scenarios/transcript.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,93 @@ test('provider transcript matches expected calls without requiring incidental or
1515
transcript.assertComplete();
1616
});
1717

18+
test('repeat entries serve every matching call and never count as unconsumed', () => {
19+
const transcript = createProviderTranscript([
20+
{ command: 'ios.runner.snapshot', repeat: true, result: { ok: 'snapshot' } },
21+
{ command: 'ios.runner.tap', result: { ok: 'tap' } },
22+
]);
23+
24+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'snapshot' });
25+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'snapshot' });
26+
assert.deepEqual(transcript.next('ios.runner.tap'), { ok: 'tap' });
27+
28+
// The repeat entry stays pending forever; only the one-shot tap had to land.
29+
transcript.assertComplete();
30+
assert.equal(transcript.calls.length, 3);
31+
});
32+
33+
test('one-shot entries still outrank a repeat entry and remain required', () => {
34+
const transcript = createProviderTranscript([
35+
{ command: 'ios.runner.snapshot', result: { ok: 'first' } },
36+
{ command: 'ios.runner.snapshot', repeat: true, result: { ok: 'rest' } },
37+
]);
38+
39+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'first' });
40+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'rest' });
41+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'rest' });
42+
transcript.assertComplete();
43+
});
44+
45+
test('a repeat declared before a matching one-shot does not shadow it', () => {
46+
// Outranking is a rule, not an accident of declaration order: taking the
47+
// first match would serve the repeat forever and strand the one-shot.
48+
const transcript = createProviderTranscript([
49+
{ command: 'ios.runner.snapshot', repeat: true, result: { ok: 'rest' } },
50+
{ command: 'ios.runner.snapshot', result: { ok: 'first' } },
51+
]);
52+
53+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'first' });
54+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'rest' });
55+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { ok: 'rest' });
56+
transcript.assertComplete();
57+
});
58+
59+
test('ordered transcripts reject repeat entries outright', () => {
60+
// Ordered lookup only ever reads entry 0, so a repeat there never advances
61+
// and makes every later entry unreachable. Refuse the combination instead of
62+
// failing later as a confusing command mismatch.
63+
assert.throws(
64+
() =>
65+
createOrderedProviderTranscript([
66+
{ command: 'ios.runner.snapshot', repeat: true, result: { ok: 'snapshot' } },
67+
{ command: 'ios.runner.tap', result: { ok: 'tap' } },
68+
]),
69+
/repeat/i,
70+
);
71+
});
72+
73+
test('unconsumed one-shot entries still fail assertComplete alongside a repeat entry', () => {
74+
const transcript = createProviderTranscript([
75+
{ command: 'ios.runner.snapshot', repeat: true, result: { ok: 'snapshot' } },
76+
{ command: 'ios.runner.tap', result: { ok: 'tap' } },
77+
]);
78+
79+
transcript.next('ios.runner.snapshot');
80+
assert.throws(() => transcript.assertComplete(), /Unconsumed provider transcript entries.*tap/s);
81+
});
82+
83+
test('a result factory is invoked per call, so repeated calls can return fresh results', () => {
84+
let capture = 0;
85+
const transcript = createProviderTranscript([
86+
{
87+
command: 'ios.runner.snapshot',
88+
repeat: true,
89+
result: () => {
90+
capture += 1;
91+
return { label: `Loading ${capture}` };
92+
},
93+
},
94+
]);
95+
96+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { label: 'Loading 1' });
97+
assert.deepEqual(transcript.next('ios.runner.snapshot'), { label: 'Loading 2' });
98+
// The recorded call carries the resolved result, not the factory.
99+
assert.deepEqual(
100+
transcript.calls.map((call) => call.result),
101+
[{ label: 'Loading 1' }, { label: 'Loading 2' }],
102+
);
103+
});
104+
18105
test('ordered provider transcript remains available when ordering is the contract', () => {
19106
const transcript = createOrderedProviderTranscript([
20107
{ command: 'ios.runner.snapshot', result: { ok: 'snapshot' } },

0 commit comments

Comments
 (0)