Repository navigation
reconsider "the way to run a crate's unit tests is x test <crate>" #140478
Description
Activity
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.
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 # unambiguousReacted by lolbinarycatI will say, one thing we do have, for some things at least, is (eg.)
./x test rustdoc. Running./x testwith 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.i am almost confident that
x test rustdocrunstests/ui/rustdoc, i. e. not all of rustdoc's tests.it seems to run
tests/rustdoc, at least. IMO it should run everything.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).
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.
Reacted by lolbinarycatI 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.
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
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-testscould work. But nowx test stddoes something people don't expect (only runs a subset of the std tests). If we changex test stdto meanx test std-tests, then now we have an inconsistency betweenx test core, which only runs crate tests, andx 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 bex test std-unitor something like that? Or only do this for library crates, and havex test rustc_resolvecontinue to mean "run the unit tests for rustc_resolve"? It could be nice to havex test rustc_resolvealso implyx test tests/ui/resolve, though.Command used
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