diff --git a/examples/test-app/.gitignore b/examples/test-app/.gitignore index db533b080..3d560e411 100644 --- a/examples/test-app/.gitignore +++ b/examples/test-app/.gitignore @@ -1,2 +1,8 @@ .expo/ node_modules/ + +# expo prebuild output, regenerated by `expo run:ios` / `expo run:android` +# (what .github/actions/setup-fixture-app does). Untracked and generated, so +# leaving them visible lets a `git commit -a` sweep the whole native project in. +android/ +ios/ diff --git a/examples/test-app/app/(tabs)/settings.tsx b/examples/test-app/app/(tabs)/settings.tsx index b87fa2528..5d2112ac0 100644 --- a/examples/test-app/app/(tabs)/settings.tsx +++ b/examples/test-app/app/(tabs)/settings.tsx @@ -16,6 +16,7 @@ export default function SettingsRoute() { diagnosticsState={state.diagnosticsState} notificationsEnabled={state.notificationsEnabled} onOpenAccessorySetup={() => router.push('/accessory-setup')} + onOpenInertSurface={() => router.push('/inert')} onConfirmReset={state.resetLabState} onLoadDiagnostics={state.loadDiagnostics} onRetryDiagnostics={state.retryDiagnostics} diff --git a/examples/test-app/app/_layout.tsx b/examples/test-app/app/_layout.tsx index 3dd77b739..7f6b88f7c 100644 --- a/examples/test-app/app/_layout.tsx +++ b/examples/test-app/app/_layout.tsx @@ -24,6 +24,7 @@ function RootLayoutContent() { > + {toastMessage ? : null} diff --git a/examples/test-app/app/inert.tsx b/examples/test-app/app/inert.tsx new file mode 100644 index 000000000..fd5890b77 --- /dev/null +++ b/examples/test-app/app/inert.tsx @@ -0,0 +1,14 @@ +import { AppFrame } from '../src/components'; +import { InertScreen } from '../src/screens/InertScreen'; + +// Presented as a fullScreenModal (see app/_layout.tsx) rather than pushed: iOS +// detaches the presenting screen once a full-screen present settles, so Settings +// and the tab bar — which carries live badges — leave the hierarchy entirely. +// The retry signature hashes every node on screen, so that matters. +export default function InertRoute() { + return ( + + + + ); +} diff --git a/examples/test-app/src/screens/InertScreen.tsx b/examples/test-app/src/screens/InertScreen.tsx new file mode 100644 index 000000000..1103d6ecb --- /dev/null +++ b/examples/test-app/src/screens/InertScreen.tsx @@ -0,0 +1,67 @@ +import { StyleSheet, Text, View } from 'react-native'; + +import { useAppColors, type AppColors } from '../theme'; + +/** + * A surface on which a tap provably changes nothing. + * + * `retryTapIfNoChange` only re-taps when the post-tap snapshot signature equals + * the pre-tap one, and that signature hashes EVERY node on screen — not just the + * tapped subtree. So any live content anywhere on the screen suppresses the + * retry, which is what made the layer-3 tap-retry scenario a coin flip on the + * home screen (its gesture lab loads a remote image whose arrival lands on either + * side of the tap). See https://github.com/callstack/agent-device/issues/1300. + * + * Everything here is therefore deliberately constrained, and each rule is load- + * bearing for that scenario rather than stylistic: + * + * - no state, effects, timers, or async work — nothing to arrive after capture + * - no images (a remote one loads whenever the network says so) + * - no ScrollView — scroll offset moves node bounds, which are part of the hash + * - nothing pressable, so a tap cannot mutate the screen even by accident + * + * Adding any of those back re-opens #1300. Put dynamic fixtures on another + * screen; this one exists to hold still. + */ +export function InertScreen() { + const colors = useAppColors(); + const styles = createStyles(colors); + + return ( + + + Inert surface + + + Tapping this text changes nothing on screen. + + + Nothing here reacts to touch, loads, or animates, so a tap leaves the + hierarchy byte-identical and retryTapIfNoChange must re-tap. + + + ); +} + +function createStyles(colors: AppColors) { + return StyleSheet.create({ + body: { + color: colors.text, + fontSize: 17, + fontWeight: '600', + }, + content: { + gap: 16, + }, + footnote: { + color: colors.textSoft, + fontSize: 14, + lineHeight: 21, + }, + title: { + color: colors.text, + fontSize: 24, + fontWeight: '700', + }, + }); +} diff --git a/examples/test-app/src/screens/SettingsScreen.tsx b/examples/test-app/src/screens/SettingsScreen.tsx index 6998302fb..f9d4ba10e 100644 --- a/examples/test-app/src/screens/SettingsScreen.tsx +++ b/examples/test-app/src/screens/SettingsScreen.tsx @@ -18,6 +18,7 @@ export interface SettingsScreenProps { notificationsEnabled: boolean; reducedMotionEnabled: boolean; onOpenAccessorySetup: () => void; + onOpenInertSurface: () => void; onLoadDiagnostics: () => void; onRetryDiagnostics: () => void; onSetNotificationsEnabled: (value: boolean) => void; @@ -71,6 +72,17 @@ export function SettingsScreen(props: SettingsScreenProps) { /> + + + + = 1 from the trace, so a retry regression is -# caught rather than assumed. +# Layer-3 device flow for retryTapIfNoChange. +# +# The engine re-taps only when the post-tap snapshot signature equals the pre-tap +# one, and that signature hashes EVERY node on screen — not just the tapped +# subtree. So a non-interactive target is not enough on its own: any live content +# anywhere on screen makes the engine see "changed" and skip the retry. Tapping +# the home screen's title used to look sufficient and was actually a coin flip +# (#1300) — its gesture lab loads a remote image whose arrival lands on either +# side of the tap depending on the network. +# +# So this drives the fixture's inert surface (examples/test-app/app/inert.tsx), +# a screen with nothing that loads, animates, or reacts to touch, presented as a +# full-screen modal so the tab bar's live badges leave the hierarchy too. On a +# real run the inert screen is 9 nodes against Settings' 55. +# +# Reaching it via the Settings TAB is deliberate: home's "Open settings" button +# sits below the fold, and scrolling to it needs scrollUntilVisible — the engine +# bug tracked in #1299. The tab is always on screen. +# +# There is deliberately no waitForAnimationToEnd before the tap. It is not needed +# — a navigating tap defers a stability requirement that the next tap settles +# before resolving its target, so the baseline signature is already captured on a +# settled screen — and adding one currently FAILS the flow outright (#1326). appId: com.callstack.agentdevicelab --- - launchApp: clearState: true -- assertVisible: Agent Device Tester - tapOn: - text: Agent Device Tester + text: Settings +- tapOn: + id: open-inert-surface +- assertVisible: + id: inert-title +- tapOn: + id: inert-target retryTapIfNoChange: true -- assertVisible: Agent Device Tester +- assertVisible: + id: inert-title diff --git a/scripts/maestro-conformance/differential/invariants.test.ts b/scripts/maestro-conformance/differential/invariants.test.ts index 93d1ef7ff..35827ea9e 100644 --- a/scripts/maestro-conformance/differential/invariants.test.ts +++ b/scripts/maestro-conformance/differential/invariants.test.ts @@ -124,22 +124,40 @@ test('a trace whose taps record no metric reports no-data rather than passing', assert.equal(result.status, 'no-data'); }); -// tap-retry-if-no-change is parked (#1300): tapRetries measured 0 then 1 across -// identical runs, so it is flaky rather than divergent. A knownDivergence -// declaration assumes the failure reproduces, so declaring a flaky scenario -// would make the schedule flip red at random. Assert it stays out of the active -// set until it has an inert control — re-adding it without one silently returns -// the coin-flip. -test('the flaky retry scenario is parked, not declared as a divergence', () => { +// tap-retry-if-no-change was parked while it was a coin flip (#1300): tapRetries +// measured 0 then 1 across identical runs. It is active again now that it taps +// the fixture's inert surface, and it carries its own detector — without the +// tapRetries invariant the scenario is back to proving nothing, because outcome +// parity passes whether or not the retry ever fires. +test('the retry scenario is active and carries the invariant that makes it mean something', () => { const retry = DIFFERENTIAL_SCENARIOS.find((s) => s.id === 'tap-retry-if-no-change'); - assert.equal(retry, undefined, 'tap-retry-if-no-change must stay parked until #1300 gives it an inert control'); + assert.ok(retry, 'tap-retry-if-no-change must stay in the active differential set'); + const [invariant, ...rest] = retry?.engineInvariants ?? []; + assert.equal(rest.length, 0); + assert.equal(invariant?.kind, 'metricAtLeast'); + assert.equal(invariant?.command, 'tapOn'); + assert.equal(invariant?.kind === 'metricAtLeast' && invariant.metric, 'tapRetries'); + assert.equal(invariant?.kind === 'metricAtLeast' && invariant.min, 1); +}); + +// A flaky scenario must be fixed, never waived: knownDivergence assumes the +// failure reproduces, so declaring this one would make the schedule flip between +// green and red at random — the trap #1300 was filed to escape. +test('the retry scenario is not waived by a divergence declaration', () => { + const retry = DIFFERENTIAL_SCENARIOS.find((s) => s.id === 'tap-retry-if-no-change'); + assert.equal(retry?.knownDivergence, undefined); }); -// The invariant itself stays implemented and proven, so the fix PR only has to -// re-add the scenario rather than rebuild the detector. -test('the tapRetries invariant remains implemented for when the scenario returns', () => { - assert.equal(evaluateInvariant([stopWithMetrics('tapOn', { tapRetries: 1 })], RETRY_INVARIANT).status, 'held'); - assert.equal(evaluateInvariant([stopWithMetrics('tapOn', { tapRetries: 0 })], RETRY_INVARIANT).status, 'violated'); +// The flow only forces the retry if it drives the inert surface. Tapping any +// live screen is what made this flaky, and that regression is invisible on the +// unit side — the invariant only goes red on a device. +test('the retry flow drives the inert surface, not a live screen', () => { + const flow = fs.readFileSync( + path.join(import.meta.dirname, 'flows/tap-retry-if-no-change.yaml'), + 'utf8', + ); + assert.match(flow, /id: open-inert-surface/); + assert.match(flow, /id: inert-target/); }); // Truncation-vs-rounding is at most 1px and cannot be observed on a device, so diff --git a/scripts/maestro-conformance/differential/scenarios.ts b/scripts/maestro-conformance/differential/scenarios.ts index 781a42de7..0857de498 100644 --- a/scripts/maestro-conformance/differential/scenarios.ts +++ b/scripts/maestro-conformance/differential/scenarios.ts @@ -148,19 +148,30 @@ export const DIFFERENTIAL_SCENARIOS: DifferentialScenario[] = [ ], divergenceMeans: 'The swipe behaves differently enough on one engine to fail the flow.', }, - // PARKED: tap-retry-if-no-change (flow kept at differential/flows/, invariant - // kept implemented and unit-tested) — https://github.com/callstack/agent-device/issues/1300 - // - // It is NOT declared as a knownDivergence, deliberately. tapRetries measured 0 - // in run 29504440599 and 1 in run 29510020718 with no change to the flow or - // commit: the tap sometimes holds the hierarchy signature still and sometimes - // does not, because the fixture home screen has live content. A declaration - // assumes the divergence REPRODUCES; a flaky one would flip between - // known-divergence (green) and stale-declaration (red) at random, which is - // worse than no scenario — a coin-flip job teaches people to ignore the - // differential. So retryIfNoChange has no device coverage until the fixture - // offers an inert control, and that absence is tracked rather than disguised - // as a green run. + { + id: 'tap-retry-if-no-change', + flow: 'differential/flows/tap-retry-if-no-change.yaml', + comparesAcrossEngines: 'A tap on an inert target completes on both engines.', + expect: 'pass', + // The whole point of the scenario. Outcome parity cannot see a retry: a tap + // that never re-taps passes just the same, which is how this scenario spent + // its first two runs proving nothing (#1300). The flow taps a surface that + // provably cannot change, so the retry MUST fire; if it does not, either the + // retry path regressed or the fixture stopped being inert, and both are + // things we want to hear about. + engineInvariants: [ + { + kind: 'metricAtLeast', + command: 'tapOn', + metric: 'tapRetries', + min: 1, + because: + 'a tap on an unchanging screen must re-tap; zero retries means retryIfNoChange never ran and the scenario proved nothing', + }, + ], + divergenceMeans: + 'agent-device stopped re-tapping when the screen holds still (a retryIfNoChange regression), or the fixture surface it taps is no longer inert.', + }, { id: 'optional-warned-not-failed', flow: 'differential/flows/optional-warned-not-failed.yaml',