Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/Support for miri? #181

Closed
NobodyXu opened this issue May 1, 2022 · 5 comments
Closed

feature/Support for miri? #181

NobodyXu opened this issue May 1, 2022 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@NobodyXu
Copy link
Contributor

NobodyXu commented May 1, 2022

Hi

First, thank you for this awesome project!

Using nextest in medium size projects such as compact_str and bytes result in significant speedup.

It will be great if we can also run miri using nextest, as miri is very slow at running tests and is single-threaded.

@sunshowers
Copy link
Member

Thanks for the report! I think support for nextest should be added to cargo miri itself, and then we should figure out what nextest needs to provide to work with miri.

@sunshowers sunshowers added enhancement New feature or request help wanted Extra attention is needed labels May 4, 2022
@eddyb
Copy link

eddyb commented Jun 7, 2022

Another miri usecase (described to me by @BoxyUwU) where nextest could come in handy, is having many tests where some of them are leaking memory, which makes it hard to know what test it was (at least in the absence of tagging the allocation with the thread it originated on and mapping that back).

Looking at how cargo miri describes its own operation, my understanding is that:

  • it mostly treats cargo test as a strange flavor of cargo run
  • it uses RUSTC_WRAPPER and CARGO_TARGET_<triple>_RUNNER to customize both "build" and "run" aspects of cargo run (and therefore cargo test as well)
  • the actual "executable" that Cargo just assumes is in the native format for the target, is instead a JSON file describing all the information Cargo passes to the "build" step (like rustc flags)

From a quick check, cargo miri test --lib -- ... behaves identically to cargo test --lib -- ... (with e.g. --list), so anything designed around invoking cargo test would also work with cargo miri test.

For example, I came up with this quick hack to help @BoxyUwU out:

cargo --quiet test --lib -- --list | sed 's/: test$//' | xargs -n1 cargo miri test --lib -- --exact

But that's much more fragile than what nextest does and AFAIK nextest invokes the test runner executable itself, so nextest going through cargo miri test doesn't sound promising.


Instead, since nextest respects CARGO_TARGET_<triple>_RUNNER, it should be possible to do this from the cargo-miri side, specifically adding a cargo miri nextest subcommand (it would go in this part of the source code) that behaves like cargo miri test but running cargo nextest instead of cargo test (not sure how the cargo nextest subcommand passing would work, but to cargo-miri it might just be freeform enough to not matter).

In fewer words, I agree with this:

I think support for nextest should be added to cargo miri itself

and also think it would take very little effort (assuming no weird Cargo features get in the way)

(@oli-obk, @RalfJung, or someone else who works on miri may also want to chime in)

@RalfJung
Copy link
Contributor

RalfJung commented Jun 7, 2022

Your summary of cargo miri operations sounds accurate. :)

bors added a commit to rust-lang/miri that referenced this issue Jul 21, 2022
[cargo-miri] support nextest

Add the ability to run `cargo miri nextest list` and `cargo miri nextest run`.

[cargo-nextest](https://nexte.st) is a new test runner for Rust maintained mostly by myself. It has several new features, but the most relevant to miri is the fact that it runs [each test in its own process](https://nexte.st/book/how-it-works.html#the-nextest-model). This gives miri users better leak detection (#1481) for free, for example.

See nextest-rs/nextest#181 for discussion, including comments by `@eddyb` and `@RalfJung.`

Future work might be to have miri read [the list of tests](https://docs.rs/nextest-metadata/latest/nextest_metadata/struct.TestListSummary.html) (or [test binaries](https://docs.rs/nextest-metadata/latest/nextest_metadata/struct.BinaryListSummary.html)) generated by `nextest list`. `@eddyb` thinks that might be useful.

I tested `cargo miri nextest run` against smallvec, and it worked great.

Note: Running tests out of archives is currently broken, as the comment in run-test.py explains.
@sunshowers
Copy link
Member

Just released cargo-nextest 0.9.27 with support for Miri. https://nexte.st/book/miri.html

@RalfJung
Copy link
Contributor

Awesome. :)
The next nightly that ships Miri should have the required support for this. (The currently latest nightly with Miri does not have it yet.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants