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

tests with #[should_panic] should print a message on failure #60790

Closed
fbenkstein opened this issue May 13, 2019 · 8 comments · Fixed by #64745
Closed

tests with #[should_panic] should print a message on failure #60790

fbenkstein opened this issue May 13, 2019 · 8 comments · Fixed by #64745
Labels
A-libtest Area: #[test] related C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@fbenkstein
Copy link

When a #[should_panic] #[test] fails because there was no panic it should produce an error message to that effect.

For example, the following test:

#[test]
#[should_panic]
fn doesnt_actually_panic() {}

when run with cargo test produces the following output:

test doesnt_actually_panic ... FAILED

failures:

failures:
    doesnt_actually_panic

There is no indication why it failed. Which can be problematic if the test itself is created by a macro. Example: https://github.com/BurntSushi/fst/blob/master/src/raw/tests.rs#L131-L140

@jonas-schievink jonas-schievink added A-libtest Area: #[test] related C-enhancement Category: An issue proposing an enhancement or a PR with one. labels May 13, 2019
@varkor varkor added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label May 13, 2019
@varkor
Copy link
Member

varkor commented May 13, 2019

I'm not familiar with the code, but I'd guess that you just need to change TrFailed in here:
https://github.com/rust-lang/rust/blob/036e368d6883e0bc141c1b54578876cd5bfb2468/src/libtest/lib.rs#L1525-L1548
to TrFailedMsg("test did not panic as expected").
(And add a test to make sure it works.)

@chansuke
Copy link
Contributor

Thanks.I would like to work on this.

@varkor
Copy link
Member

varkor commented May 13, 2019

@chansuke: go ahead! If you have any questions, feel free to ask them here.

@chansuke
Copy link
Contributor

@varkor Hi.I've added the TrFailedMsg("test did not panic as expected") in fn calc_result() and trying to write a test but is this right approach?

    #[test]
    fn test_calc_result() {
        let args = // what should I write?
        let panic_message = "test did not panic as expected"
        let result = calc_result(args)
        assert_eq!(result, panic_message)
    }

@varkor
Copy link
Member

varkor commented May 18, 2019

@chansuke: actually, I think you can get away with just changing this line:

assert!(res == TrFailed);

to update it to your TrFailedMsg, because such a test already exists for this case.

@chansuke
Copy link
Contributor

@varkor Thanks

@varkor
Copy link
Member

varkor commented Aug 30, 2019

The previous fix was closed, so this issue is open again. The previous PR contains detailed instructions.

@kungfukennyg
Copy link
Contributor

kungfukennyg commented Aug 31, 2019

I'd like to finish this out if @chansuke is no longer working on it.

Centril added a commit to Centril/rust that referenced this issue Sep 27, 2019
…varkor

Include message on tests that should panic but do not

As per issue rust-lang#60790 includes a message for tests marked `#[should_panic]` that do not panic as expected.

Fixes rust-lang#60790.
Centril added a commit to Centril/rust that referenced this issue Sep 27, 2019
…varkor

Include message on tests that should panic but do not

As per issue rust-lang#60790 includes a message for tests marked `#[should_panic]` that do not panic as expected.

Fixes rust-lang#60790.
Centril added a commit to Centril/rust that referenced this issue Sep 28, 2019
…varkor

Include message on tests that should panic but do not

As per issue rust-lang#60790 includes a message for tests marked `#[should_panic]` that do not panic as expected.

Fixes rust-lang#60790.
@bors bors closed this as completed in a60ac8e Sep 28, 2019
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-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
5 participants