Skip to content

test(cli): copy launcher module dependencies into activation fixtures - #7733

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:test/launcher-module-fixtures-20261026
Oct 26, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:test/launcher-module-fixtures-20261026

Conversation

@luvs01

@luvs01 luvs01 commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Launcher activation fixtures copy the entire TypeScript proxy tree even though they execute the real Node launcher and a small JSON CLI stub. Copy the launcher's imported module graph instead, recursively following its local .mjs dependencies, and explicitly create the stub's src/cli directory.

The graph currently contains 26 source modules. An ordinary fixture was observed at about 2,186 files / 27.1 MB before and 28–29 files / 243 KB after this change. The real Bun binary copies, synthetic installer, runtime-selection assertions, environment and existing deadlines remain unchanged.

This changes one test file only, based on dev 8ebe46b2. It does not change runtime selection policy.

Verification

Official Windows Bun 1.4.4 and Node 24.18.0, with isolated homes/caches and the existing CI hook timeout:

  • All seven Windows activation modes passed across two focused runs: missing, broken, inspection, bundled, repair, override and terminal-none (35 assertions).
  • Privacy and file-size checks passed; the committed diff has no whitespace errors.
  • Native processes reached terminal exit with both output streams complete, no remaining direct child and verified fixture cleanup.
  • The unchanged reference snapshot passed four selected modes and timed out in repair. An earlier reference attempt could not resolve zod/v4 before test dispatch; it was retained as a harness failure, then both comparison runs used the same existing dependencies without installation.

Sequential execution, OS caches and recursive fixture observation can affect timings, so those measurements do not establish a speedup or the cause of the earlier repair timeout. The claim is the narrower copy scope with preserved observed behavior.

CI run 40239151609 passed on this exact head, including Typecheck, privacy and all four Ubuntu test shards. Its launcher test log also confirms all activation cases and the POSIX rejected-PATH case passed.

A full local suite was not repeated for this one-file fixture change because concurrent Windows worktrees share the validation host. The focused Windows activation runs and the hosted Ubuntu tests cover the changed preparation path. The complete hosted Windows/macOS matrix was not run. No installed app, service or provider call was used.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed; this is test-only preparation.
  • The change was reviewed for secrets, auth and unsafe defaults.

Summary by CodeRabbit

  • Tests
    • Updated launcher test fixtures to copy only the modules needed by the Node launcher, while retaining existing runtime and execution coverage.

…n fixtures

The PATH-fallback fixture copied the whole TypeScript src tree; the Node launcher only needs its own .mjs import closure (26 modules including src/service/state-record.mjs), copied with a repository-escape guard. A comment on launcherFallbackFixture records which modes share the warmed PATH Bun and which execute a fresh copy via the fixture's synthetic install.js.
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8ebe46b and a0b77b8.

Files selected for processing: 1
  • tests/cli/ocx-launcher-runtime.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The launcher runtime test fixture now copies the Node launcher’s imported module graph instead of recursively copying the full src tree. The fixture retains its existing setup for PATH candidates, bundled runtimes, and launcher execution.

Changes: Launcher runtime fixture

Layer / File(s) Summary
Copy the launcher module graph
tests/cli/ocx-launcher-runtime.test.ts
The fixture now traverses the launcher’s imports, rejects sources outside the repository, and copies the discovered modules. It recursively follows imports from .mjs files and uses this helper instead of copying the full src tree.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related issues

  • lidge-jun/opencodex#7578 — The issue concerns setup time for a test using the same fixture. This change reduces the copied source set, but does not change the test budget or PATH Bun probe, and does not establish that the issue is resolved.

Possibly related PRs

Merge Risk: Minimal · up to a0b77

No actionable merge risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: updating CLI launcher tests to copy the launcher module dependencies into activation fixtures.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • 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.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Oct 25, 2026
@luvs01
luvs01 marked this pull request as ready for review October 25, 2026 23:04
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer dev integration record (release 2.93 dev-merge loop), per the MAINTAINERS.md maintainer-integration exception for dev. This is maintainer integration, not an approving review.

  • Exact head: a0b77b8; no change requests; 0 unresolved review threads.
  • Required CI on the exact head: CI run 40239151609 (ci, gates: success), enforce-target 40243256842: success, hygiene 40243256871: success.
  • Independent technical review (separate agent): verdict MERGE, 0 blocking findings; with the repository-pinned Bun 1.4.4 the combined tree ran tests/cli/ocx-launcher-runtime.test.ts via test:changed at 73 pass / 6 skip / 0 fail; layout guards, ratchet, structure:check, privacy:scan pass. fix(cli): enforce the Bun patch floor in automatic PATH fallback #7734 (which shares this test file) is not included.
  • Maintainer recheck on the head merged onto origin/dev 0eb8cd0: typecheck exit 0. With the host's PATH Bun (1.3.14) the launcher file shows the same 8 PATH-Bun validation/activation failures on clean origin/dev as on the combined tree (65 pass / 6 skip / 8 fail both), so they are host-environment and not introduced here.

@lidge-jun
lidge-jun merged commit 8e18e60 into lidge-jun:dev Oct 26, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants