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

Adjust design of unit tests with non-() type #49909

Closed
nikomatsakis opened this issue Apr 12, 2018 · 2 comments
Closed

Adjust design of unit tests with non-() type #49909

nikomatsakis opened this issue Apr 12, 2018 · 2 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Per this comment, we want to alter the design of unit tests with non-() type. The new design is:

  • #[should_panic] can only be applied to tests with () return type and continues to have the same meaning.
  • For non-should-panic tests, the return type T defines the error result using the Termination trait.

Achieving these changes should be relatively straight-forward. Basically the only thing we have to do is to make it an error to apply #[should_panic] unless the test has (). Currently we check that the test has the correct signature here:

rust/src/libsyntax/test.rs

Lines 330 to 355 in 9afed64

fn has_test_signature(cx: &TestCtxt, i: &ast::Item) -> HasTestSignature {
match i.node {
ast::ItemKind::Fn(ref decl, _, _, _, ref generics, _) => {
// If the termination trait is active, the compiler will check that the output
// type implements the `Termination` trait as `libtest` enforces that.
let output_matches = if cx.features.termination_trait_test {
true
} else {
let no_output = match decl.output {
ast::FunctionRetTy::Default(..) => true,
ast::FunctionRetTy::Ty(ref t) if t.node == ast::TyKind::Tup(vec![]) => true,
_ => false
};
no_output && !generics.is_parameterized()
};
if decl.inputs.is_empty() && output_matches {
Yes
} else {
No
}
}
_ => NotEvenAFunction,
}
}

In any case, we just have to extend that function to also check whether there is a should_panic attribute -- if so, we can report an error, sort of like this:

diag.span_err(i.span, "functions used as tests must have signature fn() -> ()");

@nikomatsakis nikomatsakis added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 12, 2018
@rcoh
Copy link
Contributor

rcoh commented Apr 12, 2018

I'm going to try this right now 👍

@nikomatsakis
Copy link
Contributor Author

Implemented in #49911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants