10000 bug #46792 [Bridge] Corrects bug in test listener trait (magikid) · symfony/symfony@df6cb75 · GitHub
[go: up one dir, main page]

Skip to content

Commit df6cb75

Browse files
committed
bug #46792 [Bridge] Corrects bug in test listener trait (magikid)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Bridge] Corrects bug in test listener trait If you enable the `SymfonyTestsListenerTrait`, use the `ExpectDeprecationTrait`, and call `$this->expectNotToPerformAssertions()`, an error was returned in your test saying it was risky as it did not perform any assertions even though it was expected not to. This bug turned out to be caused by not checking if the `doesNotPerformAssertions` flag had been set. Issue: #41444 | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #41444 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> <!-- Replace this notice by a short README for your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against the latest branch. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> This fixes a bug that could occur when using this listener along with the `ExpectDeprecationTrait` trait and the `TestCase::expectNotToPerformAssertions()` method. When that method is called, the user is saying that no assertions will happen but the listener didn't care and saw no assertions and none skipped so threw the 'No assertions' warning message. This updates the listener to check the value of `TestCase::doesNotPerformAssertions` when deciding to warn about no assertions or not. Commits ------- d7d4bd7 [Bridge] Corrects bug in test listener trait
2 parents 04d3e55 + d7d4bd7 commit df6cb75

File tree

3 files changed

+51
-1
lines changed

3 files changed

+51
-1
lines changed

src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public function endTest($test, $time)
271271
$assertions = \count(self::$expectedDeprecations) + $test->getNumAssertions();
272272
if ($test->doesNotPerformAssertions() && $assertions > 0) {
273273
$test->getTestResultObject()->addFailure($test, new RiskyTestError(sprintf('This test is annotated with "@doesNotPerformAssertions", but performed %s assertions', $assertions)), $time);
274-
} elseif ($assertions === 0 && $test->getTestResultObject()->noneSkipped()) {
274+
} elseif ($assertions === 0 && !$test->doesNotPerformAssertions() && $test->getTestResultObject()->noneSkipped()) {
275275
$test->getTestResultObject()->addFailure($test, new RiskyTestError('This test did not perform any assertions'), $time);
276276
}
277277

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bridge\PhpUnit\Tests\FailTests;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
16+
17+
/**
18+
* This class is deliberately suffixed with *TestRisky.php so that it is ignored
19+
* by PHPUnit. This test is designed to fail. See ../expectnotrisky.phpt.
20+
*/
21+
final class NoAssertionsTestNotRisky extends TestCase
22+
{
23+
use ExpectDeprecationTrait;
24+
25+
/**
26+
* Do not remove this test in the next major version.
27+
*/
28+
public function testOne()
29+
{
30+
$this->expectNotToPerformAssertions();
31+
}
32+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Test NoAssertionsTestNotRisky not risky test
3+
--SKIPIF--
4+
<?php if ('\\' === DIRECTORY_SEPARATOR && !extension_loaded('mbstring')) die('Skipping on Windows without mbstring');
5+
--FILE--
6+
<?php
7+
$test = realpath(__DIR__.'/FailTests/NoAssertionsTestNotRisky.php');
8+
passthru('php '.getenv('SYMFONY_SIMPLE_PHPUNIT_BIN_DIR').'/simple-phpunit.php --fail-on-risky --colors=never '.$test);
9+
?>
10+
--EXPECTF--
11+
PHPUnit %s
12+
13+
%ATesting Symfony\Bridge\PhpUnit\Tests\FailTests\NoAssertionsTestNotRisky
14+
. 1 / 1 (100%)
15+
16+
Time: %s, Memory: %s
17+
18+
OK (1 test, 0 assertions)

0 commit comments

Comments
 (0)
0