Skip to content

Commit 4d4fc36

Browse files
authored
[TASK] Do not assertX but fail() in tearDown() (#660)
phpunit has a best practice that tests should usually have at least one assertion to be sure they actually do something. All assertions thus raise a counter that is checked after test execution. If zero, phpunit marks the test risky with "test has no assertion". There are two ways to suppress this: * Setting beStrictAboutTestsThatDoNotTestAnything="false" via phpunit config * Adding #[DoesNotPerformAssertions] attribute to single tests to actively mark tests that do not assert something as legit Our abstract UnitTestCase spoils this by always doing assertions in tearDown(). The patch turns these assertions into check+fail() code instead. Unit tests that don't have assertions for whatever reason are now properly marked as risky as intended by phpunit. Releases: main
1 parent 32cc772 commit 4d4fc36

File tree

1 file changed

+18
-16
lines changed

1 file changed

+18
-16
lines changed

Classes/Core/Unit/UnitTestCase.php

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,13 @@ protected function tearDown(): void
127127
$singletonInstances = GeneralUtility::getSingletonInstances();
128128
// Reset singletons anyway to not let all further tests fail
129129
GeneralUtility::resetSingletonInstances([]);
130-
self::assertEmpty(
131-
$singletonInstances,
132-
'tearDown() integrity check found left over singleton instances in GeneralUtility::makeInstance()'
133-
. ' instance list. The test should probably set \'$this->resetSingletonInstances = true;\' to'
134-
. ' reset this framework state change. Found singletons: ' . implode(', ', array_keys($singletonInstances))
135-
);
130+
if (!empty($singletonInstances)) {
131+
self::fail(
132+
'tearDown() integrity check found left over singleton instances in GeneralUtility::makeInstance()'
133+
. ' instance list. The test should probably set \'$this->resetSingletonInstances = true;\' to'
134+
. ' reset this framework state change. Found singletons: ' . implode(', ', array_keys($singletonInstances))
135+
);
136+
}
136137
}
137138

138139
// Delete registered test files and directories
@@ -175,18 +176,19 @@ protected function tearDown(): void
175176
if (!empty($notCleanInstances)) {
176177
// Reset instance list (including singletons & container) to not let all further tests fail
177178
GeneralUtility::purgeInstances();
179+
self::fail(
180+
'tearDown() integrity check found left over instances in GeneralUtility::makeInstance() instance list.'
181+
. ' Always consume instances added via GeneralUtility::addInstance() in your test by the test subject.'
182+
. ' Found instances of these classes: ' . implode(', ', array_keys($notCleanInstances))
183+
);
178184
}
179-
// Let the test fail if there were instances left and give some message on why it fails
180-
self::assertEquals(
181-
[],
182-
$notCleanInstances,
183-
'tearDown() integrity check found left over instances in GeneralUtility::makeInstance() instance list.'
184-
. ' Always consume instances added via GeneralUtility::addInstance() in your test by the test subject.'
185-
);
186185

187-
self::assertTrue($this->setUpMethodCallChainValid, 'tearDown() integrity check detected that setUp has a '
188-
. 'broken parent call chain. Please check that setUp() methods properly calls parent::setUp(), starting from "'
189-
. static::class . '"');
186+
if ($this->setUpMethodCallChainValid === false) {
187+
self::fail(
188+
'tearDown() integrity check detected that setUp() has a broken parent call chain.'
189+
. ' Please check that setUp() methods properly calls parent::setUp(), starting from "' . static::class . '"'
190+
);
191+
}
190192

191193
parent::tearDown();
192194
}

0 commit comments

Comments
 (0)