Skip to content

Commit c37aa5f

Browse files
committed
fix: make layer-3 scenarios prove what they claim, and parse the Maestro version
Run 3 (29497919702) got the whole device path working: Expo build (30m), app installed, simctl check, pinned Maestro CLI install. Only the version ASSERTION failed — `maestro --version` prints an analytics banner before the version, and `tr -d '[:space:]'` mashed banner+version into one string. The CLI was correctly 2.5.1. Match the semver line instead, and set MAESTRO_CLI_NO_ANALYTICS (CI should not phone home). Verified the parse against the exact CI output: banner and clean forms both yield 2.5.1, wrong/empty still fail. tap-retry-if-no-change was vacuous: it tapped a navigating control, so the first tap always succeeded and retryIfNoChange never ran — it passed while proving nothing. It now taps the app's non-interactive title so the screen cannot change and the retry path is forced, and asserts tapRetries >= 1 from the trace (MaestroRuntimeMetrics already records it per step). A new metricAtLeast invariant kind carries the assertion; a test reproduces the old vacuity. percent-swipe no longer claims bug class 1. Truncation vs rounding is a <=1px delta that no app-observable device outcome can distinguish, so pass/pass could never back that claim up. The runtime half is instead pinned exactly by a pure unit test of resolveMaestroCoordinate (it short-circuits on a known viewport, so no device is needed) — verified to catch the regression by flipping trunc->round, which turns 3 of 6 tests red. Truncation had no test coverage at all before this. A test now forbids any device scenario from re-claiming bug class 1.
1 parent 84cb81a commit c37aa5f

6 files changed

Lines changed: 222 additions & 26 deletions

File tree

.github/workflows/conformance-differential.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ concurrency:
4343
env:
4444
AGENT_DEVICE_CLI: "--experimental-strip-types src/bin.ts"
4545
DIFFERENTIAL_ONLY: ${{ github.event.inputs.only || '' }}
46+
# CI should not phone home, and it keeps `maestro --version` to just the
47+
# version instead of prefixing an analytics notice.
48+
MAESTRO_CLI_NO_ANALYTICS: "1"
4649

4750
jobs:
4851
differential-ios:
@@ -115,7 +118,10 @@ jobs:
115118
run: |
116119
set -euo pipefail
117120
EXPECTED=$(node -p "require('./scripts/maestro-conformance/pinned-upstream.json').version")
118-
ACTUAL=$(maestro --version 2>/dev/null | tr -d '[:space:]')
121+
# `maestro --version` can print notices (e.g. the analytics banner)
122+
# around the version, so match the semver line rather than the whole
123+
# output. Empty ACTUAL fails the comparison below, as it should.
124+
ACTUAL=$(maestro --version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | tail -1 || true)
119125
echo "expected=$EXPECTED actual=$ACTUAL"
120126
if [ "$ACTUAL" != "$EXPECTED" ]; then
121127
echo "::error::Layer-3 Maestro CLI is $ACTUAL but the oracle pins $EXPECTED. A differential against a different Maestro proves nothing about the pinned behavior."
Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
1-
# Layer-3 device flow. A tap whose screen may not change exercises the
2-
# retryIfNoChange path on both engines.
1+
# Layer-3 device flow. Taps the app's static title, which is NOT interactive, so
2+
# the screen cannot change and the retryIfNoChange path is forced to run. A tap on
3+
# a navigating control (e.g. home-open-form) succeeds on the first attempt and
4+
# never exercises retry at all — the scenario would pass while proving nothing.
5+
# The scenario asserts tapRetries >= 1 from the trace, so a retry regression is
6+
# caught rather than assumed.
37
appId: com.callstack.agentdevicelab
48
---
59
- launchApp:
610
clearState: true
711
- assertVisible: Agent Device Tester
812
- tapOn:
9-
id: home-open-form
13+
text: Agent Device Tester
1014
retryTapIfNoChange: true
11-
- assertVisible: Checkout form
15+
- assertVisible: Agent Device Tester

