Skip to content

fix: automatically enable coverage when --coverage-reporter is specified - #17737

Closed
Electroid wants to merge 1 commit into
mainfrom
issue-17502-coverage-reporter-enables-coverage
Closed

Electroid wants to merge 1 commit into
mainfrom
issue-17502-coverage-reporter-enables-coverage

Conversation

@Electroid

Copy link
Copy Markdown
Contributor

Description

This PR automatically enables coverage when `--coverage-reporter` is specified, improving the user experience by not requiring an explicit `--coverage` flag.

Changes

  • Modified `src/cli.zig` to automatically enable coverage when a coverage reporter is specified
  • Added a regression test to verify this functionality

Related Issues

Fixes #17502

Testing

  • Verified that coverage is automatically enabled when a coverage reporter is specified
  • Added regression test that confirms the expected behavior

@robobun

robobun commented Feb 26, 2025 •

Copy link
Copy Markdown
Collaborator
Updated 4:42 PM PT - Feb 26th, 2025

❌ @Electroid, your commit 0410685 has 4 failures in Build #12344:


🧪   try this PR locally:

bunx bun-pr 17737

Fixes #17502

This change ensures that when a user specifies --coverage-reporter, coverage is automatically enabled, improving the user experience by not requiring an explicit --coverage flag.
@Electroid
Electroid force-pushed the issue-17502-coverage-reporter-enables-coverage branch from 44aa307 to 0410685 Compare February 26, 2025 23:50
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

I think the 17502 test is real

@Electroid Electroid closed this Jun 30, 2025
Jarred-Sumner pushed a commit that referenced this pull request Oct 3, 2026
Fixes #17502

### Problem
- `bun test --coverage-reporter=lcov` runs the tests, exits 0, and
writes no report. It prints no warning.
- The `--coverage-reporter` block of `parse_test_command_options`
(`src/runtime/cli/Arguments.rs:1798`) sets `coverage.reporters` and
never `coverage.enabled`.

### Fix
- That block now sets `coverage.enabled = true`. The help text and
`docs/snippets/cli/test.mdx` say `Implies --coverage`.
- `--coverage-dir` and the bunfig key `coverageReporter` do not change
(see Notes).
- Verified: `test/cli/test/coverage.test.ts`, 5 new tests (lcov, the
separate-argument form, text, both reporters, `--parallel=2`). The
released build fails 5 of 5. A debug build with the change passes them,
and the whole file passes (32 tests).

### Background
- `bun test` collects coverage only when `coverage.enabled` is true.
`--coverage` or `coverage = true` in `bunfig.toml` sets it.
- A `--parallel` coordinator starts its workers with `--coverage` when
`coverage.enabled` is true
(`src/runtime/cli/test/parallel/runner.rs:422`), so the same switch
covers that mode.
- No other place was weighed: one block parses the flag. #20736 and
#32420 made the same change, and a review of #32420 asked for the
reporter half only.

### Downsides
- A run with `--coverage-reporter` and no `--coverage` now collects
coverage. It is slower, it prints the table or writes
`coverage/lcov.info`, and a `coverageThreshold` in `bunfig.toml` can now
fail it.
- `--coverage-dir` with neither flag is still dropped with no message.
- Other runs pay nothing: one assignment at argument parsing, only when
the flag is present. The help text grows by 20 bytes.

<details><summary>Notes</summary>

**Rows, before and after.** A directory with `f.ts` and `f.test.ts`:

| command | released build | with the change |
| --- | --- | --- |
| `bun test --coverage` | text table | text table |
| `bun test --coverage --coverage-reporter=lcov` | `coverage/lcov.info`
| `coverage/lcov.info` |
| `bun test --coverage-reporter=lcov` | nothing | `coverage/lcov.info` |
| `bun test --coverage-reporter=text` | nothing | text table |
| `bun test --coverage-dir=cov2 --coverage-reporter=lcov` | nothing |
`cov2/lcov.info` |
| `bun test` | nothing | nothing |

**Why `--coverage-dir` stays.** Only the lcov writer reads the directory
(`src/runtime/cli/test_command.rs:1631-1681`). `bun test
--coverage-dir=out` alone would print the text table and never create
`out/`. The comparable flags `--cpu-prof-dir` and `--heap-prof-dir` do
not turn their switch on. They fail with `must be used with --cpu-prof`
(`Arguments.rs:1392`, `:1449`). Whether `--coverage-dir` alone must be
an error is an open question.

**Why the bunfig key stays.** A reporter that is set in `bunfig.toml`
must not turn coverage on for each `bun test` run. `coverage = true` is
the key for that.

**`bunfig.toml` wins over the flags today.** With `coverage = false` in
`bunfig.toml`, `--coverage` does not turn coverage on, and
`--coverage-reporter` does not either. With `coverageReporter = "lcov"`
there, `--coverage-reporter=text` writes `lcov.info`. #40348 (open)
changes that order and edits the same function.

**Threshold.** With `coverageThreshold = 1.0` and one function that no
test calls: `bun test` exits 0, `bun test --coverage` exits 1, and `bun
test --coverage-reporter=lcov` now exits 1 too.

**Earlier pull requests.** #17737, #20736, #32420. The last one was
closed by a cleanup of stale pull requests, with no objection to the
change.

</details>
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.

bun test --coverage-reporter does not imply --coverage

3 participants