Improving test coverage (as in, the amount of tests we manage to run)

[saethlin]

I recently adjusted the timeout logic to not include test build time in the 15 minute timeout. While this may be a bit unintuitive, our total lack of caching here means that if we don't do this, we see some crates time out without executing any code at all bevy-inspector-egui is one example. And quite a few crates have build times above 10 minutes (with --jobs=1), which is hardly giving their tests a fair chance to run.

But we have other much thornier problems with test coverage. For example, parking_lot_core builds in 10 seconds then spends the rest of its 15 minutes running its first integration test. So we should have some mechanism to time out individual tests. I looked into the unstable --ensure-time option in libtest and it looks like it is both broken and not what I want here.

But there are also crates like unicode-xid which experience blowup in the Stacked Borrows runtime because of borrow stacks associated with a static allocation. In that scenario, the initial memory footprint of the test runner will grow from test to test. Normally the Stacked Borrows memory overhead stays in check because all resources associate with an allocation are freed when the allocation is, but that's not the case with a large static that is oft-reborrowed in complex ways (like when doing a lot of binary searches).

Therefore, I think the best solution here is to run each test in its own process, and time them out individually. We can't use the -Cpanic=abort support in libtest, because the libtest code is run inside Miri and Miri does not have all the shims required to launch a subprocess and capture output. We also can't do this inside cargo-miri by running cargo test -- --list then cargo test -- --exact test_name because --list does not print a unique name that can be fed back to --exact. It might be this last issue that is the easiest to fix.

[oli-obk]

We could just do the latter for now. It's suboptimal, but should work well enough?

[oli-obk]

One funny thing we could try is give miri a tumour scheme where it just causes a panic at the next step if enough time has passed. That will be very random, but it would timeout individual tests

[saethlin]

I would prefer "give miri a timeout scheme", except that timing out the interpreter as a whole will produce a timeout if the test suite has been running too long, not an individual test. It might work to time out individual tests if we could launch tests as subprocesses.

[oli-obk]

Hmm yea, we can't detect individual test starts in a meaningful manner. Maybe we can convince upstream libtest to run a cfg(miri) extern function that does nothing but notify us to reset the panic timeout?

[RalfJung]

We also can't do this inside cargo-miri by running cargo test -- --list then cargo test -- --exact test_name because --list does not print a unique name that can be fed back to --exact. It might be this last issue that is the easiest to fix.

I was about to suggest this snippet but it looks like you already know it and it doesn't work well enough for your purposes?

[saethlin]

The problem is that doctests re-parse the command line arguments. Also if you want to run a doctest with a filter you always need to pass --doc which is just inconvenient.

So if you try something like this:

cargo test "src/value/mod.rs - value::Value::get (line 293)" --doc

the doctest runner splits the filter argument, and the single - as well as the (line end up matching every doctest, rendering the filter useless.

[RalfJung]

That sounds like a rustdoc bug? It certainly shouldn't do word splitting...

[saethlin]

🤷 https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Trying.20to.20individually.20run.20doctests/near/286643329

[sunshowers]

With cargo-nextest 0.9.27, you should be able to try out miri with nextest. Check out the --tool-config-file option which will let you specify defaults for cargo miri nextest run with miri-tools.

Happy to answer questions as they come up.

[saethlin]

I finally got to hacking together some code that can convert the output from nextest into reasonable HTML. The whole situation definitely needs some polish, but I think just nextest with a timeout set answers everything I wrote out in this issue.