Skip to content

fix(cli): the npx shim test runs a stub npx, not the runners - #4175

Open
miguel-heygen wants to merge 2 commits into
mainfrom
fix/npx-command-test-no-host-npx
Open

miguel-heygen wants to merge 2 commits into
mainfrom
fix/npx-command-test-no-host-npx

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changes

The npxCommand unit test that ran the host's real npx --version through cmd.exe timed out on the Windows CI runner (spawnSync cmd.exe ETIMEDOUT), so an unrelated PR could go red on a machine-load problem. A unit test should not depend on the runner's npx or the network.

The test now writes a stub npx (npx.cmd on Windows) into a temp folder, puts that folder first on PATH, and runs the command buildNpxCommand builds. It expects the stub's output. So cmd.exe /d /s /c npx.cmd resolution is still exercised end to end, with no host npx, no network and a 15 second bound. The test itself is not retried.

The per-platform command-shape tests are unchanged.

Verification

  • npxCommand.test.ts: 7 pass on Linux at the first head. The last commit only adds maxRetries: 3, retryDelay: 100 to the temp-dir cleanup (a Windows file lock could otherwise hide the assertion); it was not re-run on the remote runner, so CI on this head is its verification.
  • Deliberate mutation: with the stub folder left off PATH, the new test fails (expected '10.9.4' to be '9.9.9', the real npx's answer), so it does test the stub path.
  • Not exercised here: Windows and macOS. The cli-npx-shim CI job runs this file on Ubuntu, macOS and Windows, which is the check for the cmd.exe and npx.cmd path.

Review

An independent review report is attached to the gate record for this head.

The test spawned the host npx through cmd.exe, which timed out on the Windows

runner. It now puts a stub npx first on PATH and runs the built command against

it, so shim resolution is still exercised with no host npx and no network.
Windows can briefly lock a just-run temp dir; a cleanup error would hide the assertion.
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