Skip to content

Fix the shared PTY write spy unit-test CI blocker - #24651

Closed
nwparker wants to merge 1 commit into
mainfrom
nwparker/baseline-pty-spy-test-typing
Closed

nwparker wants to merge 1 commit into
mainfrom
nwparker/baseline-pty-spy-test-typing

Conversation

@nwparker

@nwparker nwparker commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​15 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​9
Prod 0 0 0 0

ELI5

The same unchanged unit test blocks several integration PRs because it uses three patterns rejected by CI. Give its mock the real Playwright type and read the checked delay property directly.

What Changed

Only terminal-pty-write-spy.unit.test.ts: use ElectronApplication['evaluate'] for the mock, pass the imported Electron module to function callbacks, and check the stubbed global exists before typed property reads. The test still executes the callback with evaluate’s second argument and proves delay changes from 35 to 0.

Why

#24572, #24575, and #24584 all fail the same anti-slop gate on main’s broad object callback and two Reflect.get calls. This fixes the test boundary without changing the runtime helper, weakening the assertions, adding casts, or suppressing lint.

Linked Issue

Shared CI blocker observed in #24572, #24575, and #24584; no issue closure requested.

Visual Proof

N/A — pure unit-test typing/property access; no UI or runtime behavior changes.

Testing

  • Automated tests updated; existing backpressure regression passes.
  • Manual app test: unnecessary; no app launched.

ORCA_BACKGROUND_LAUNCH=1 pnpm test tests/e2e/helpers/terminal-pty-write-spy.unit.test.ts, exact pnpm run audit:anti-slop, and changed-code quality pass. pnpm typecheck:e2e reports 222 existing errors; its full diagnostics are byte-identical against unmodified main 53930a161be4dd4420e9c71be0dadac6e9388752, with no errors in this test.

Review

Self-reviewed the single-file diff: both delay assertions still depend on the callback’s second argument. Runtime helper unchanged; no casts, broad object/any declarations, reflection, or lint suppressions added.

Agent skill upstream boundary

  • Not applicable.

Notes

No platform, SSH, folder workspace, wire, or production execution change. No app, auth, or global configuration changes. Antigravity draft #24627 remains separate and unchanged.

Checklist

  • Small and focused; what/why explained.
  • N/A visual proof explained.
  • Self-reviewed for correctness, security, and performance.
  • Cross-platform and remote impact considered: none.
  • Focused test and exact CI gate pass; pre-existing e2e typecheck errors documented above. Full production build unnecessary for unit-only change.

@nwparker
nwparker marked this pull request as ready for review October 2, 2026 08:49
@nwparker

nwparker commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by the test typing/property fix that has now landed on main in b49abdb (#24250). I inspected the exact current-main test: the broad object callback and both Reflect.get calls are gone, and the stubbed delay has a typed global declaration. The standalone fix here also passed the existing regression and exact anti-slop gate independently, but is now redundant. Closing this PR; integration PRs will rebase onto current main rather than wait for it.

@nwparker nwparker closed this Oct 2, 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