scripts/maestro-conformance/differential/invariants.test.ts

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,66 @@ test('readTrace on a missing file returns no events', () => {
7878

7979
test('bug class 4 has a machine-checkable invariant, not just outcome parity', () => {
8080
const settle = DIFFERENTIAL_SCENARIOS.find((scenario) => scenario.bugClass === 4);
81-
assert.ok(settle?.engineInvariants?.length, 'settle scenario must carry an engine-side invariant');
82-
assert.equal(settle?.engineInvariants?.[0]?.maxMs, MAESTRO_DEFAULT_SETTLE_TIMEOUT_MS);
81+
const invariant = settle?.engineInvariants?.[0];
82+
assert.ok(invariant, 'settle scenario must carry an engine-side invariant');
83+
assert.equal(invariant?.kind, 'stepDurationBelow');
84+
assert.equal(
85+
invariant?.kind === 'stepDurationBelow' ? invariant.maxMs : undefined,
86+
MAESTRO_DEFAULT_SETTLE_TIMEOUT_MS,
87+
);
88+
});
89+
90+
// --- metricAtLeast: proves a code path actually ran, not just that it passed ---
91+
92+
const RETRY_INVARIANT: Invariant = {
93+
kind: 'metricAtLeast',
94+
command: 'tapOn',
95+
metric: 'tapRetries',
96+
min: 1,
97+
because: 'test',
98+
};
99+
100+
const stopWithMetrics = (command: string, metrics: Record<string, number>, step = 1) => ({
101+
type: 'replay_action_stop',
102+
step,
103+
command,
104+
ok: true,
105+
durationMs: 100,
106+
resultTiming: metrics,
107+
});
108+
109+
test('a tap that actually retried holds the retry invariant', () => {
110+
const result = evaluateInvariant([stopWithMetrics('tapOn', { tapRetries: 1 })], RETRY_INVARIANT);
111+
assert.equal(result.status, 'held');
112+
});
113+
114+
// The exact vacuity that made this scenario worthless before: the tap succeeded
115+
// first try (navigating control), so retry never ran — yet the flow passed.
116+
test('a tap that never retried violates the invariant (catches a vacuous scenario)', () => {
117+
const result = evaluateInvariant([stopWithMetrics('tapOn', { tapRetries: 0 })], RETRY_INVARIANT);
118+
assert.equal(result.status, 'violated');
119+
assert.match(result.detail, /was 0/);
120+
});
121+
122+
test('a trace whose taps record no metric reports no-data rather than passing', () => {
123+
const result = evaluateInvariant([stopWithMetrics('tapOn', {})], RETRY_INVARIANT);
124+
assert.equal(result.status, 'no-data');
125+
});
126+
127+
test('the retry scenario asserts the retry path ran', () => {
128+
const retry = DIFFERENTIAL_SCENARIOS.find((s) => s.id === 'tap-retry-if-no-change');
129+
const invariant = retry?.engineInvariants?.[0];
130+
assert.ok(invariant, 'retry scenario must prove a retry happened, not assume it');
131+
assert.equal(invariant?.kind, 'metricAtLeast');
132+
});
133+
134+
// Truncation-vs-rounding is at most 1px and cannot be observed on a device, so
135+
// no scenario may claim to guard bug class 1; a unit test pins it instead.
136+
test('no device scenario claims to prove percent truncation (bug class 1)', () => {
137+
const claiming = DIFFERENTIAL_SCENARIOS.filter((s) => s.bugClass === 1);
138+
assert.deepEqual(
139+
claiming.map((s) => s.id),
140+
[],
141+
'a 1px truncation delta is not app-observable; keep bug class 1 in runtime-port-geometry.test.ts',
142+
);
83143
});

scripts/maestro-conformance/differential/invariants.ts

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,27 @@ export type TraceEvent = {
2121
command?: string;
2222
ok?: boolean;
2323
durationMs?: number;
24+
/** Per-step MaestroRuntimeMetrics delta (hierarchyCaptures/screenshotCaptures/tapRetries). */
25+
resultTiming?: Record<string, unknown>;
2426
};
2527

26-
export type Invariant = {
27-
kind: 'stepDurationBelow';
28-
/** Maestro command name the step must match (e.g. "tapOn"). */
29-
command: string;
30-
maxMs: number;
31-
/** Why this bound means the engine behaved correctly. */
32-
because: string;
33-
};
28+
export type Invariant =
29+
| {
30+
kind: 'stepDurationBelow';
31+
/** Maestro command name the step must match (e.g. "tapOn"). */
32+
command: string;
33+
maxMs: number;
34+
/** Why this bound means the engine behaved correctly. */
35+
because: string;
36+
}
37+
| {
38+
kind: 'metricAtLeast';
39+
command: string;
40+
/** MaestroRuntimeMetrics key, recorded per step as a delta. */
41+
metric: 'tapRetries' | 'hierarchyCaptures' | 'screenshotCaptures';
42+
min: number;
43+
because: string;
44+
};
3445

3546
export type InvariantResult = {
3647
invariant: Invariant;
@@ -56,12 +67,7 @@ export function readTrace(file: string): TraceEvent[] {
5667
}
5768

5869
function completedSteps(events: TraceEvent[], command: string): TraceEvent[] {
59-
return events.filter(
60-
(event) =>
61-
event.type === 'replay_action_stop' &&
62-
event.command === command &&
63-
typeof event.durationMs === 'number',
64-
);
70+
return events.filter((event) => event.type === 'replay_action_stop' && event.command === command);
6571
}
6672

6773
export function evaluateInvariant(events: TraceEvent[], invariant: Invariant): InvariantResult {
@@ -73,7 +79,43 @@ export function evaluateInvariant(events: TraceEvent[], invariant: Invariant): I
7379
detail: `no completed ${invariant.command} steps in the trace`,
7480
};
7581
}
76-
const worst = Math.max(...steps.map((step) => step.durationMs ?? 0));
82+
83+
if (invariant.kind === 'metricAtLeast') {
84+
const values = steps
85+
.map((step) => step.resultTiming?.[invariant.metric])
86+
.filter((value): value is number => typeof value === 'number');
87+
if (values.length === 0) {
88+
return {
89+
invariant,
90+
status: 'no-data',
91+
detail: `no ${invariant.command} step recorded a ${invariant.metric} metric`,
92+
};
93+
}
94+
// Per-step deltas: the strongest single step is what proves the path ran.
95+
const best = Math.max(...values);
96+
if (best < invariant.min) {
97+
return {
98+
invariant,
99+
status: 'violated',
100+
detail: `highest ${invariant.command} ${invariant.metric} was ${best} (< ${invariant.min}): ${invariant.because}`,
101+
};
102+
}
103+
return {
104+
invariant,
105+
status: 'held',
106+
detail: `${invariant.command} ${invariant.metric} reached ${best} (>= ${invariant.min})`,
107+
};
108+
}
109+
110+
const timed = steps.filter((step) => typeof step.durationMs === 'number');
111+
if (timed.length === 0) {
112+
return {
113+
invariant,
114+
status: 'no-data',
115+
detail: `no ${invariant.command} step recorded a duration`,
116+
};
117+
}
118+
const worst = Math.max(...timed.map((step) => step.durationMs ?? 0));
77119
if (worst >= invariant.maxMs) {
78120
return {
79121
invariant,

scripts/maestro-conformance/differential/scenarios.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,39 @@ export const DIFFERENTIAL_SCENARIOS: DifferentialScenario[] = [
6363
'agent-device settled in a different order than upstream (sleep-before vs sleep-after capture) or never latches within the shared budget.',
6464
},
6565
{
66-
id: 'percent-truncation',
67-
bugClass: 1,
66+
id: 'percent-swipe',
67+
// Deliberately NOT tagged bugClass 1. Truncation-vs-rounding is at most a
68+
// one-pixel difference: no app-observable outcome on a real device can
69+
// distinguish it, so claiming this scenario guards bug class 1 would be a
70+
// lie the pass/pass result cannot back up. That half is pinned exactly by a
71+
// pure unit test of the conversion
72+
// (src/compat/maestro/__tests__/runtime-port-geometry.test.ts), and the
73+
// parse-level half by layer 1 (bug-classes/percent-decimal-swipe). What this
74+
// scenario adds is only that percentage swipes work end-to-end on both.
6875
flow: 'differential/flows/percent-swipe.yaml',
6976
comparesAcrossEngines:
70-
'The percentage swipe completes on both engines. NOTE: this does not compare the resolved pixel coordinatestruncation-vs-rounding is a 1px difference that outcome parity cannot see. Parse-level decimal rejection is covered by layer 1 (bug-classes/percent-decimal-swipe).',
77+
'A percentage-endpoint swipe completes on both engines. It does NOT compare resolved pixel endpointsno metric records them and a 1px delta is not app-observable.',
7178
expect: 'pass',
7279
divergenceMeans: 'The swipe behaves differently enough on one engine to fail the flow.',
7380
},
7481
{
7582
id: 'tap-retry-if-no-change',
7683
flow: 'differential/flows/tap-retry-if-no-change.yaml',
7784
comparesAcrossEngines:
78-
'A retryTapIfNoChange tap succeeds on both engines. NOTE: the retap COUNT is not compared — the trace does not currently expose per-tap attempt counts.',
85+
'A retryTapIfNoChange tap succeeds on both engines. The flow taps a non-interactive title so the screen cannot change and the retry path is forced; the invariant below then proves a retry actually happened.',
7986
expect: 'pass',
87+
// Outcome parity would pass even if retry never ran. tapRetries is recorded
88+
// per step as a delta (replay-plan-execution.ts), so assert the path executed.
89+
engineInvariants: [
90+
{
91+
kind: 'metricAtLeast',
92+
command: 'tapOn',
93+
metric: 'tapRetries',
94+
min: 1,
95+
because:
96+
'a tap on an unchanging screen must re-tap; zero retries means retryIfNoChange never ran and the scenario proved nothing',
97+
},
98+
],
8099
divergenceMeans: 'agent-device fails a tap upstream completes (or vice versa) under retry-if-no-change.',
81100
},
82101
{
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import assert from 'node:assert/strict';
2+
import { describe, test } from 'vitest';
3+
import { resolveMaestroCoordinate } from '../runtime-port-geometry.ts';
4+
5+
// Bug class 1 (#1217), runtime half: upstream Maestro converts percentage
6+
// coordinates to pixels by integer division (Maestro.kt), so the result is
7+
// TRUNCATED, never rounded. The parse-level half (rejecting decimal percentages)
8+
// is covered by the conformance oracle's layer-1 corpus.
9+
//
10+
// This is deliberately a unit test rather than a layer-3 device scenario:
11+
// truncation and rounding differ by at most one pixel, which no app-observable
12+
// outcome on a real device can distinguish. A pure test of the conversion pins it
13+
// exactly. `resolveMaestroCoordinate` short-circuits on a known viewport, so no
14+
// port or device is involved.
15+
describe('resolveMaestroCoordinate percentage conversion', () => {
16+
const viewport = (width: number, height: number, x = 0, y = 0) => ({ x, y, width, height });
17+
const resolve = (percent: { x: number; y: number }, vp: ReturnType<typeof viewport>) =>
18+
resolveMaestroCoordinate(
19+
{ space: 'percent', x: percent.x, y: percent.y },
20+
undefined as never,
21+
undefined as never,
22+
vp,
23+
);
24+
25+
test('truncates rather than rounds when the pixel is fractional', async () => {
26+
// 1125 * 50 / 100 = 562.5 -> trunc 562 (rounding would give 563)
27+
// 2436 * 33 / 100 = 803.88 -> trunc 803 (rounding would give 804)
28+
const point = await resolve({ x: 50, y: 33 }, viewport(1125, 2436));
29+
assert.deepEqual(point, { x: 562, y: 803 });
30+
});
31+
32+
test('truncates fractions above .5, where rounding would go up', async () => {
33+
// Both fractions round UP, so these only pass under truncation:
34+
// 1179 * 5 / 100 = 58.95 -> trunc 58 (round -> 59)
35+
// 2556 * 35 / 100 = 894.6 -> trunc 894 (round -> 895)
36+
const point = await resolve({ x: 5, y: 35 }, viewport(1179, 2556));
37+
assert.deepEqual(point, { x: 58, y: 894 });
38+
});
39+
40+
test('is exact when the pixel is whole', async () => {
41+
const point = await resolve({ x: 50, y: 25 }, viewport(1080, 1920));
42+
assert.deepEqual(point, { x: 540, y: 480 });
43+
});
44+
45+
test('adds the viewport origin after truncating the fraction', async () => {
46+
// Origin is added to the truncated span, not truncated together with it.
47+
const point = await resolve({ x: 50, y: 50 }, viewport(1125, 2436, 7, 11));
48+
assert.deepEqual(point, { x: 7 + 562, y: 11 + 1218 });
49+
});
50+
51+
test('0% and 100% map to the viewport edges', async () => {
52+
assert.deepEqual(await resolve({ x: 0, y: 0 }, viewport(1125, 2436)), { x: 0, y: 0 });
53+
assert.deepEqual(await resolve({ x: 100, y: 100 }, viewport(1125, 2436)), { x: 1125, y: 2436 });
54+
});
55+
56+
test('absolute coordinates pass through untouched', async () => {
57+
const point = await resolveMaestroCoordinate(
58+
{ space: 'absolute', x: 100, y: 200 },
59+
undefined as never,
60+
undefined as never,
61+
viewport(1125, 2436),
62+
);
63+
assert.deepEqual(point, { x: 100, y: 200 });
64+
});
65+
});

0 commit comments

Comments
 (0)