fix(ci): use is-ci-cli binary so npm test actually runs Jest#263
Merged
Conversation
The `is-ci` binary in node_modules/.bin/ was resolving to the standalone
`is-ci` package (a transitive dep), whose bin is just
`process.exit(require('./') ? 0 : 1)` -- it exits 0 in CI without
running anything. As a result `npm test` has been silently green for
some time across every matrix cell without invoking Jest at all.
The `is-ci-cli` devDep exposes both `is-ci` and `is-ci-cli` binaries,
but loses the bin-name collision with the standalone package depending
on install order. Calling the unique `is-ci-cli` name avoids the
collision and matches the package's documented usage.
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Surprising bug:
npm testhas been silently green-without-tests in CI for some time. Across every matrix cell on master (ae850d0), the test step prints> is-ci 'test:ci' 'test:local'and exits in ~0.1s without ever invoking Jest. NoTests: N passedline, no coverage written.Root cause
Two installed packages publish a binary called
is-ci:The standalone
is-cipackage, pulled in transitively. Its bin is literally:It exits 0 in CI, 1 otherwise. Arguments are ignored.
Our
is-ci-clidevDep, which exposes bothis-ciandis-ci-clibinaries pointing at the samecli.js(the one that actually spawns the named npm script).When both are installed,
node_modules/.bin/is-ciwins for whichever package was linked last. In this repo it's the standalone package:So
"test": "is-ci 'test:ci' 'test:local'"invoked the dummy bin and silently succeeded. Theis-ci-cliREADME documents the correct invocation as the unambiguous binaryis-ci-cli-- using that name avoids the collision entirely.The fix
One word in
package.json:(Quotes also dropped to match the package's documented usage; no spaces in the script names.)
Local verification
Related
Unblocks #262 (codecov-action migration) -- once both merge, the new
coveragejob will actually have anlcov.infofile to upload.