Skip to content

test: tighten the round-2 parity, serve and watcher tests - #30267

Merged
nwparker merged 1 commit into
mainfrom
nwparker/r2-cleanup-test-hygiene
Oct 18, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/r2-cleanup-test-hygiene

Conversation

@nwparker

@nwparker nwparker commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​57 $\color{#cf222e}{\Huge{\mathbf{−}}}$​47 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5

ELI5

This tidies three tests added during round 2 of the SSH hardening work. The tests check the same things; they are just harder to break by accident.

What Changed

  • runtime-composition-parity.test.ts
    • One stringEnd helper replaces three copies of the quote/escape skipping loop.
    • stripComments no longer loops forever on an unterminated /*. Before, indexOf returned −1 and reset the scan to the start.
    • Internal shorthand ("C2", "D3") is gone from the comments.
  • orcad-serve-argv.ts and orcad-serve-parity.integration.test.ts
  • filesystem-watcher-ignore-real.test.ts
    • The post-storm sentinel is written at both watched levels, so the "events arrive in order" argument never crosses directories.

Production behaviour is unchanged. The CLI keeps calling orcadServeArgs(args, webClientRoot).

Why

These are findings from the round-2 clean-code review. The infinite loop would hang the parity test the first time a composition root ends in an unterminated comment. Positional argv splicing breaks silently when the argv order changes.

Linked Issue

N/A (follow-ups to #30212, #30222 and #30236).

Visual Proof

N/A: test-only, plus an optional parameter.

Testing

  • The parity test, the watcher test and serve-orcad-launch.test.ts pass, single-worker.
  • Ran the full local "static analysis and typecheck" job: clean.
  • Codex review (gpt-6.1-sol, high): clean.
  • Not run locally: orcad-serve-parity.integration.test.ts. A local build:orcad needs the mobile-web bundle, and the machine is under heavy load. CI's integration lane runs it.

Parity test: one string-skip helper, no infinite loop on an unterminated block comment, no internal shorthand. Serve test: orcadServeArgs takes a bindHost instead of the test splicing argv, typed readiness, and stderr back in the daemon failure. Watcher test: a sentinel at both watched levels.
@coderabbitai

coderabbitai Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 8 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6c4c9813-a371-4ab0-ae28-7d9befd8a4b7
📥 Commits

Reviewing files that changed from the base of the PR and between 2d0fa93 and ce10ee2.

📒 Files selected for processing: 4
  • src/main/ipc/filesystem-watcher-ignore-real.test.ts
  • src/main/orcad/orcad-serve-parity.integration.test.ts
  • src/main/runtime/construction/runtime-composition-parity.test.ts
  • src/shared/orcad-serve-argv.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker
nwparker merged commit 5229ef8 into main Oct 18, 2026
47 checks passed
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