From 616af6eb83630b76bb3a9bde57a00f5ebe5dbd6c Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Thu, 4 Dec 2014 23:02:36 -0800 Subject: [PATCH] Allow message specification for should_fail 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]; } ``` --- src/compiletest/compiletest.rs | 2 +- src/librustdoc/test.rs | 2 +- src/libsyntax/test.rs | 34 ++++++- src/libtest/lib.rs | 93 ++++++++++++++----- .../run-fail/test-should-fail-bad-message.rs | 22 +++++ .../run-pass/test-should-fail-good-message.rs | 26 ++++++ 6 files changed, 150 insertions(+), 29 deletions(-) create mode 100644 src/test/run-fail/test-should-fail-bad-message.rs create mode 100644 src/test/run-pass/test-should-fail-good-message.rs diff --git a/src/compiletest/compiletest.rs b/src/compiletest/compiletest.rs index e28029c4d9d..47ab675aff9 100644 --- a/src/compiletest/compiletest.rs +++ b/src/compiletest/compiletest.rs @@ -346,7 +346,7 @@ pub fn make_test(config: &Config, testfile: &Path, f: || -> test::TestFn) desc: test::TestDesc { name: make_test_name(config, testfile), ignore: header::is_test_ignored(config, testfile), - should_fail: false + should_fail: test::ShouldFail::No, }, testfn: f(), } diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 7ca7ae4b211..5759adf3244 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -280,7 +280,7 @@ impl Collector { desc: testing::TestDesc { name: testing::DynTestName(name), ignore: should_ignore, - should_fail: false, // compiler failures are test failures + should_fail: testing::ShouldFail::No, // compiler failures are test failures }, testfn: testing::DynTestFn(proc() { runtest(test.as_slice(), diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs index 05828fc05f8..3d2cb988434 100644 --- a/src/libsyntax/test.rs +++ b/src/libsyntax/test.rs @@ -37,12 +37,17 @@ use {ast, ast_util}; use ptr::P; use util::small_vector::SmallVector; +enum ShouldFail { + No, + Yes(Option), +} + struct Test { span: Span, path: Vec , bench: bool, ignore: bool, - should_fail: bool + should_fail: ShouldFail } struct TestCtxt<'a> { @@ -360,8 +365,16 @@ fn is_ignored(i: &ast::Item) -> bool { i.attrs.iter().any(|attr| attr.check_name("ignore")) } -fn should_fail(i: &ast::Item) -> bool { - attr::contains_name(i.attrs.as_slice(), "should_fail") +fn should_fail(i: &ast::Item) -> ShouldFail { + match i.attrs.iter().find(|attr| attr.check_name("should_fail")) { + Some(attr) => { + let msg = attr.meta_item_list() + .and_then(|list| list.iter().find(|mi| mi.check_name("message"))) + .and_then(|mi| mi.value_str()); + ShouldFail::Yes(msg) + } + None => ShouldFail::No, + } } /* @@ -550,7 +563,20 @@ fn mk_test_desc_and_fn_rec(cx: &TestCtxt, test: &Test) -> P { vec![name_expr]); let ignore_expr = ecx.expr_bool(span, test.ignore); - let fail_expr = ecx.expr_bool(span, test.should_fail); + let should_fail_path = |name| { + ecx.path(span, vec![self_id, test_id, ecx.ident_of("ShouldFail"), ecx.ident_of(name)]) + }; + let fail_expr = match test.should_fail { + ShouldFail::No => ecx.expr_path(should_fail_path("No")), + ShouldFail::Yes(ref msg) => { + let path = should_fail_path("Yes"); + let arg = match *msg { + Some(ref msg) => ecx.expr_some(span, ecx.expr_str(span, msg.clone())), + None => ecx.expr_none(span), + }; + ecx.expr_call(span, ecx.expr_path(path), vec![arg]) + } + }; // self::test::TestDesc { ... } let desc_expr = ecx.expr_struct( diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index c943d8706e5..5cdd346f981 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -47,6 +47,7 @@ use self::TestEvent::*; use self::NamePadding::*; use self::OutputLocation::*; +use std::any::{Any, AnyRefExt}; use std::collections::TreeMap; use stats::Stats; use getopts::{OptGroup, optflag, optopt}; @@ -78,7 +79,7 @@ pub mod test { MetricChange, Improvement, Regression, LikelyNoise, StaticTestFn, StaticTestName, DynTestName, DynTestFn, run_test, test_main, test_main_static, filter_tests, - parse_opts, StaticBenchFn}; + parse_opts, StaticBenchFn, ShouldFail}; } pub mod stats; @@ -184,13 +185,19 @@ pub struct Bencher { pub bytes: u64, } +#[deriving(Clone, Show, PartialEq, Eq, Hash)] +pub enum ShouldFail { + No, + Yes(Option<&'static str>) +} + // The definition of a single test. A test runner will run a list of // these. #[deriving(Clone, Show, PartialEq, Eq, Hash)] pub struct TestDesc { pub name: TestName, pub ignore: bool, - pub should_fail: bool, + pub should_fail: ShouldFail, } #[deriving(Show)] @@ -346,7 +353,7 @@ fn optgroups() -> Vec { fn usage(binary: &str) { let message = format!("Usage: {} [OPTIONS] [FILTER]", binary); - println!(r"{usage} + println!(r#"{usage} The FILTER regex is tested against the name of all tests to run, and only those tests that match are run. @@ -366,10 +373,12 @@ Test Attributes: function takes one argument (test::Bencher). #[should_fail] - This function (also labeled with #[test]) will only pass if the code causes a failure (an assertion failure or panic!) + A message may be provided, which the failure string must + contain: #[should_fail(message = "foo")]. #[ignore] - When applied to a function which is already attributed as a test, then the test runner will ignore these tests during normal test runs. Running with --ignored will run these - tests.", + tests."#, usage = getopts::usage(message.as_slice(), optgroups().as_slice())); } @@ -902,13 +911,13 @@ fn should_sort_failures_before_printing_them() { let test_a = TestDesc { name: StaticTestName("a"), ignore: false, - should_fail: false + should_fail: ShouldFail::No }; let test_b = TestDesc { name: StaticTestName("b"), ignore: false, - should_fail: false + should_fail: ShouldFail::No }; let mut st = ConsoleTestState { @@ -1114,7 +1123,7 @@ pub fn run_test(opts: &TestOpts, let stdout = reader.read_to_end().unwrap().into_iter().collect(); let task_result = result_future.into_inner(); - let test_result = calc_result(&desc, task_result.is_ok()); + let test_result = calc_result(&desc, task_result); monitor_ch.send((desc.clone(), test_result, stdout)); }) } @@ -1148,13 +1157,17 @@ pub fn run_test(opts: &TestOpts, } } -fn calc_result(desc: &TestDesc, task_succeeded: bool) -> TestResult { - if task_succeeded { - if desc.should_fail { TrFailed } - else { TrOk } - } else { - if desc.should_fail { TrOk } - else { TrFailed } +fn calc_result(desc: &TestDesc, task_result: Result<(), Box>) -> TestResult { + match (&desc.should_fail, task_result) { + (&ShouldFail::No, Ok(())) | + (&ShouldFail::Yes(None), Err(_)) => TrOk, + (&ShouldFail::Yes(Some(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, + _ => TrFailed, } } @@ -1437,7 +1450,7 @@ mod tests { TestDesc, TestDescAndFn, TestOpts, run_test, Metric, MetricMap, MetricAdded, MetricRemoved, Improvement, Regression, LikelyNoise, - StaticTestName, DynTestName, DynTestFn}; + StaticTestName, DynTestName, DynTestFn, ShouldFail}; use std::io::TempDir; #[test] @@ -1447,7 +1460,7 @@ mod tests { desc: TestDesc { name: StaticTestName("whatever"), ignore: true, - should_fail: false + should_fail: ShouldFail::No, }, testfn: DynTestFn(proc() f()), }; @@ -1464,7 +1477,7 @@ mod tests { desc: TestDesc { name: StaticTestName("whatever"), ignore: true, - should_fail: false + should_fail: ShouldFail::No, }, testfn: DynTestFn(proc() f()), }; @@ -1481,7 +1494,7 @@ mod tests { desc: TestDesc { name: StaticTestName("whatever"), ignore: false, - should_fail: true + should_fail: ShouldFail::Yes(None) }, testfn: DynTestFn(proc() f()), }; @@ -1491,6 +1504,40 @@ mod tests { assert!(res == TrOk); } + #[test] + fn test_should_fail_good_message() { + fn f() { panic!("an error message"); } + let desc = TestDescAndFn { + desc: TestDesc { + name: StaticTestName("whatever"), + ignore: false, + should_fail: ShouldFail::Yes(Some("error message")) + }, + testfn: DynTestFn(proc() f()), + }; + let (tx, rx) = channel(); + run_test(&TestOpts::new(), false, desc, tx); + let (_, res, _) = rx.recv(); + assert!(res == TrOk); + } + + #[test] + fn test_should_fail_bad_message() { + fn f() { panic!("an error message"); } + let desc = TestDescAndFn { + desc: TestDesc { + name: StaticTestName("whatever"), + ignore: false, + should_fail: ShouldFail::Yes(Some("foobar")) + }, + testfn: DynTestFn(proc() f()), + }; + let (tx, rx) = channel(); + run_test(&TestOpts::new(), false, desc, tx); + let (_, res, _) = rx.recv(); + assert!(res == TrFailed); + } + #[test] fn test_should_fail_but_succeeds() { fn f() { } @@ -1498,7 +1545,7 @@ mod tests { desc: TestDesc { name: StaticTestName("whatever"), ignore: false, - should_fail: true + should_fail: ShouldFail::Yes(None) }, testfn: DynTestFn(proc() f()), }; @@ -1544,7 +1591,7 @@ mod tests { desc: TestDesc { name: StaticTestName("1"), ignore: true, - should_fail: false, + should_fail: ShouldFail::No, }, testfn: DynTestFn(proc() {}), }, @@ -1552,7 +1599,7 @@ mod tests { desc: TestDesc { name: StaticTestName("2"), ignore: false, - should_fail: false + should_fail: ShouldFail::No, }, testfn: DynTestFn(proc() {}), }); @@ -1588,7 +1635,7 @@ mod tests { desc: TestDesc { name: DynTestName((*name).clone()), ignore: false, - should_fail: false + should_fail: ShouldFail::No, }, testfn: DynTestFn(testfn), }; @@ -1629,7 +1676,7 @@ mod tests { desc: TestDesc { name: DynTestName(name.to_string()), ignore: false, - should_fail: false + should_fail: ShouldFail::No, }, testfn: DynTestFn(test_fn) } diff --git a/src/test/run-fail/test-should-fail-bad-message.rs b/src/test/run-fail/test-should-fail-bad-message.rs new file mode 100644 index 00000000000..dc703de489c --- /dev/null +++ b/src/test/run-fail/test-should-fail-bad-message.rs @@ -0,0 +1,22 @@ +// 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. + +// check-stdout +// error-pattern:task 'test_foo' panicked at +// compile-flags: --test +// ignore-pretty: does not work well with `--test` + +#[test] +#[should_fail(message = "foobar")] +fn test_foo() { + panic!("blah") +} + + diff --git a/src/test/run-pass/test-should-fail-good-message.rs b/src/test/run-pass/test-should-fail-good-message.rs new file mode 100644 index 00000000000..9d422700fbf --- /dev/null +++ b/src/test/run-pass/test-should-fail-good-message.rs @@ -0,0 +1,26 @@ +// 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 +// ignore-pretty: does not work well with `--test` + +#[test] +#[should_fail(message = "foo")] +fn test_foo() { + panic!("foo bar") +} + +#[test] +#[should_fail(message = "foo")] +fn test_foo_dynamic() { + panic!("{} bar", "foo") +} + +