test: cover runtime benchmark script

[ndycode]

Summary

• add a focused smoke test for scripts/benchmark-runtime-path.mjs
• verify the script writes a JSON benchmark payload with the expected result entries
• improve coverage for a previously untested dist-backed entrypoint script

Validation

• npm run typecheck
• npm run lint -- test/benchmark-runtime-path-script.test.ts
• npm run test -- test/benchmark-runtime-path-script.test.ts
• manual QA: build and run node scripts/benchmark-runtime-path.mjs --iterations=1 --output=<tmp>/runtime-benchmark.json

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

adds a focused smoke test for scripts/benchmark-runtime-path.mjs that spawns the script with stub dist fixtures and verifies the written json payload contains the expected result entry names. both prior blocking concerns (bare rmSync in cleanup, unused dirname import) are fully addressed in this revision.

• afterEach correctly uses removeWithRetry with { recursive: true, force: true } — windows filesystem safety is satisfied
• import block is clean: only join from node:path, no dead imports
• fixture construction properly creates the stub dist/lib/... files so the script's static esm imports resolve against the temp dir, not the real dist/
• the one remaining gap is that spawnSync has no timeout option (hang risk) and doesn't surface result.stderr in assertion failures, making CI debugging harder if the child process crashes

Confidence Score: 4/5

• safe to merge after adding a spawnSync timeout and stderr assertion; no correctness or windows safety regressions
• both prior blocking issues are resolved; the only remaining feedback is a P2 style suggestion (timeout + stderr surfacing on spawnSync). the test logic, fixture setup, and cleanup are correct.
• test/benchmark-runtime-path-script.test.ts — spawnSync timeout and stderr surfacing

Important Files Changed

Filename	Overview
test/benchmark-runtime-path-script.test.ts	new smoke test for the benchmark script; prior blocking issues (bare rmSync, unused dirname import) are fully resolved; one remaining P2 — spawnSync lacks a timeout guard and doesn't surface stderr on failure

Sequence Diagram

sequenceDiagram
    participant T as Test (vitest)
    participant F as createRuntimeBenchmarkFixture()
    participant FS as tmp filesystem
    participant S as spawnSync (node)
    participant B as benchmark-runtime-path.mjs

    T->>F: call fixture helper
    F->>FS: mkdtempSync → fixtureRoot
    F->>FS: write stub dist/lib/request/request-transformer.js
    F->>FS: write stub dist/lib/request/helpers/tool-utils.js
    F->>FS: write stub dist/lib/accounts.js
    F->>FS: copyFileSync real script → fixtureRoot/scripts/
    F-->>T: { fixtureRoot, scriptCopy }

    T->>S: spawnSync(node, [scriptCopy, --iterations=1, --output=...])
    S->>B: executes script
    B->>FS: import stubs from ../dist/lib/...
    B->>FS: writeFile(outputPath, JSON payload)
    B-->>S: stdout "Runtime benchmark written: ..."
    S-->>T: { status, stdout }

    T->>FS: readFileSync(outputPath)
    T->>T: assert payload.iterations === 1 && result names match

    T->>T: afterEach → removeWithRetry(fixtureRoot)

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/benchmark-runtime-path-script.test.ts
Line: 74-78

Comment:
**`spawnSync` missing `timeout` and stderr surfacing**

no `timeout` option means the test can hang indefinitely if the script gets stuck (e.g., the async promise never resolves). add a reasonable cap. also, when `result.status !== 0`, the test failure message will only show "expected 1 to be 0" — you lose the actual error output from the script, which makes CI failures hard to diagnose. consider asserting stderr is empty or including it in a custom message:

```suggestion
		const result = spawnSync(
			process.execPath,
			[scriptCopy, "--iterations=1", `--output=${outputPath}`],
			{ encoding: "utf8", timeout: 15_000 },
		);

		expect(result.stderr).toBe("");
```

the `expect(result.stderr).toBe("")` line gives you the full error text in the failure diff if the script crashes.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "Drop unused benchmar..."}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

• skip-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a3579c4a-5e70-4428-af6c-866a88820f92

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/pr10-benchmark-runtime-path-smoke

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch test/pr10-benchmark-runtime-path-smoke

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}