Repository navigation
test(ios): isolate smoke contracts and synchronize observable state - #3786
Conversation
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 15 files
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
At 5c7efc8 one thing should change before this is ready: the unrelated-prompt Smoke Tests on iOS 26.2 is still queued, not failed. It runs the ios.yml steps, the runner XCTests and the smoke harness this PR changes, so its result at this head is the signal we still need. I read the diff and took the retry-policy mutation result and the local 22/22 and 5/5 runs from the PR body and evidence doc. I did not reproduce them. I know of no conflicts. Not blocking: live-runner.ts:159 decides whether a scenario owns its launch by comparing three hard-coded scenario ids, and Could Two smaller threads from the other reviewer still apply: one P3 on the delivery assertion in RunnerTests+SynthesizedTextEntryTests.swift:173, and one P3 on the stale "deep link landed" comment. The P2 thread on the /webview URL does not apply, because that URL was a setup handoff and not a declared contract. The Next: add the matcher, answer the design question, and let Smoke Tests (iOS 26.2) pass at the new head. |
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
The earlier findings from 5c7efc8 are mostly addressed at b78ff23, but one ask is still open, so this needs one more change before merge. CI is green: 18/18 checks pass at b78ff23, including the iOS workflow that runs the changed keyboard step, the runner XCTests and the smoke harness. There are no conflicts. The unrelated-prompt Not blocking, and you can take or leave it: the conformance probe in On the open inline threads, these still apply: the missing matcher above, and the cleanup failure artifacts thread ( I did not run the simulator smoke, the focused 49/49 tests or the cleanup base-vs-patch experiment. Those results come from the PR body and the evidence doc. I could not confirm that the 22/22 local run used exactly b78ff23, since the evidence doc names harness source 316052b. I also did not check whether the built dist exposes an importable daemon transport. |
b78ff23 to
1067255
Compare
|
Rebased onto The unrelated-prompt test now requires its intended AssertionError. A mutation confirms the old matcher accepted an unrelated Error, while the new matcher fails it. Cleanup captures screenshot/snapshot/report artifacts after exhausting its own budget, without reissuing the command or losing typed already-clean handling on attempt three. Restored the masked prefilled diagnostic, corrected the WebView comment, and restored serial-command/signature rationale in the authoring guide. The conformance probe retains the agreed existing-daemon transport ( Focused checks: 32/32. Independent review: no findings. |
|
Reviewed 1067255. The code looks good, and the findings from b78ff23 (#3786 (comment)) are fixed: exhausted cleanup now records its failure evidence, and the deep-link destination test matches the assertion that names the prompt. All 18 checks pass, including the iOS job that runs the changed harness. There are no conflicts. Not blocking: no test drives The local simulator run in the evidence doc is from harness source 316052b, so at this head only the CI iOS smoke covers the rebased code. The remaining open threads can be resolved. Fixed at this head: r4303084766, r4303084772, r4303084778, r4303318244, r4303318255 and r4303318258. Not applicable: r4303084769 (smoke:deep-link-navigation still covers URL delivery) and r4303318250 (the stricter Application/Window-only frontier is deliberate for the truncated raw-depth contract). |
|
Summary
Give iOS smoke distinct sessions, declared starting states, and independent subtests. Use one built CLI and fixture controls for depth/WebView setup. Keep deep-link delivery and exact confirmation handling separate; readable misses remain fatal.
Masked XCTests require app-observed complete-suffix delivery and zero toggle presses. Readable fields retain exact replacement and dropped-clear checks. Cleanup captures exhaustion evidence without reissuing commands or losing typed already-clean handling. The prompt test now requires the intended AssertionError.
Scope: 22 files across iOS tests/fixtures, workflow, shared failure recording, and authoring guidance. Rebased onto
6e8c2a046c; preserved upstream documentation. Detailed evidence.Validation
360ae21bfb: focused 32/32; independent review passed. Prompt mutation: old test 1 false pass, new matcher 1 failure, repaired 1 pass.316052ba2e: simulator 22/22, 102s, no skips; iPhone 17 Pro/iOS27.0, current Release fixture. Sessions/daemon cleaned; keyboard preference restored. Prior focused suite 49/49; revised native tests 5/5.92db564a30: masked pair 1 failure/1 pass. Cleanup base5c7efc83ea: 1 failure → patched 1 pass.pnpm check:affected --run --head HEADpassed at1067255afada175396fbbf98db710f80938f9d14, exactCommit=true, receiptrun-U6l3D6.CI at
1067255afcompleted without failures, including iOS 26.2/macOS smoke, Integration Tests, and Coverage. The fixture-build matrix was skipped; Cubic was neutral.