Skip to content
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

Support for defining hook methods with both attribute and annotation without getting a deprecation #5695

Closed
stof opened this issue Feb 6, 2024 · 5 comments
Labels
type/enhancement A new idea that should be implemented

Comments

@stof
Copy link
Contributor

stof commented Feb 6, 2024

For packages wanting to support multiple major versions of PHPUnit (from 9 to latest in the case of prophecy-phpunit), it would be great to be able to define hook methods in a way supported in all versions without getting deprecations. See phpspec/prophecy-phpunit#60 for the discussion.

My suggestion would be supporting to define both the attribute and the annotation on the test method. Such cases should ignore the annotation entirely (making the corresponding attribute provide the configuration) without reporting a deprecation in PHPUnit 11 about the annotation being deprecated.
What do you think about this proposal ?

@stof stof added the type/enhancement A new idea that should be implemented label Feb 6, 2024
@stof
Copy link
Contributor Author

stof commented Feb 6, 2024

Note that the proposal might involve backporting it to PHPunit 10.x as well to avoid getting double registration of hook methods there, in case this is the result of having both the attribute and the annotation (I haven't tried this yet)

@sebastianbergmann
Copy link
Owner

My suggestion would be supporting to define both the attribute and the annotation on the test method.

PHPUnit 10 supports metadata in annotations and attributes. If an attribute is found on a unit of code, annotations on that unit of code are ignored.

Now PHPUnit 11 deprecated support for annotations, but still supports them in the same way PHPUnit 10 does. PHPUnit 12 will no longer support metadata in annotation, which means that all code related to parsing annotations can be removed. I do not think the additional complexity that would be required to not emit the deprecation for an annotation when the respective attribute is also present is worth it.

I am sorry, but you will need different versions of a package such as prophecy-phpunit for different major versions of PHPUnit.

@sebastianbergmann
Copy link
Owner

FYI:

$ cat ExampleTest.php     
<?php declare(strict_types=1);
use PHPUnit\Framework\Attributes\Before;
use PHPUnit\Framework\TestCase;

final class ExampleTest extends TestCase
{
    /**
     * @before
     */
    #[Before]
    protected function beforeMethod(): void
    {
    }

    public function testOne(): void
    {
        $this->assertTrue(true);
    }
}
$ phpunit --no-output --log-events-text php://stdout ExampleTest.php
PHPUnit Started (PHPUnit 11.0.2 using PHP 8.3.2 (cli) on Linux)
Test Runner Configured
Test Suite Loaded (1 test)
Event Facade Sealed
Test Runner Started
Test Suite Sorted
Test Runner Execution Started (1 test)
Test Suite Started (ExampleTest, 1 test)
Test Preparation Started (ExampleTest::testOne)
Before Test Method Called (ExampleTest::beforeMethod)
Before Test Method Finished:
- ExampleTest::beforeMethod
Test Prepared (ExampleTest::testOne)
Test Passed (ExampleTest::testOne)
Test Finished (ExampleTest::testOne)
Test Suite Finished (ExampleTest, 1 test)
Test Runner Execution Finished
Test Runner Finished
PHPUnit Finished (Shell Exit Code: 0)

When both /** @before */ and #[Before] are used then 1) the before-test method is called only once and 2) the deprecation is not emitted.

@stof
Copy link
Contributor Author

stof commented Feb 6, 2024

great. this is exactly the behavior I was hoping for.

@sebastianbergmann
Copy link
Owner

this is exactly the behavior I was hoping for

It's also exactly the behavior I wanted when I implemented this :-) I'm sorry for not immediately understanding what you were asking for. I'm all the more happy that what you were asking for was already there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A new idea that should be implemented
Projects
None yet
Development

No branches or pull requests

2 participants