From d6689fdc616fd3e7af9946072cb0aaceb2bb26b9 Mon Sep 17 00:00:00 2001 From: Josh Driver Date: Sun, 13 Nov 2016 19:39:27 +1030 Subject: [PATCH 1/2] Add warnings when the #[should_panic] attribute is invalid --- src/libsyntax/test.rs | 44 ++++++++++++++++---- src/test/run-pass/test-should-panic-attr.rs | 46 +++++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 src/test/run-pass/test-should-panic-attr.rs diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs index 618878c1f7980..59a7e75d12557 100644 --- a/src/libsyntax/test.rs +++ b/src/libsyntax/test.rs @@ -132,7 +132,7 @@ impl<'a> fold::Folder for TestHarnessGenerator<'a> { path: self.cx.path.clone(), bench: is_bench_fn(&self.cx, &i), ignore: is_ignored(&i), - should_panic: should_panic(&i) + should_panic: should_panic(&i, &self.cx) }; self.cx.testfns.push(test); self.tests.push(i.ident); @@ -395,14 +395,44 @@ fn is_ignored(i: &ast::Item) -> bool { i.attrs.iter().any(|attr| attr.check_name("ignore")) } -fn should_panic(i: &ast::Item) -> ShouldPanic { +fn should_panic(i: &ast::Item, cx: &TestCtxt) -> ShouldPanic { match i.attrs.iter().find(|attr| attr.check_name("should_panic")) { Some(attr) => { - let msg = attr.meta_item_list() - .and_then(|list| list.iter().find(|mi| mi.check_name("expected"))) - .and_then(|li| li.meta_item()) - .and_then(|mi| mi.value_str()); - ShouldPanic::Yes(msg) + let sd = cx.span_diagnostic; + if attr.is_value_str() { + sd.struct_span_warn( + attr.span(), + "attribute must be of the form: \ + `#[should_panic]` or \ + `#[should_panic(expected = \"error message\")]`" + ).note("Errors in this attribute were erroneously allowed \ + and will become a hard error in a future release.") + .emit(); + return ShouldPanic::Yes(None); + } + match attr.meta_item_list() { + // Handle #[should_panic] + None => ShouldPanic::Yes(None), + // Handle #[should_panic(expected = "foo")] + Some(list) => { + let msg = list.iter() + .find(|mi| mi.check_name("expected")) + .and_then(|mi| mi.meta_item()) + .and_then(|mi| mi.value_str()); + if list.len() != 1 || msg.is_none() { + sd.struct_span_warn( + attr.span(), + "argument must be of the form: \ + `expected = \"error message\"`" + ).note("Errors in this attribute were erroneously \ + allowed and will become a hard error in a \ + future release.").emit(); + ShouldPanic::Yes(None) + } else { + ShouldPanic::Yes(msg) + } + }, + } } None => ShouldPanic::No, } diff --git a/src/test/run-pass/test-should-panic-attr.rs b/src/test/run-pass/test-should-panic-attr.rs new file mode 100644 index 0000000000000..2d068872a4d3d --- /dev/null +++ b/src/test/run-pass/test-should-panic-attr.rs @@ -0,0 +1,46 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: --test + +#[test] +#[should_panic = "foo"] +//~^ WARN: attribute must be of the form: +fn test1() { + panic!(); +} + +#[test] +#[should_panic(expected)] +//~^ WARN: argument must be of the form: +fn test2() { + panic!(); +} + +#[test] +#[should_panic(expect)] +//~^ WARN: argument must be of the form: +fn test3() { + panic!(); +} + +#[test] +#[should_panic(expected(foo, bar))] +//~^ WARN: argument must be of the form: +fn test4() { + panic!(); +} + +#[test] +#[should_panic(expected = "foo", bar)] +//~^ WARN: argument must be of the form: +fn test5() { + panic!(); +} From fb5ccf80fe83653018794562bfc105d6914384ae Mon Sep 17 00:00:00 2001 From: Josh Driver Date: Fri, 18 Nov 2016 21:01:19 +1030 Subject: [PATCH 2/2] Warn when a #[should_panic] test has an unexpected message --- src/libtest/lib.rs | 43 +++++++++++++------ .../run-fail/test-should-panic-bad-message.rs | 19 ++++++++ .../run-fail/test-should-panic-no-message.rs | 19 ++++++++ 3 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 src/test/run-fail/test-should-panic-bad-message.rs create mode 100644 src/test/run-fail/test-should-panic-no-message.rs diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 95ae6eb2efe8e..8749a64e5fdb4 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -75,9 +75,9 @@ const TEST_WARN_TIMEOUT_S: u64 = 60; // to be used by rustc to compile tests in libtest pub mod test { pub use {Bencher, TestName, TestResult, TestDesc, TestDescAndFn, TestOpts, TrFailed, - TrIgnored, TrOk, Metric, MetricMap, StaticTestFn, StaticTestName, DynTestName, - DynTestFn, run_test, test_main, test_main_static, filter_tests, parse_opts, - StaticBenchFn, ShouldPanic}; + TrFailedMsg, TrIgnored, TrOk, Metric, MetricMap, StaticTestFn, StaticTestName, + DynTestName, DynTestFn, run_test, test_main, test_main_static, filter_tests, + parse_opts, StaticBenchFn, ShouldPanic}; } pub mod stats; @@ -473,6 +473,7 @@ pub struct BenchSamples { pub enum TestResult { TrOk, TrFailed, + TrFailedMsg(String), TrIgnored, TrMetrics(MetricMap), TrBench(BenchSamples), @@ -611,7 +612,7 @@ impl ConsoleTestState { pub fn write_result(&mut self, result: &TestResult) -> io::Result<()> { match *result { TrOk => self.write_ok(), - TrFailed => self.write_failed(), + TrFailed | TrFailedMsg(_) => self.write_failed(), TrIgnored => self.write_ignored(), TrMetrics(ref mm) => { self.write_metric()?; @@ -638,6 +639,7 @@ impl ConsoleTestState { match *result { TrOk => "ok".to_owned(), TrFailed => "failed".to_owned(), + TrFailedMsg(ref msg) => format!("failed: {}", msg), TrIgnored => "ignored".to_owned(), TrMetrics(ref mm) => mm.fmt_metrics(), TrBench(ref bs) => fmt_bench_samples(bs), @@ -773,6 +775,14 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec) -> io::Resu st.failed += 1; st.failures.push((test, stdout)); } + TrFailedMsg(msg) => { + st.failed += 1; + let mut stdout = stdout; + stdout.extend_from_slice( + format!("note: {}", msg).as_bytes() + ); + st.failures.push((test, stdout)); + } } Ok(()) } @@ -1270,12 +1280,16 @@ fn calc_result(desc: &TestDesc, task_result: Result<(), Box>) -> Tes match (&desc.should_panic, task_result) { (&ShouldPanic::No, Ok(())) | (&ShouldPanic::Yes, Err(_)) => TrOk, - (&ShouldPanic::YesWithMessage(msg), Err(ref err)) + (&ShouldPanic::YesWithMessage(msg), Err(ref err)) => if err.downcast_ref::() - .map(|e| &**e) - .or_else(|| err.downcast_ref::<&'static str>().map(|e| *e)) - .map(|e| e.contains(msg)) - .unwrap_or(false) => TrOk, + .map(|e| &**e) + .or_else(|| err.downcast_ref::<&'static str>().map(|e| *e)) + .map(|e| e.contains(msg)) + .unwrap_or(false) { + TrOk + } else { + TrFailedMsg(format!("Panic did not include expected string '{}'", msg)) + }, _ => TrFailed, } } @@ -1482,8 +1496,9 @@ pub mod bench { #[cfg(test)] mod tests { - use test::{TrFailed, TrIgnored, TrOk, filter_tests, parse_opts, TestDesc, TestDescAndFn, - TestOpts, run_test, MetricMap, StaticTestName, DynTestName, DynTestFn, ShouldPanic}; + use test::{TrFailed, TrFailedMsg, TrIgnored, TrOk, filter_tests, parse_opts, TestDesc, + TestDescAndFn, TestOpts, run_test, MetricMap, StaticTestName, DynTestName, + DynTestFn, ShouldPanic}; use std::sync::mpsc::channel; #[test] @@ -1565,18 +1580,20 @@ mod tests { fn f() { panic!("an error message"); } + let expected = "foobar"; + let failed_msg = "Panic did not include expected string"; let desc = TestDescAndFn { desc: TestDesc { name: StaticTestName("whatever"), ignore: false, - should_panic: ShouldPanic::YesWithMessage("foobar"), + should_panic: ShouldPanic::YesWithMessage(expected), }, testfn: DynTestFn(Box::new(move |()| f())), }; let (tx, rx) = channel(); run_test(&TestOpts::new(), false, desc, tx); let (_, res, _) = rx.recv().unwrap(); - assert!(res == TrFailed); + assert!(res == TrFailedMsg(format!("{} '{}'", failed_msg, expected))); } #[test] diff --git a/src/test/run-fail/test-should-panic-bad-message.rs b/src/test/run-fail/test-should-panic-bad-message.rs new file mode 100644 index 0000000000000..7186672b4049e --- /dev/null +++ b/src/test/run-fail/test-should-panic-bad-message.rs @@ -0,0 +1,19 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: --test + +// error-pattern:panicked at 'bar' +// check-stdout +#[test] +#[should_panic(expected = "foo")] +pub fn test_bar() { + panic!("bar") +} diff --git a/src/test/run-fail/test-should-panic-no-message.rs b/src/test/run-fail/test-should-panic-no-message.rs new file mode 100644 index 0000000000000..50dc2aed8e9d0 --- /dev/null +++ b/src/test/run-fail/test-should-panic-no-message.rs @@ -0,0 +1,19 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: --test + +// error-pattern:panicked at 'explicit panic' +// check-stdout +#[test] +#[should_panic(expected = "foo")] +pub fn test_explicit() { + panic!() +}