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

Support cargo nextest #85

Closed
6 of 11 tasks
sourcefrog opened this issue Aug 22, 2022 · 19 comments · Fixed by #223
Closed
6 of 11 tasks

Support cargo nextest #85

sourcefrog opened this issue Aug 22, 2022 · 19 comments · Fixed by #223
Assignees
Labels
enhancement New feature or request

Comments

@sourcefrog
Copy link
Owner

sourcefrog commented Aug 22, 2022

https://nexte.st/ promises to be faster and in some ways nicer than cargo test on trees where it works.

  • Do cargo-mutants's own tests pass under nextest?
  • Add an option to use it instead of cargo test, probably a TestDriver or TestTool enum.
  • Let this option be set in config or on the command line.
  • Document the new option.
  • Integration test with nextest, probably off by default as it may not be installed.
  • Test PR mutants (and all mutants?) using nextest in cargo-mutants own CI.

Perhaps later:

Rather than a specific --nextest option, this perhaps should build on some general concept of configuring which tools to use, allied to #77. Although, it's an interesting edge case as it also has a lot in common with Cargo, and should use Cargo for package discovery, builds, etc.

Maybe later it would also be interesting to add a test tool that runs Miri.

@sourcefrog sourcefrog added the enhancement New feature or request label Aug 22, 2022
@sourcefrog
Copy link
Owner Author

cargo nextest run does pass in this tree, in 7.4s on my desktop vs 7.6s for cargo test.

So, not too noticeable. But this tree spends most of its time in one big cli test binary, which can already internally parallelize.

The output format is nice though.

@sourcefrog sourcefrog changed the title Try nextest Support cargo nextest Aug 22, 2022
@sourcefrog
Copy link
Owner Author

cc @sunshowers fyi

@sunshowers
Copy link

Thanks, that makes sense! If your tests are mostly all in one binary then nextest doesn't have the scope to do much better than cargo test. (Of course, you might want some of its other features like per-test process isolation.)

@dmoka
Copy link
Contributor

dmoka commented Aug 23, 2022

Cargo nextest is indeed a nice one!

We just have to be careful with its limitation that doctests are not supported by it. So we should document this if we add this feature.

Additonally, if we want to use it in the CI of cargo-mutants, then we would need to run cargo test --doc in a separate step. Anyway, I just ran through the code base and could not find any doctest, so we should be fine for now in this regard, I guess.

@sourcefrog
Copy link
Owner Author

On the whole, although nextest is pretty awesome I'm not sure it makes a lot of sense for cargo-mutants to specifically call it, so I'm going to close. If someone comes along later with a crate where nextest is dramatically faster for some crate then it might be worth using it to make mutation testing faster.

@sourcefrog
Copy link
Owner Author

I think the discussion in #186 shows Nextest could actually be pretty good to use to run tests on mutants, because it will exit early when tests fail. Also, there may be trees where the developers only use Nextest, and where cargo test won't run reliably.

@sourcefrog sourcefrog reopened this Dec 15, 2023
@kpreid
Copy link

kpreid commented Dec 16, 2023

Note that because Nextest runs every test in a separate process, it may be significantly slower in the case where tests use a costly lazy-static initializer (as I found out myself). Therefore, it definitely isn't always desirable to use Nextest.

@moodmosaic
Copy link

I’m very enthusiastic about this feature and its potential. If there’s any way to see it implemented sooner, that would be fantastic. 🚀

@sourcefrog
Copy link
Owner Author

I’m very enthusiastic about this feature and its potential. If there’s any way to see it implemented sooner, that would be fantastic. 🚀

Thanks for the sponsorship! I dropped you a mail.

@sunshowers
Copy link

I'm happy to help and/or add features to nextest as necessary, time permitting.

Since I last commented here, the main thing that's changed is that an increasing number of projects (e.g. wgpu, some of our projects at work, nextest itself) require nextest. Getting cargo-mutants working with them would be really nice.

sourcefrog added a commit that referenced this issue Jan 2, 2024
Fixes Support `cargo nextest` #85 (but still needs docs and an integration test)
@sourcefrog sourcefrog linked a pull request Jan 2, 2024 that will close this issue
4 tasks
sourcefrog added a commit that referenced this issue Jan 2, 2024
Fixes Support `cargo nextest` #85 (but still needs docs and an integration test)
@sourcefrog sourcefrog mentioned this issue Jan 2, 2024
4 tasks
@sourcefrog
Copy link
Owner Author

@moodmosaic @ASuciuX @wileyj You could try out the code in #223 if you like: to me that seems to run reliably and to find some potentially interesting coverage gaps in your tree.

I tried nextest's own tree as a different test case of something that needs to run under nextest, but it seems to have some unrelated issue, #225, which I'll look in to separately.

@sourcefrog
Copy link
Owner Author

I see there is also a library interface. Potentially we could call that rather than spawning a subprocess, but, I'm not sure if there would be any particular advantage... It would avoid some amount of process-startup overhead per test, which we could attempt to measure, although I would hope that at least on Linux that would be reasonably small compared to the overhead for the test itself.

@sourcefrog
Copy link
Owner Author

@sunshowers For mutation testing it's important to distinguish "failed to build" (unviable) from "tests failed" (caught the mutant). (It's not completely critical to distinguish them since neither is actionable by the test author, but it's nice.)

For cargo test, I just run cargo build --tests and cargo test separately.

Is cargo build --tests sufficient to build everything that nextest will need, or is there some other way to run the build separately? From the progress bar it does look like it sometimes does some additional build steps when nextest run is invoked.

I see there is archive but that seems like it would do too much; I don't need a separate archive.

@sunshowers
Copy link

Note: cargo build --tests is not the right command in either case, because it uses the dev profile while cargo test uses the test profile. The two are typically the same (test inherits from dev) but can be different in some cases. GitHub code search has several examples of setting profile.test.

The correct command is cargo test <package and binary selection args> --no-run, and this is what nextest uses as well.

For nextest: To run the build separately you can use cargo nextest run <package and binary selection args> --no-run, but it can be more elegant to use the exit code. Nextest has semantic exit codes, and test runs failing are always indicated by TEST_RUN_FAILED (code 100).

@sourcefrog
Copy link
Owner Author

Oh, good to know, thanks!

@sunshowers
Copy link

I see there is also a library interface.

Oh, didn't see this earlier. I'd strongly recommend using the nextest binary -- the library one isn't stable (each release bumps the major version) and you'll have to do a lot of work yourself (see https://github.com/nextest-rs/nextest/blob/main/cargo-nextest/src/dispatch.rs). I'd originally had a separation between nextest-runner and cargo-nextest because I did need it as a library at my last job, but no one uses the library interface now.

@sourcefrog
Copy link
Owner Author

That makes sense, thanks. I think the main thing I might want to do for richer control, but not right away, would be to give it clues about running tests early that we think we think might be most likely to surface a failure. But that can certainly wait, I'm not sure it would be useful or a priority, and if it does look useful perhaps we could add it in the CLI.

@sunshowers
Copy link

Yeah, I've been wanting to add that kind of test-reordering support anyway and would love to spec out a design.

@sourcefrog
Copy link
Owner Author

Oh right, I put some extremely early thoughts on that in #194.

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

Successfully merging a pull request may close this issue.

5 participants