Skip to content

[PhpUnitBridge] Clean up mocked features only when @group is present #60484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2025

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented May 20, 2025

Q A
Branch? 7.2
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

PR #60174 introduced a bug for tests that don't use the @group annotation but register mocks manually, e.g. in setUpBeforeClass:

class ExampleTest extends TestCase
{
    public static function setUpBeforeClass(): void
    {
        ClockMock::register(self::class);
        ClockMock::withClockMock(strtotime('2024-05-20 15:30:00'));
    }

    public static function tearDownAfterClass(): void
    {
        ClockMock::withClockMock(false);
    }

    public function testDate()
    {
        self::assertSame('2024-05-20 15:30:00', date('Y-m-d H:i:s'));
    }

    public function testTime()
    {
        self::assertSame(1716219000, time());
    }
}

testDate passes, but after that FinishedSubscriber is triggered which cleans up the mock and causes testTime to fail.

I am not sure what to do about BeforeTestMethodErroredSubscriber since the test object is not available in that event. I think it's fine to leave it as is.

cc @xabbuh

@xabbuh
Copy link
Member

xabbuh commented May 20, 2025

The change looks good. 👍 We need to update the code a bit after merging it into the 7.3 branch to account for the attributes introduced in #59384.

I am not sure what to do about BeforeTestMethodErroredSubscriber since the test object is not available in that event. I think it's fine to leave it as is.

We can access the test with PHPUnit 12.1+ (see sebastianbergmann/phpunit@9f05cee).

@HypeMC HypeMC force-pushed the fix-symfonyextension branch from f1ced79 to cc14427 Compare May 20, 2025 07:51
@HypeMC
Copy link
Contributor Author

HypeMC commented May 20, 2025

@HypeMC HypeMC force-pushed the fix-symfonyextension branch from cc14427 to ea204b9 Compare May 20, 2025 09:10
@nicolas-grekas
Copy link
Member

Thank you @HypeMC.

@nicolas-grekas nicolas-grekas merged commit 920035e into symfony:7.2 May 20, 2025
11 of 12 checks passed
@HypeMC HypeMC deleted the fix-symfonyextension branch May 20, 2025 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants