Skip to content

test(e2e): capture scenario assertion failure evidence - #2921

Open
thymikee wants to merge 2 commits into
mainfrom
test/scenario-assertion-failure-evidence
Open

thymikee wants to merge 2 commits into
mainfrom
test/scenario-assertion-failure-evidence

Conversation

@thymikee

@thymikee thymikee commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Summary

Capture a screenshot, snapshot, and device facts when a live E2E scenario assertion fails after allowed CLI misses. Preserve the original error and avoid recapturing a command failure. The iOS smoke lane deliberately exercises the assertion route after a real session opens and checks its evidence. Successful product scenarios are unchanged.

This refreshes #2368 on current main; the original implementation commit retains @billsbooth as author. Three test-harness files changed. No workflow expansion.

Validation

Rebased head 15b7d9dcb5 on fa1082dd: pnpm install --frozen-lockfile, pnpm build, pnpm format, focused harness tests (7/7), and AGENT_DEVICE_VITEST_MAX_WORKERS=1 pnpm check:affected --run passed. The previous-head iOS run at 6920816c1d passed and logged a 6.84s canary; its uploaded archive contains the PNG, successful snapshot JSON, and deliberate-assertion report. The new exact-head iOS job and renewed uploaded artifact proof are pending.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.81 MB 4.81 MB 0 B
Package (unpacked) 4.81 MB 4.81 MB 0 B
Package (download) 1.44 MB 1.44 MB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.7 ms 26.8 ms +0.0 ms
CLI --help 76.5 ms 78.4 ms +2.0 ms

@thymikee

Copy link
Copy Markdown
Member Author

Re the review on #2368: the replacement now has live assertion-path evidence on exact head c1cb496558. On an isolated iOS 26.2 simulator, I opened Settings, ran an allowed missing-landmark probe, then threw a deliberate scenario assertion. runScenario rethrew the identical Error object; before cleanup it wrote a PNG with a valid signature (86,460 bytes), a successful snapshot containing the Settings tree (28,070 bytes), and a report naming both files. I closed the session and removed the dedicated daemon and simulator. The first cold runner launch timed out before the assertion; the bounded retry reached and verified the intended path.

