[PhpUnitBridge] Fix expectDeprecation() in isolation #37515

alexpott · Jul 7, 2020

Q	A
Branch?	5.1
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #...
License	MIT
Doc PR	symfony/symfony-docs#...

Tests like

    /**
     * Do not remove this test in the next major version.
     *
     * @group legacy
     * @runInSeparateProcess
     */
    public function testOneInIsolation()
    {
        $this->expectDeprecation('foo');
        @trigger_error('foo', E_USER_DEPRECATED);
    }

will fail due to:

Testing Symfony\Bridge\PhpUnit\Tests\ExpectDeprecationTraitTest
R                                                                   1 / 1 (100%)R

Time: 111 ms, Memory: 6.00 MB

There were 2 risky tests:

1) Symfony\Bridge\PhpUnit\Tests\ExpectDeprecationTraitTest::testOneInIsolation
This test did not perform any assertions

/Users/alex/dev/symfony/src/Symfony/Bridge/PhpUnit/Tests/ExpectDeprecationTraitTest.php:38

2) Symfony\Bridge\PhpUnit\Tests\ExpectDeprecationTraitTest::testOneInIsolation
This test did not perform any assertions

OK, but incomplete, skipped, or risky tests!
Tests: 1, Assertions: 0, Risky: 2.

alexpott · Jul 7, 2020

The problem with the fix is that now the assertion count is wrong for tests not run in a separate process.

For example:

./phpunit src/Symfony/Bridge/PhpUnit/Tests/ExpectDeprecationTraitTest.php --filter testOneWithAnnotation                                                                                                             
#!/usr/bin/env php
PHPUnit 8.3.5 by Sebastian Bergmann and contributors.

Testing Symfony\Bridge\PhpUnit\Tests\ExpectDeprecationTraitTest
.                                                                   1 / 1 (100%)

Time: 52 ms, Memory: 6.00 MB

OK (1 test, 3 assertions)

Before this change it was OK (1 test, 2 assertions)

alexpott · Jul 7, 2020

Actually this has unearthed a significant but in the symfony expectDeprecation method in isolated tests - if you change the expectation to any value it'll still work because essentially the expectation is ignored.

    /**
     * Do not remove this test in the next major version.
     *
     * @group legacy
     * @runInSeparateProcess
     */
    public function testOneInIsolation()
    {
        $this->expectDeprecation('fdfdef2');
        @trigger_error('foo', E_USER_DEPRECATED);
    }

will happily pass.

nicolas-grekas

thanks for working on this!

src/Symfony/Bridge/PhpUnit/Tests/expectdeprecationfail.phpt

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

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

src/Symfony/Bridge/PhpUnit/FailTests/ExpectDeprecationTraitFailTest.php

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

src/Symfony/Bridge/PhpUnit/Tests/expectdeprecationfail.phpt

src/Symfony/Bridge/PhpUnit/composer.json

src/Symfony/Bridge/PhpUnit/Tests/FailTests/ExpectDeprecationTraitTestFail.php

nicolas-grekas · Jul 9, 2020

Thank you @alexpott.

alexpott requested review from dunglas, jderusse, lyrixx, sroze and xabbuh as code owners July 7, 2020 16:50

carsonbot added Status: Needs Review Bug labels Jul 7, 2020

alexpott changed the base branch from 5.0 to master July 7, 2020 16:51

alexpott force-pushed the expect-deprecations-in-isolation branch from 0e1e77a to c781eeb Compare July 7, 2020 17:27

alexpott changed the base branch from master to 5.1 July 7, 2020 17:28

nicolas-grekas added this to the 5.1 milestone Jul 8, 2020

nicolas-grekas reviewed Jul 8, 2020

View reviewed changes

stof reviewed Jul 8, 2020

View reviewed changes

src/Symfony/Bridge/PhpUnit/FailTests/ExpectDeprecationTraitFailTest.php Outdated Show resolved Hide resolved

src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php Outdated Show resolved Hide resolved

alexpott commented Jul 8, 2020

View reviewed changes

src/Symfony/Bridge/PhpUnit/Tests/expectdeprecationfail.phpt Outdated Show resolved Hide resolved

stof reviewed Jul 8, 2020

View reviewed changes

src/Symfony/Bridge/PhpUnit/composer.json Outdated Show resolved Hide resolved

src/Symfony/Bridge/PhpUnit/Tests/FailTests/ExpectDeprecationTraitTestFail.php Outdated Show resolved Hide resolved

alexpott requested review from nicolas-grekas and stof July 9, 2020 07:51

nicolas-grekas approved these changes Jul 9, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jul 9, 2020

nicolas-grekas changed the title ~~Expect deprecations in isolation~~ [PhpUnitBridge] Expect deprecations in isolation Jul 9, 2020

nicolas-grekas changed the title ~~[PhpUnitBridge] Expect deprecations in isolation~~ [PhpUnitBridge] Fi expectDeprecation() in isolation Jul 9, 2020

nicolas-grekas changed the title ~~[PhpUnitBridge] Fi expectDeprecation() in isolation~~ [PhpUnitBridge] Fix expectDeprecation() in isolation Jul 9, 2020

Expect deprecations in isolation

e7e2ee7

nicolas-grekas force-pushed the expect-deprecations-in-isolation branch from c6a5d7c to e7e2ee7 Compare July 9, 2020 08:07

nicolas-grekas merged commit 5992bf0 into symfony:5.1 Jul 9, 2020

fabpot mentioned this pull request Jul 24, 2020

Release v5.1.3 #37650

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PhpUnitBridge] Fix expectDeprecation() in isolation #37515

[PhpUnitBridge] Fix expectDeprecation() in isolation #37515

alexpott commented Jul 7, 2020 •

edited

Loading

alexpott commented Jul 7, 2020

alexpott commented Jul 7, 2020

nicolas-grekas left a comment

nicolas-grekas commented Jul 9, 2020

Search code, repositories, users, issues, pull requests...

[PhpUnitBridge] Fix expectDeprecation() in isolation #37515

[PhpUnitBridge] Fix expectDeprecation() in isolation #37515

Conversation

alexpott commented Jul 7, 2020 • edited Loading

alexpott commented Jul 7, 2020

alexpott commented Jul 7, 2020

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Jul 9, 2020

alexpott commented Jul 7, 2020 •

edited

Loading