Skip to content

Migrate workflow browser permutations to mounted RTL - #80

Closed
comp615 wants to merge 3 commits into
mainfrom
migrate-e2e-rtl
Closed

comp615 wants to merge 3 commits into
mainfrom
migrate-e2e-rtl

Conversation

@comp615

@comp615 comp615 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

🤖 AI-authored implementation and PR description.

This moves seven semantic workflow journeys from Playwright into a colocated, mounted WorkflowChannel RTL/jsdom suite under StrictMode. The four policy browser files changed by the earlier draft are restored byte-for-byte to pinned main, so live-status broker transitions and notification/unread UI behavior remain browser-covered. Production code and fixture setup/history declarations are unchanged.

Coverage map (browser → mounted RTL):

  • lazy/paged run history, cursor disposal, and unknown-run acknowledgment without replay
  • exact lost-save readback adoption without resubmission
  • different-head explicit review, retained draft, Escape, failed dismissal restoration, and fresh retry
  • optimistic dismissal removal/restoration while the modal and editor ownership remain locked until controlled settlement
  • legacy deletion acceptance without claiming runtime removal
  • invalid timeout matrix (oops, 0s, 1.5, unsafe integer), mode round-trips, retained draft, 5m → exact timeout_secs: 300, and blank omission
  • huge parsed step ID followed by both Add actions allocating step_1 and step_2

Retained workflow browser seams: the mixed editor case, keyboard case, real StrictMode session/community/access journey, reconnect/in-flight run journey, and generic Outbox no-replay journey. Other browser files and all live-status/notification/unread cases remain at pinned base.

Negative control: replacing collision-based step-ID allocation with numeric max+1 reproduces the unsafe-integer regression as a non-settling mounted test; the injected production change was restored and git diff verified production clean. The controlled dismissal gate separately proves pending ownership, Escape blocking, failed persistence restoration, and successful late settlement without sleeps.

Local evidence on the initial mounted-test commit:

  • mounted suite: 7/7 passed; Vitest duration 3.36s (5.07s process wall; local timing is not a controlled benchmark)
  • retained workflow browser file: Chromium 5/5 in 8.2s; WebKit 5/5 in 10.3s
  • TypeScript and Biome checks passed; pre-push related Vitest 8/8 passed
  • exact case delta against pinned base: Playwright −7, mounted Vitest +7; no production diff

The mounted-test CI run passed JavaScript, native, measurements and three browser shards, but failed Chromium shard 2 at message-navigation.spec.mjs:148 with “Execution context was destroyed” during fixtureNavigation.open. This is outside the changed workflow files and matches the unresolved error seen on #78. Its root cause is not established. No green full-gate or speedup claim is made for that run.

Follow-up review fixes re-query the mounted timeout field after Form/YAML transitions, preserve exact save-count assertions, and release held dismissal operations in finally. All seven mounted cases passed after these changes; TypeScript/Biome and the eight related pre-push tests also passed. The latest push starts fresh CI; this remains draft pending its result.

Prior candidate run 35116310013 is superseded. Its 548.845s value was the slowest Playwright step, not the full required gate. The actual first-job interval was 15:34:59–15:45:33 = 634s (10m34s), compared with pinned baseline intervals of 587s and 716s. Timing comparisons must use the same units.

comp615 and others added 2 commits September 16, 2026 11:34
Signed-off-by: Charlie Croom <ccroom@squareup.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0a149-b278-7550-acc4-356168b0c4b5
Co-authored-by: Amp <amp@ampcode.com>
Signed-off-by: Charlie Croom <ccroom@squareup.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0a149-b278-7550-acc4-356168b0c4b5
Co-authored-by: Amp <amp@ampcode.com>
@comp615 comp615 changed the title Migrate policy browser permutations to owner tests Migrate workflow browser permutations to mounted RTL Sep 16, 2026
Signed-off-by: Charlie Croom <ccroom@squareup.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0a149-b278-7550-acc4-356168b0c4b5
Co-authored-by: Amp <amp@ampcode.com>
@comp615

comp615 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Testing update for head 78d3828:

The full local Vitest suite passed: 148 files / 1,559 tests. The focused workflow + broker run also passed (17 tests).

Latest CI passed all four browser shards, browser measurements, native checks, and the seven new mounted workflow tests. JavaScript failed in the unchanged broker priority/cancellation test: observed upstream-call spacing was 488.313ms against a ≥490ms assertion. The scheduler sets its 500ms interval before broker authentication signing, while the test records timestamps after signing and signature verification. That measurement-boundary mismatch is a likely explanation, not a reproduced root cause; no assertions were weakened and no unrelated broker code was changed.

The first-job-to-required-gate interval was 11m35s, versus 11m28s for the latest pinned baseline run (35049707495, attempt 4). This run does not demonstrate an overall speedup, and its required gate is red. Keeping this PR draft.

@comp615

comp615 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Closing at Charlie’s request to focus review on #81#83. This workflow RTL migration did not establish an overall CI speedup. Its branch and coverage work remain available; no tests are removed from main by closing this PR.

@comp615 comp615 closed this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant