Conversation
… not exist bun test ./missing --pass-with-no-tests printed 'Test filter "./missing" had no matches' and exited 1. The single-path DoesNotExist branch exits before the flag is consulted, while two missing paths, an empty directory, and -t with no match all reach the shared no-tests path and exit 0 with the flag. Skip the early exit when the flag is set so the single-path case takes the same path.
|
Warning Review limit reached
On-demand reviews are free for the next 13 days. After that, they cost $0.25 per reviewed file. Or wait 21 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: ready for review. Reproduced on bun 1.4.3 in a project with With this change the first row is rc 0 and prints the same "The following filters did not match any test files:" report as the other two. Without the flag all three rows stay rc 1. CI (build 112639): every lane that runs this diff is green. The one red job is |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — one-clause guard change that makes the single-missing-path branch consistent with the multi-path fall-through it already sits next to.
What was reviewed:
- Traced the fall-through: with the flag set and
scan()returningDoesNotExist,test_filesstays empty, hits the shared "did not match any test files" report at ~2450, andshould_fail_on_no_testsat 2664 evaluates false → exit 0. - Confirmed no behavior change without the flag: the original error message and
global_exit()still fire whenpass_with_no_testsis false. - Tests cover both flag states across single missing path / multiple missing paths / empty dir, assert stderr before exit code, and live in the existing
pass-with-no-tests.test.ts.
Extended reasoning...
Overview
The PR touches two files: a one-line condition change in src/runtime/cli/test_command.rs (adding && !ctx.test_options.pass_with_no_tests to the guard on the single-path DoesNotExist early-exit branch), and three new test.each rows in the existing test/cli/test/pass-with-no-tests.test.ts. The fix aligns the single-missing-path case with the multi-path case, which already fell through to the shared "did not match any test files" reporting path that honors --pass-with-no-tests at line 2664.
Security risks
None. This is CLI exit-code behavior for the test runner when a user-supplied path does not exist. No parsing of untrusted data, no auth/crypto/permissions surface, no memory management changes. The only effect is that a run which previously exited 1 now exits 0 when the user explicitly opted into --pass-with-no-tests.
Level of scrutiny
Low. The change is a single boolean clause appended to an existing if guard, and the fall-through target is code that already handles this exact scenario for the len() > 1 case. I traced the control flow: when the guard is skipped, test_files is empty, pass_with_no_tests_from_filter is false, so failed_to_find_any_tests becomes true (printing the filter report), and should_fail_on_no_tests = !pass_with_no_tests && ... evaluates to false, yielding exit 0. Without the flag, the original branch is unchanged — same error message, same global_exit() with code 1.
Other factors
Tests are well-structured per repo conventions: added to the existing feature test file, use tempDir/bunExe/bunEnv, await using on spawned processes, assert stderr content before exit code, and cover the variant matrix (single path — the actual fix; multiple paths and empty dir — regression guards for already-working siblings) in both flag states. The new tests only drain stderr (not stdout) in Promise.all, but this matches every existing test in the file and bun test writes negligible stdout on the no-tests path, so there is no deadlock risk. No CODEOWNERS entry covers these paths. Exit reason was dry_streak with no findings and no outstanding reviewer objections.
|
Closing in favor of #44017. That PR removes the single-path early exit that this PR gates, so I built #44017 at d1b2306 and ran the three cases that this PR tests:
Each run names the path on stderr and runs no test. The exit codes are the same as with this PR. The message differs: #44017 prints one The tests of #44017 already have these three rows, with and without the flag ( Main still has the bug: If #44017 does not land, reopen this PR. It is a four-line change against main and it does not depend on #43361. |
2 participants
Problem
bun test ./does-not-exist --pass-with-no-testsprintsTest filter "./does-not-exist" had no matchesand exits 1. The same flag already gives exit 0 for two missing paths, for a directory with no test files, for-twith no match, and for an empty project. A monorepo step likebun test ./packages/$P --pass-with-no-testsfails for a package that has no tests yet, which is the main use of jest's--passWithNoTests.ScanError::DoesNotExistbranch insrc/runtime/cli/test_command.rs:2044. It sets exit code 1 and callsglobal_exit()before anything readsctx.test_options.pass_with_no_tests.Fix
--pass-with-no-testsis not set. With the flag, the single missing path falls through to the shared "The following filters did not match any test files:" report, which already honors the flag attest_command.rs:2662and exits 0.test/cli/test/pass-with-no-tests.test.ts(three newtest.eachrows, stock bun fails the single-path row). Also ran the scanner path cases intest/cli/test/bun-test.test.ts.Background
bun testtreats a positional that starts with./or../, or is absolute, as a path to scan. Any other positional is a substring filter over discovered test file paths. Only the path form can hitDoesNotExist.--pass-with-no-testsmaps toctx.test_options.pass_with_no_tests. It only flips the final exit code when no test ran. It does not suppress the report of what was searched.