Skip to content

Allow message specification for should_fail #19560

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

Closed
wants to merge 3 commits into from

Conversation

sfackler
Copy link
Member

@sfackler sfackler commented Dec 5, 2014

The test harness will make sure that the panic message contains the
specified string. This is useful to help make #[should_fail] tests a
bit less brittle by decreasing the chance that the test isn't
"accidentally" passing due to a panic occurring earlier than expected.
The behavior is in some ways similar to JUnit's expected feature:
@Test(expected=NullPointerException.class).

Without the message assertion, this test would pass even though it's not
actually reaching the intended part of the code:

#[test]
#[should_fail(message = "out of bounds")]
fn test_oob_array_access() {
    let idx: uint = from_str("13o").unwrap(); // oops, this will panic
    [1i32, 2, 3][idx];
}

@sfackler
Copy link
Member Author

sfackler commented Dec 5, 2014

cc @steveklabnik - we talked about this on IRC yesterday.

@alexcrichton
Copy link
Member

Interesting! This was somewhat surprising to me as I thought this would be a custom message to print out on test failure rather than "this test failed" with no output from the test. The semantic meaning implemented here, however, I think makes more sense to me.

This is one of the major reasons I avoid should_fail tests as much as possible because I'm not actually sure that the part I wanted to fail actually failed, so this seems like a great thing to have. @sfackler do you think that my initial interpretation is a possible to make, or am I off my rocker? Perhaps we could get "expected" somewhere in the name?

@sfackler
Copy link
Member Author

sfackler commented Dec 6, 2014

Sure, changing message to expected seems reasonable.

The test harness will make sure that the panic message contains the
specified string. This is useful to help make `#[should_fail]` tests a
bit less brittle by decreasing the chance that the test isn't
"accidentally" passing due to a panic occurring earlier than expected.
The behavior is in some ways similar to JUnit's `expected` feature:
`@Test(expected=NullPointerException.class)`.

Without the message assertion, this test would pass even though it's not
actually reaching the intended part of the code:
```rust
 #[test]
 #[should_fail(message = "out of bounds")]
fn test_oob_array_access() {
    let idx: uint = from_str("13o").unwrap(); // oops, this will panic
    [1i32, 2, 3][idx];
}
```
@sfackler
Copy link
Member Author

sfackler commented Dec 6, 2014

@alexcrichton updated. Also added a bit to the testing docs.

@sfackler sfackler mentioned this pull request Dec 8, 2014
bors added a commit that referenced this pull request Dec 8, 2014
The test harness will make sure that the panic message contains the
specified string. This is useful to help make `#[should_fail]` tests a
bit less brittle by decreasing the chance that the test isn't
"accidentally" passing due to a panic occurring earlier than expected.
The behavior is in some ways similar to JUnit's `expected` feature:
`@Test(expected=NullPointerException.class)`.

Without the message assertion, this test would pass even though it's not
actually reaching the intended part of the code:
```rust
#[test]
#[should_fail(message = "out of bounds")]
fn test_oob_array_access() {
    let idx: uint = from_str("13o").unwrap(); // oops, this will panic
    [1i32, 2, 3][idx];
}
```
@bors bors closed this Dec 8, 2014
@sfackler sfackler deleted the should-fail-reason branch December 11, 2014 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants