From c6b1f3f070f6bbfe90619041607bba03adbef8e1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 7 Mar 2014 10:03:24 -0800 Subject: [PATCH] Fail Arcanist tests when they make zero assertions Summary: Fixes T4570. When a test case doesn't make any assertions, fail it: - A tiny fraction of tests pass by not throwing. These tests can easily make a trivial assertion. It took about 5 minutes to fix them all (D8435, D8436). - In other cases, no assertions means a test construction problem, as with T4570. In these cases, failing loudly catches a severe error. - Fixes the no-assertion test cases in `arcanist/` - Makes the PHP 5.4 test pass for the moment, see discussion in T4334. Test Plan: Ran `arc unit --everything`. Reviewers: leebyron, btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4570 Differential Revision: https://secure.phabricator.com/D8437 --- src/lint/linter/__tests__/xhpast/php54.lint-test | 5 ++++- .../engine/__tests__/XUnitTestResultParserTestCase.php | 4 ++++ src/unit/engine/phutil/ArcanistPhutilTestCase.php | 8 ++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/lint/linter/__tests__/xhpast/php54.lint-test b/src/lint/linter/__tests__/xhpast/php54.lint-test index 8bc7d664..37265d05 100644 --- a/src/lint/linter/__tests__/xhpast/php54.lint-test +++ b/src/lint/linter/__tests__/xhpast/php54.lint-test @@ -1,7 +1,10 @@ m()[0]; + m()[0]; + +// The check above should be this, see T4334. +// $o->m()[0]; ~~~~~~~~~~ disabled:3:5 diff --git a/src/unit/engine/__tests__/XUnitTestResultParserTestCase.php b/src/unit/engine/__tests__/XUnitTestResultParserTestCase.php index 8f96941d..f9bb30a1 100644 --- a/src/unit/engine/__tests__/XUnitTestResultParserTestCase.php +++ b/src/unit/engine/__tests__/XUnitTestResultParserTestCase.php @@ -37,6 +37,8 @@ final class XUnitTestResultParserTestCase extends ArcanistTestCase { } catch (Exception $e) { // OK } + + $this->assertEqual(true, true); } public function testInvalidXmlInputFailure() { @@ -50,6 +52,8 @@ final class XUnitTestResultParserTestCase extends ArcanistTestCase { } catch (Exception $e) { // OK } + + $this->assertEqual(true, true); } } diff --git a/src/unit/engine/phutil/ArcanistPhutilTestCase.php b/src/unit/engine/phutil/ArcanistPhutilTestCase.php index 2825ebcd..73234d77 100644 --- a/src/unit/engine/phutil/ArcanistPhutilTestCase.php +++ b/src/unit/engine/phutil/ArcanistPhutilTestCase.php @@ -464,6 +464,14 @@ abstract class ArcanistPhutilTestCase { $exceptions); } } + + if (!$this->assertions) { + $this->failTest( + pht( + 'This test case made no assertions. Test cases must make at '. + 'least one assertion.')); + } + } catch (ArcanistPhutilTestTerminatedException $ex) { // Continue with the next test. } catch (ArcanistPhutilTestSkippedException $ex) {