Skip to content

Disable empty Cargo test targets#21584

Merged
charliemarsh-oai merged 1 commit into
mainfrom
charlie/shear
May 7, 2026
Merged

Disable empty Cargo test targets#21584
charliemarsh-oai merged 1 commit into
mainfrom
charlie/shear

Conversation

@charliemarsh-oai
Copy link
Copy Markdown
Contributor

Summary

cargo test has entails both running standard Rust tests and doctests. It turns out that the doctest discovery is fairly slow, and it's a cost you pay even for crates that don't include any doctests.

This PR disables doctests with doctest = false for crates that lack any doctests.

For the collection of crates below, this speeds up test execution by >4x.

E.g., before this PR:

Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
  Time (mean ± σ):      1.849 s ±  4.455 s    [User: 0.752 s, System: 1.367 s]
  Range (min … max):    0.418 s … 14.529 s    10 runs

And after:

Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
  Time (mean ± σ):     428.6 ms ±   6.9 ms    [User: 187.7 ms, System: 219.7 ms]
  Range (min … max):   418.0 ms … 436.8 ms    10 runs

For a single crate, with >2x speedup, before:

Benchmark 1: cargo test -p codex-utils-string
  Time (mean ± σ):     491.1 ms ±   9.0 ms    [User: 229.8 ms, System: 234.9 ms]
  Range (min … max):   480.9 ms … 512.0 ms    10 runs

And after:

Benchmark 1: cargo test -p codex-utils-string
  Time (mean ± σ):     213.9 ms ±   4.3 ms    [User: 112.8 ms, System: 84.0 ms]
  Range (min … max):   206.8 ms … 221.0 ms    13 runs

Co-authored-by: Codex <noreply@openai.com>
@charliemarsh-oai charliemarsh-oai requested a review from a team as a code owner May 7, 2026 20:52
@zanie-oai
Copy link
Copy Markdown
Contributor

We might need to write some sort of linter to enforce this, e.g., if you add a doctest to one of these crates then you might expect it to run in CI but it won't :/

@charliemarsh-oai
Copy link
Copy Markdown
Contributor Author

cargo-shear does this if you deny warnings; I just wanted to separate it into an independent PR.

@zanie-oai
Copy link
Copy Markdown
Contributor

Great work, boss.

@bolinfest
Copy link
Copy Markdown
Collaborator

@charliemarsh-oai I'm supportive, but FYI, we have this script that is run as part of CI:

https://github.com/openai/codex/blob/main/.github/scripts/verify_cargo_workspace_manifests.py#L19

to ensure that all Cargo.toml files in the Cargo workspace are uniform (with carve-outs for exceptions, where appropriate).

So whatever behavior you want to enforce, can you please codify there? It would be nice to ensure we are setting doctest = false when there are no doctests (though honestly, we have never encouraged them on this project...).

FYI, just test runs cargo nextest instead of cargo test because nextest seemed to have some perf wins.

@charliermarsh
Copy link
Copy Markdown

@bolinfest -- Thanks! I know we run cargo shear in CI -- if we upgrade to --deny-warnings, then cargo shear will enforce this for us (i.e., require that we set doctest = false when doctests are absent and vice versa). I was planning to do that in a separate PR since it entails some additional churn.

@bolinfest
Copy link
Copy Markdown
Collaborator

@charliemarsh-oai ah, thanks for the additional context: approving!

@charliemarsh-oai charliemarsh-oai merged commit 54ef99a into main May 7, 2026
26 checks passed
@charliemarsh-oai charliemarsh-oai deleted the charlie/shear branch May 7, 2026 22:44
@github-actions github-actions Bot locked and limited conversation to collaborators May 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants