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 death tests in libtest #32512

Open
gnzlbg opened this issue Mar 26, 2016 · 10 comments
Open

Support death tests in libtest #32512

gnzlbg opened this issue Mar 26, 2016 · 10 comments
Labels
A-libtest Area: #[test] related C-feature-accepted Category: A feature request that has been accepted pending implementation. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-testing-devex Relevant to the testing devex team (testing DX), which will review and decide on the PR/issue.

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Mar 26, 2016

I would like to be able to mark a test as a death test of some kind, like, running this code should trigger an abort (instead of just an unwind), an exit with a particular exit code, or an assert (in case assert doesn't unwind if, e.g. panic=abort).

This requires two fundamental ingredients, libtests needs to support:

  • spawning test in different processes (that is, process spawning/setup and teardown)
  • marking tests as death tests (and maybe indicate the type).

A way to mark a test as a death test is important, since in the case in which panic != abort one still wants all the non death tests to happen within the same process for speed, but libtest stills need to spawn different processes for the death tests only (so it needs a way to tell these apart).

For crates in which panic == abort, it would be nice if one could turn all tests into death tests at the crate level.

This issue is tangentially related to: rust-lang/rfcs#1513

Google Test is probably the most famous unit-testing framework that supports these although in C++ other frameworks do as well. It usually takes 3k LOC C++ code to implement a "portable" process spawning/set up/tear down mechanism for spawning death tests, so it is quite a bit of machinery that might better belong outside of rustc.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added A-libtest Area: #[test] related C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jul 24, 2017
@Mark-Simulacrum Mark-Simulacrum added C-feature-accepted Category: A feature request that has been accepted pending implementation. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Aug 10, 2017
@alexcrichton
Copy link
Member

alexcrichton commented Aug 10, 2017

In #43788 I had two possible solutions to the panic=abort problem, although only the former solves the "death test" problem

  • Instead of starting a thread for all tests we could instead start a process. This process could be a re-execution of the executable itself and we'd just do that once per test. The downside of this is that it may be super slow.
  • We could continue to start a thread for each test but override the panic handler for the process. This panic handler would do "test related things" and provide an opportunity for a "clean exit". The downside of this is that you wouldn't get a set of all tests that failed, just the first few (maybe)

@KizzyCode
Copy link

I think we definitively should address the "death test" problem – there are a lot of situations where programs call abort() directs (e.g. an external C library like libsodium or a custom memory allocator).

About the performance: wouldn't it be possible to limit the re-execution to "death tests" or [should_panic] tests in panic = abort scenarios?
And if those tests are slow – then so be it... I mean those tests are relatively rare and if the overhead is communicated clearly, I don't see any problem with it (unless we have a better alternative).

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jan 20, 2019

It's pretty easy to write your own custom_test_framework today that runs each test on its own process. If you need "death tests" for some of your tests, the easiest way would be to use such a test framework only for those, while continuing to use libtest for the rest.

@tmandry
Copy link
Member

tmandry commented Aug 17, 2019

I'm working on a change to libtest that will implement the first of @alexcrichton's solutions above, specifically to support panic=abort. It should be pretty easy to extend to allow proper death tests, though.

So far I have a working prototype that's around 150 lines of implementation code. My intention is to make it "opt-in," so you have to either compile with a special flag or supply an argument on the command line. (Not sure how I'm going to go about this yet. Ideally for me, we can supply an option while building libtest to enable this for all tests, because we want panic=abort in our whole build.)

cc @rust-lang/libs, WDYT? Would this be a welcome addition to libtest?

@alexcrichton
Copy link
Member

I would personally at least welcome a change to libtest to support panic=abort mode in libtest. Ideally this would all be automatically inferred in that the test crate would automatically switch to process-per-test if linked into an executable as panic=abort, continuing to use threads by default otherwise. Having a runtime switch to use one over the other also makes sense to me though!

@tmandry
Copy link
Member

tmandry commented Aug 19, 2019

Ideally this would all be automatically inferred in that the test crate would automatically switch to process-per-test if linked into an executable as panic=abort

I agree that this would be ideal. Any suggestions on how to do it?

The best way I can think of off the top of my head is choosing a mode at the time of building libtest. I'm not sure how to detect that a specific binary is linked as panic=abort.

@alexcrichton
Copy link
Member

Choosing when libtest is built is possible but probably not gonna work out well because we ship a prebuilt libtest which is always compiled as panic=unwind. I think though that fn main() for the test harness is always synthesized at the end crate, so we could synthesize a main function which passes a boolean to libtest. That'd all be unstable and not really usable for other crates, so no new extra surface area to stabilize!

@djrenren
Copy link
Contributor

djrenren commented Aug 27, 2019

The fn main() that's synthesized basically just calls the test_runner.

Currently, the compiler selects libtest's test_main_static as the test_runner. We could build another entrypoint called test_main_static_abort (or something less gross), and when the compiler sets the test_runner it inspects the panic mode and chooses accordingly.

@djrenren
Copy link
Contributor

I think for panic=abort we don't want to group tests at all by default. Because this would only report the first test failure as opposed to all. I think this is a good thing to put behind a runtime flag, for if you really need the performance.

@workingjubilee

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: #[test] related C-feature-accepted Category: A feature request that has been accepted pending implementation. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-testing-devex Relevant to the testing devex team (testing DX), which will review and decide on the PR/issue.
Projects
Status: No status
Development

No branches or pull requests

8 participants