The 11 focused tests and exact-head pnpm check:affected --run pass. GitHub CI is pending, and this local proof does not yet supply the uploaded CI artifact requested in the original review. The always-on artifact uploader includes test/artifacts/** if that lane reaches a scenario assertion.

@thymikee

Copy link
Copy Markdown
Member Author

The code looks right, but the capture path is not yet proven on a real run at c1cb496.

The changed route (runScenario's catch → recordScenarioFailure → captureFailedStepEvidence, https://github.com/callstack/agent-device/blob/c1cb496/test/integration/live-device-e2e/runtime.ts#L130) is exercised by the platform Smoke Tests jobs: ios.yml's smoke-ios job runs smoke-ios-simulator.test.ts → runIosSimulatorE2E → harness.runScenario, and android.yml/linux.yml follow the same chain. No CI run or uploaded artifact yet shows this exact head's capture path firing on a real device; the manual claim in the PR is unlinked and the author himself flags it as insufficient. Merging without that artifact leaves the device-facing capture behavior verified only by reading the code, not by seeing it run. Please wait for Smoke Tests to finish on c1cb496 and link the uploaded test/artifacts/** evidence (failed-step-N.png, failed-step-N-snapshot.json, failed-step.txt) from a run that reaches a real scenario assertion, closing the gap the #2368 review asked about.

Smoke Tests on c1cb496 route through this same runtime.ts, so a failure there would overlap this PR's diff, but the run was still queued or in progress at review time, so there is nothing to attribute yet. I could not check out the PR branch to run the new test file myself, and I could not confirm the author's manual iOS 26.2 simulator session (file sizes, object-identity check, cleanup) since no artifact or CI link was given for it. The next thing needed before merge is Smoke Tests completing on c1cb496 with an uploaded test/artifacts/** capture from a run that reaches a scenario assertion, per the outstanding #2368 request.

@thymikee
thymikee force-pushed the test/scenario-assertion-failure-evidence branch from c1cb496 to 6920816 Compare September 24, 2026 17:02
@thymikee

Copy link
Copy Markdown
Member Author

Following up on the artifact request: I rebased onto the main that includes #2896 and #2919 and added a permanent live iOS assertion canary at 6920816c1d. After a real smoke scenario opens a session, it throws a unique assertion through runScenario, checks that the same error comes back, and validates failed-step-N.png, failed-step-N-snapshot.json, and failed-step.txt. The existing ios-artifacts upload includes these files. No workflow step or retry layer was added.

The exact-head local gate passed (AGENT_DEVICE_VITEST_MAX_WORKERS=1 pnpm check:affected --run), along with the focused harness tests (7/7). The new iOS Smoke Tests job is queued. I will link its uploaded artifact and measured canary time after it finishes; the CI artifact proof is still pending.

@thymikee

Copy link
Copy Markdown
Member Author

This is a follow-up on the earlier review (c1cb496). The earlier round asked for an uploaded test/artifacts/** capture from a real Smoke Tests run reaching a scenario assertion on this head before merge. The delta at live-runner.ts:99 adds a deterministic canary, assertLiveAssertionCapture, that would produce exactly that evidence, but the Smoke Tests job on 6920816 was still in progress at review time, so no completed run or uploaded ios-artifacts link exists yet showing the canary pass on real hardware. Merging before that run finishes ships the device-facing capture-on-assertion path with only static-review confidence, not the live confirmation the earlier round asked for. Please wait for the ios.yml Smoke Tests run on 6920816 to finish, confirm it is green, and link the uploaded ios-artifacts artifact showing failed-step-N.png, failed-step-N-snapshot.json, and failed-step.txt from the canary.

Not blocking: could the dependency of assertLiveAssertionCapture on smoke:automation-input running before smoke:capture-close in SCENARIO_DEFINITIONS order be worth a comment near that list, since it's implicit ordering rather than an enforced invariant? Either way the existing assert.ok already fails loudly if it breaks, so this can be taken or left.

CI is pending: ios.yml's Smoke Tests job runs test/integration/smoke-ios-simulator.test.ts through runIosSimulatorE2E and executeLiveScenarios, the exact function this delta changed, and that job was still in progress at review time with no result yet to attribute. I could not observe that run complete, so I have no uploaded ios-artifacts link confirming the canary passes on real hardware or that failed-step-N.png, failed-step-N-snapshot.json, and failed-step.txt are produced as designed. I did not independently run the local gate or the unit tests the author cites; I took the author's local-pass claim on faith since it covers non-device-facing unit coverage, not the required live evidence. The next thing that has to happen before merge is the Smoke Tests run on 6920816 finishing green with that artifact linked.

@thymikee

Copy link
Copy Markdown
Member Author

The requested exact-head live proof is now available. iOS Smoke Tests on 6920816c1d finished green, and its log reports iOS live assertion capture: 6840ms from executeLiveScenarios.

I downloaded the run's ios-artifacts archive and verified the same smoke directory contains failed-step-67.png (valid 402×874 PNG showing the fixture Settings screen), failed-step-67-snapshot.json (success: true, with nodes), and failed-step.txt naming smoke:failure-evidence-canary, the deliberate AssertionError, and both evidence files. The existing upload step succeeded. This is the capture route requested in the earlier #2368 review, on the current PR head.

For the nonblocking ordering note: the canary's assert.ok(context.sessionOpen) makes a future scenario-order change fail at the point of use, so I kept that invariant executable rather than adding a comment that could drift.

@thymikee
thymikee force-pushed the test/scenario-assertion-failure-evidence branch from 6920816 to 15b7d9d Compare September 24, 2026 17:54
@thymikee

Copy link
Copy Markdown
Member Author

I rebased #2921 onto fa1082dd because merged snapshot-quality, wait, and timeout changes affect the CLI capture path that the canary exercises. The new head is 15b7d9dcb5; the three-file PR patch replayed without conflicts, the original contributor remains the author of the implementation commit, and the exact-head local affected gate passed.

The 6.84s live canary and uploaded evidence are from the prior head 6920816c1d. The new exact-head iOS job is queued. I will verify and link its uploaded assertion artifact before treating the rebased head as live-proven.

@thymikee

Copy link
Copy Markdown
Member Author

The code looks right at 15b7d9d, but merge readiness still comes down to one thing: live proof on this exact head.

The only uploaded live-canary evidence in this thread (PNG, snapshot JSON, failed-step.txt from a real iOS session) is linked from run 36031511215 on the prior head 6920816. That head is gone after the rebase. 15b7d9d rebases onto fa1082d, which pulls in changes to the same CLI capture path this canary walks (screenshot/snapshot output shape, wait/timeout policy) in https://github.com/callstack/agent-device/blob/15b7d9d/test/integration/ios-simulator-e2e/live-runner.ts#L115. No completed iOS Smoke Tests run and no uploaded test/artifacts/** exist yet for 15b7d9d, so we don't yet know that assertLiveAssertionCapture still fires correctly on real hardware after the rebase. Merging without that would repeat the exact gap the last two review rounds asked to close.

Android, Linux, and macOS Smoke Tests are already green on 15b7d9d. The iOS Smoke Tests job (run 36037593196, job 107761446770) is still in progress; it runs runIosSimulatorE2E -> executeLiveScenarios -> assertLiveAssertionCapture, the function this PR adds, so it's the one check that matters here.

The path is clear once that job finishes: link the completed, green iOS Smoke Tests run for exact head 15b7d9d, with its uploaded ios-artifacts showing failed-step-N.png, failed-step-N-snapshot.json, and failed-step.txt from the canary. I didn't re-run the unit/integration suite myself for these files; I relied on the author's stated pnpm check:affected pass and confirmed the diff is unchanged from the already-reviewed 6920816 head. I also didn't read the merged upstream capture-path commits line by line, only confirmed they touch the same route, which is why we need the live run rather than another static pass.

@thymikee

Copy link
Copy Markdown
Member Author

The exact-head iOS job on 15b7d9dcb5 failed before the assertion canary. The Settings replay's step 3 snapshot never connected to the XCTest runner: two sends each spent the 45 s transport budget, and simulator runner cleanup reported xcrun timed out after 2000ms. The fixture E2E step that contains the canary was skipped, so this run supplies no evidence for or against the canary.

I downloaded the uploaded ios-artifacts and confirmed those timings in the request log. I requested a failed-job rerun of the same run; attempt 2 is queued. #2921 remains unready until an exact-head live run reaches the canary and uploads the PNG, snapshot JSON, and report.

@thymikee

Copy link
Copy Markdown
Member Author

The rerun completed the missing live proof on exact head 15b7d9dcb5. iOS Smoke Tests attempt 2 is green; the fixture E2E ran and logged iOS live assertion capture: 7043ms.

I downloaded that attempt's ios-artifacts archive. The canary directory contains failed-step-67.png (valid 402×874 PNG), failed-step-67-snapshot.json (success: true, 41 nodes), and failed-step.txt naming smoke:failure-evidence-canary, the deliberate AssertionError, and both files. This is the requested capture path after the rebase; all exact-head checks are green and GitHub reports the PR mergeable. I have not merged it.

The rerun waited 34m31s for a runner and executed for 10m38s. The earlier attempt's Settings replay transport failure remains documented above; it never reached the canary.

@thymikee

Copy link
Copy Markdown
Member Author

The live proof for 15b7d9d is now complete. iOS Smoke Tests attempt 2 ran the fixture E2E on this exact head, and the uploaded canary files show the PNG, the snapshot JSON, and the failure report. The earlier code review had no open findings, so this is ready for human review.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants