reconsider "the way to run a crate's unit tests is `x test <crate>`"

[jyn514]

Summary

@ChrisDenton brought up to me the following thorny problem:

Say you are a libs contributor working on std. Right now, to run all relevant tests, the command you need is something like x test library tests/ui/std rustdoc-js-std. This is verbose and a little unreasonable to expect people to remember.

We would like to add an alias for this; x test std-tests could work. But now x test std does something people don't expect (only runs a subset of the std tests). If we change x test std to mean x test std-tests, then now we have an inconsistency between x test core, which only runs crate tests, and x test std, which means "run a superset of crate tests".

When I added unit tests in #95503, I wasn't really thinking about "run all the tests for an area", because we didn't have any equivalent of std-tests, then or now. But now that I've added it, the naming conflict is unfortunate. Maybe we should reconsider the naming here, and have the way to run unit tests for a crate be x test std-unit or something like that? Or only do this for library crates, and have x test rustc_resolve continue to mean "run the unit tests for rustc_resolve"? It could be nice to have x test rustc_resolve also imply x test tests/ui/resolve, though.

Command used

x test std
x test library
x test rustc_resolve

Expected behaviour

Actual behaviour

Bootstrap configuration (bootstrap.toml)

see above

Operating system

ubuntu 22.04, but this is a problem on all OSes

HEAD

a114bcf

[lolbinarycat]

I was just having this same thought recently, except with rustdoc, where the problem is even worse, due to how many test suites are involved.

[kpreid]

Speaking as an occasional contributor who hasn't learned all the corners of the codebase, it would be nice to have both of the following readily available:

• Run all tests in this directory
• Run all tests of the code in this directory

Perhaps exactly this distinction could be specified with a syntax, and an error produced when it's ambiguous:

./x test in:library/std   # explicit
./x test of:library/std   # explicit
./x test library/std      # error: ambiguous
./x test tests/ui/typeck  # unambiguous

[lolbinarycat]

I will say, one thing we do have, for some things at least, is (eg.) ./x test rustdoc. Running ./x test with the name of something, instead of the path to it, will run all relevant tests.

Not sure if this also applies to ./x test std.

[jyn514]

i am almost confident that x test rustdoc runs tests/ui/rustdoc, i. e. not all of rustdoc's tests.

[lolbinarycat]

it seems to run tests/rustdoc, at least. IMO it should run everything.

[Mark-Simulacrum]

Run all tests of the code in this directory

Presumably this means something closer to "some rough heuristic of tests intending to cover this directory"? E.g., std is indirectly tested by basically everything, even (say) UI tests, but it's probably unreasonable to run all of the UI tests as someone changing just std.

My sense is that heuristics are probably fine here, but we should be clear that's what we'd actually be aiming for, not some kind of "definitive truth". And then it may also make sense to have gradients (e.g., I have a good computer, run all of the tests, or "run minimum we can to get some confidence" for the less powerful computers).

FWIW, my personal workflow here is basically never running tests locally until they fail in CI - maybe there's more we can do to encourage that (e.g., dedicated builders for the fast profile of the tests per area that hopefully complete in <30 minutes for example).

[kpreid]

Presumably this means something closer to "some rough heuristic of tests intending to cover this directory"?

I see how you are saying this is ambiguous, and I can imagine coverage being the best option, but I actually did not mean that; I meant “the tests which, if this were a more conventional project made up of Cargo packages, would be located in this package”. Probably a coverage-oriented definition is more likely to be actually useful, though.

FWIW, my personal workflow here is basically never running tests locally until they fail in CI

I can’t imagine doing that myself, because if I can’t see whether tests pass, I often can’t see whether my idea is even feasible, or implemented close enough to the right way, to be worth posting as a PR.

[jyn514]

I don't think anyone expects "all tests that could possibly be affected by this code" to run when they say x test std. that's the bazel model, but we don't have bazel level caching, so it's too expensive for us (it would result in e.g. libs contributors running debuginfo tests on each change).

I think people are expecting "tests that are explicitly targeting std", which is ~roughly the list I put at the top (library/std, tests/ui/std, tests/rustdoc-js-std, probably a few but not all of the run-make tests). and that is certainly something we can do with a little manual work in bootstrap.

[jyn514]

FWIW, my personal workflow here is basically never running tests locally until they fail in CI

I can’t imagine doing that myself, because if I can’t see whether tests pass, I often can’t see whether my idea is even feasible, or implemented close enough to the right way, to be worth posting as a PR

+1 — I feel the same way about this as I do about IDE tooling, we should make it as good as we can, but we should not consider it a requirement for testing code at all