[dfrankland]

What

Part of TRUNK-19074. This covers the action-only parts, and supersedes #146, whose wording change is folded in. The certain "failed without running tests" message needs a signal from the CLI first; see "Not in this PR".

A failed test step no longer means failing tests (from #146)

When the step that ran the tests failed, the annotation said "The test results you are uploading contain non quarantined test failures". That is wrong whenever the step failed without producing any failures: no test files matched, a build or setup error, or a crash before results were written. The annotation is the only part of a failure GitHub shows without opening the log. The action knows the step's outcome but never the cause, so the message now says only that:

The step that ran the tests failed -- see above for details. If no failing tests are listed, it failed without reporting any (for example: no test files matched, or it exited before writing results).

A cancelled step gets its own wording rather than being called a failure:

The step that ran the tests was cancelled -- the uploaded results are likely incomplete.

This is @EliSchleifer's change from #146, credited as co-author on the commit.

Quieter setup logging (ask #3)

These lines now go to core.debug, or to core.info when verbose: true. The new src/log.ts handles that.

• Downloading trunk-analytics-cli from <url>... and Downloaded <artifact> from release <version>
• Extracting <path> to <path>... and Extraction complete. tar now runs without -v, so the file list is gone.
• Checking trunk-analytics-cli version... and the --version output
• The cache hit/save lines (Cache restored with key, Binary found in cache, Cache saved with key). The ticket was filed before feat: download trunk-analytics-cli from trunk.io (GitHub as fallback), cache by default #151 turned caching on by default, and these lines are the same kind of noise.

Resolved "latest" to version: X stays at info, so the log still says which CLI version ran. Every failure on these paths is still a core.warning or setFailed.

getFailureReason misread the exit code

It looked for exit code 70 in error.message. Node's execSync never puts the exit code there; the message is just Command failed: <command>, and the code is on error.status. Two consequences:

• Wrong message: when the CLI failed on its own (exit 70: a bad token, an unreadable repo, or no results with allow-missing-junit-files: false) after a failed test step, the action blamed the test step. It now says "A failure occurred while executing the command".
• Missing telemetry: exit 70 was never reported to telemetry. It is now, as the fixed string Command failed with exit code 70. error.message holds the whole command line, --token included, so it is not sent.

Tests

errors.test.ts mocked @actions/core with jest.mock, which does nothing under ESM, so the test never saw the fixture. It now uses jest.unstable_mockModule, like the other suites, and checks the setFailed message in each case. There are new cases for exit 70 after a failed test step, and for a cancelled step. Two tests in download.test.ts cover the setup lines: hidden by default, shown with verbose: true.

Not in this PR

The ticket's copy, "testing step appears to have failed without running tests", states the cause with certainty, so it needs the CLI to say how many tests it saw. Until then, the hedged message above covers it. My plan is to read summary.total from the summary JSON file (trunk-io/analytics-cli#1122, TRUNK-18881) rather than add a new exit code. The CLI's exit code is the test process's code passed through, so changing it would affect people who run the CLI directly. Once the CLI side lands, a follow-up can replace the hedge with the certain message whenever summary.total is 0.

Testing

• jest: 65 tests pass.
• tsc --noEmit, eslint src __tests__ and prettier --check src __tests__: clean.
• dist/ is rebuilt with ncc build -m src/index.ts -o dist. That's the hook's build step; I committed with --no-verify because trunk's pre-commit hook couldn't find pnpm in my environment.

🤖 Generated with Claude Code

[trunk-staging-io]

   

View Full Report ↗︎ ⋅ Docs

[dfrankland]

why not use instanceof?

[dfrankland]

There isn't a class to check. execSync throws a plain Error with status, signal, stdout and stderr attached, and ExecException in @types/node is only an interface, so there's nothing to instanceof against. Switched to "status" in error && typeof error.status === "number" instead, which narrows without the cast.

[dfrankland]

Do we need such verbose comments? I think we can cut down or remove some of this. Just explain unobvious "why"s instead of narrating

[dfrankland]

Cut them down to one line each, keeping only the non-obvious whys: what exit 70 means, why getExitCode doesn't parse the message, why the message can't go to telemetry, and why the test-step copy is hedged. Also trimmed the ones in log.ts and errors.test.ts.