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

panic=abort testing / subprocess testing #67650

Open
Mark-Simulacrum opened this issue Dec 27, 2019 · 9 comments
Open

panic=abort testing / subprocess testing #67650

Mark-Simulacrum opened this issue Dec 27, 2019 · 9 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Dec 27, 2019

This is the tracking issue for -Zpanic-abort-tests and related functionality.

Specifically, this flag switches the default test strategy of (loosely) a thread per test to a process per test, and implements the associated functionality of collecting process failures and treating that as passing/failing the test. It also includes a currently unstable option in test binaries, --force-run-in-process.

This was originally added in #64158 and Cargo support landed in rust-lang/cargo#7460.

@Mark-Simulacrum Mark-Simulacrum added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-libtest Area: #[test] related C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Dec 27, 2019
@Mark-Simulacrum
Copy link
Member Author

cc @tmandry

I would like to suggest that we change the flag to something like -Zprocess-tests or something along those lines, since AFAICT this functionality should be much more general (e.g., supporting #32512, #47506).

It's also currently a CLI flag but I would hope that we could also make this a per-test decision in panic=unwind binaries (with an attribute on the tests); panic=abort binaries would unconditionally apply it to all tests I guess.

It'd also be good to write up some docs on how to use this feature.

@tmandry
Copy link
Member

tmandry commented Jan 2, 2020

I don't have any problem with generalizing this feature. Using -Zpanic-abort-tests is still something we haven't fully turned on by default in Fuchsia, but I'm actively working on that. I think that's a good way to uncover any issues with this approach (e.g. performance).

A related (but separate) test attribute is disallowing certain tests from being run in parallel with others. This request has come up in Fuchsia before. I believe the test harness already does this with benchmark tests.

@euclio
Copy link
Contributor

euclio commented Jan 3, 2020

This would also allow better output capturing (#42474).

@tmandry
Copy link
Member

tmandry commented Jun 11, 2020

Update: Fuchsia has been using this feature for months now. There is one outstanding issue: #68936.

We should think about stabilizing an MVP of this feature, which is that tests work automatically when -Cpanic=abort is used. @rust-lang/libs, are there any other process steps that need to be taken first?

@dtolnay
Copy link
Member

dtolnay commented Jun 13, 2020

We should think about stabilizing an MVP of this feature, which is that tests work automatically when -Cpanic=abort is used.

I would be on board with this change. I guess send a stabilization PR?

@ehuss
Copy link
Contributor

ehuss commented Jun 21, 2020

cc #73509. There's an issue with -Zpanic_abort_tests if the library has any #[bench] functions. The current implementation panics here because it tries to run the benchmark out-of-process.

It looks like for normal benchmark mode (--bench), that it just runs all benchmarks in-process. Perhaps in normal test mode, it should also run "run once" #[bench] functions in-process?

@Stargateur
Copy link
Contributor

I come to an interesting problem about abort too, https://stackoverflow.com/questions/66750987/how-to-handle-abort-in-test-unit.

Could we have a #[could_abort] that ask to run a test in a process ? Or is there a better way to have a test that can abort ?

@DCNick3
Copy link

DCNick3 commented Feb 15, 2022

nextest seems to use separate processes for each test and (usually) can be used as a drop-in replacement for cargo test

@djkoloski
Copy link
Contributor

djkoloski commented Nov 8, 2022

It looks like there are two main issues blocking stabilization for -Zpanic_abort_tests so we can run tests under panic=abort:

Once we solve these two outstanding issues, it seems like we'd be ready to stabilize this. Are there any other issues I'm missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
Status: Unstable, no backers
Development

No branches or pull requests