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 panic=abort without #[should_panic] from libtest #62318

Closed
tmandry opened this issue Jul 2, 2019 · 5 comments
Closed

Support panic=abort without #[should_panic] from libtest #62318

tmandry opened this issue Jul 2, 2019 · 5 comments
Labels
A-libtest Area: #[test] related C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@tmandry
Copy link
Member

tmandry commented Jul 2, 2019

When a project wants to turn on panic=abort today, they are forced to choose between

  1. Re-compiling their whole project with panic=unwind for the purpose of running tests (this not only creates longer build times, but also changes the behavior of code between testing and production)
  2. Using a custom test harness (either ad-hoc which requries changes the source code for tests, or by using custom_test_harness which is unstable)

Neither of these solutions are great. One solution that might be palatable is to support compiling panic=abort from libtest, but not support #[should_panic] tests in this mode. This could be advertised with an explicit error message anytime a test with #[should_panic] is run. Later, these could be supported as "death tests" as discussed in #32512.

In Fuchsia we'd really like to switch to panic=abort, and this is the main thing preventing us from doing so.

cc @cramertj @petrhosek

@jonas-schievink jonas-schievink added A-libtest Area: #[test] related C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 2, 2019
@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 3, 2019
@cramertj
Copy link
Member

cramertj commented Jul 4, 2019

@Centril IMO this is something of a libs question since it interacts with the behavior of libtest and custom test harnesses.

@Centril
Copy link
Contributor

Centril commented Jul 5, 2019

@cramertj libtest seems primarily under under T-dev-tools right now and #[should_panic] is an attribute known by the language (hence T-lang), and it is documented as such in the reference. Custom test frameworks are also not a libs team issue as far as I know.

@Centril Centril added the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Jul 5, 2019
@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 17, 2019

We already have custom test frameworks that run each test in a single process (e.g. this one but it is easy to write your own), and those solve more problems than the alternative approaches discussed here (e.g. they trivially support #![should_panic] test for panic=abort which are essentially death tests).

The only downside I see mentioned for this approach is that custom_test_frameworks are unstable. It might be better to work towards stabilizing these instead of making libtest more complicated to partially-support panic=abort, but not quite.

Custom test frameworks is one of those things that has been started, is almost there, but not quite yet, and finishing those things is part of this years roadmap.

@tmandry
Copy link
Member Author

tmandry commented Aug 26, 2019

Agreed that partial support isn't really the way to go. I'm working on fully supporting panic=abort from libtest (see #32512 (comment)).

I also think stabilizing custom test frameworks would be nice, but I think panic=abort should be fully supported without one.

@tmandry
Copy link
Member Author

tmandry commented Aug 14, 2020

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

#67650 tracks the unstable feature.

@tmandry tmandry closed this as completed Aug 14, 2020
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-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants