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

Stabilize allow_fail test flag #46488

Closed
GuillaumeGomez opened this issue Dec 4, 2017 · 10 comments
Closed

Stabilize allow_fail test flag #46488

GuillaumeGomez opened this issue Dec 4, 2017 · 10 comments
Assignees
Labels
A-libtest Area: #[test] related B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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

@GuillaumeGomez
Copy link
Member

Implemented in #42219.

I'll make the stabilization.

@GuillaumeGomez GuillaumeGomez self-assigned this Dec 4, 2017
@TimNN TimNN added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Dec 5, 2017
@gnzlbg
Copy link
Contributor

gnzlbg commented Dec 12, 2017

So I just had the following problem with #[allow_fail].

I have a test that should raise a cpu exception (e.g. segfault) and fail, so from the docs it looked like #[allow_fail] was exactly what I needed.

This does not work: the test segfaulting brings the whole test harness down (I assume that a test aborting the process with an error code also brings the whole test harness down).

In hindsight, after having read the source code, it is clear what is going on, but users should not have to read the source code to know what is meant by "fail" (it is a pretty general term).

As I see it the best solution would be to rename #[allow_fail] to #[allow_assert_fail] or similar and to document in the docs exactly which kind of "failures" are supported.

An alternative that I would prefer more (since it would cover my use case) but involves significant work that will delay stabilization would be to allow more kinds of failures by spawning the #[allow_fail] tests on a different process and documenting exactly which kind of failures are allowed.

@jonhoo
Copy link
Contributor

jonhoo commented Dec 12, 2017

@gnzlbg interesting... That's almost more in the domain of #[should_panic], but even that wouldn't be enough to handle a cpu exception. I can see how the naming of #[allow_fail] might make you think that it could work for your use-case though. I don't love #[allow_assert_fail], as it's fairly long, and prone to misremembering. I also don't know that #[allow_fail] currently only allows fails as a result of calls to assert*! macros (maybe someone can clarify).

@nrc
Copy link
Member

nrc commented Jan 18, 2018

Discussed at the dev-tools meeting today: we plan to support this via external test frameworks, and so will not stabilise this. It will be deprecated and removed once such test frameworks exist.

@killercup
Copy link
Member

Notes from meeting: macro_rules! possibly_failing_test { ($name:ident, $body:expr) => { #[test] fn $name() { let _ = ::std::panic::catch_unwind(|| $body ); } } } is probably a stable alternative.

@jonas-schievink jonas-schievink added A-libtest Area: #[test] related T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Nov 26, 2019
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 31, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Jan 26, 2022

This hasn't gone anywhere in many years, with no clear use cases.

A new similar feature could probably use a RFC with clear use cases and a discussion of all possible solutions.

@rfcbot close

@rfcbot
Copy link

rfcbot commented Jan 26, 2022

Team member @m-ou-se has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Jan 26, 2022
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jan 26, 2022
@rfcbot
Copy link

rfcbot commented Jan 26, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@U007D
Copy link

U007D commented Feb 1, 2022

Discussed at the dev-tools meeting today: we plan to support this via external test frameworks... It will be deprecated and removed once such test frameworks exist.

For anyone reading this issue looking for a solution, crates such as trybuild now exist.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Feb 5, 2022
@rfcbot
Copy link

rfcbot commented Feb 5, 2022

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Feb 5, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Feb 10, 2022
@JohnTitor
Copy link
Member

Triage: the flag has been removed by #93416, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: #[test] related B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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
None yet
Development

Successfully merging a pull request may close this issue.