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

#[IgnoreDeprecations] attribute to ignore E_(USER_)DEPRECATED issues on test class and test method level #5532

Closed
OskarStark opened this issue Oct 5, 2023 · 9 comments
Assignees
Labels
feature/metadata/attributes feature/test-runner CLI test runner type/enhancement A new idea that should be implemented
Milestone

Comments

@OskarStark
Copy link
Contributor

OskarStark commented Oct 5, 2023

https://packagist.org/packages/symfony/phpunit-bridge

Today at the SymfonyLive Berlin, @sebastianbergmann, @derrabus, @nicolas-grekas and me talked about the possibility of dropping the need for most features in symfony/phpunit-bridge. Many things are already implemented out of the box in PHPUnit 10.4, while not all, some features like ClockMock and class_exists mocks are still a valid use case for a 3rd party bridge.

Right now one can filter the deprecations and get them as grouped output via restrictDeprecations option 👍

In symfony/phpunit-bridge we use @group legacy annotation. Tests/classes marked with this annotation ignore all raised deprecations to be able to test this code properly without notifying about the deprecations. More infos here...

An idea would be to contribute a new #[IgnoreDeprecations] attribute, to use this as a replacement. Open for better naming ideas 🙏

Any thoughts?

@OskarStark OskarStark added the type/enhancement A new idea that should be implemented label Oct 5, 2023
@sebastianbergmann sebastianbergmann changed the title Things which could help drop most of the code/magic in the symfony/phpunit-bridge #[IgnoreDeprecations] attribute to ignore E_(USER_)DEPRECATE issues on test class and test method level Oct 6, 2023
@sebastianbergmann sebastianbergmann self-assigned this Oct 6, 2023
@sebastianbergmann sebastianbergmann added this to the PHPUnit 10.5 milestone Oct 6, 2023
@sebastianbergmann sebastianbergmann changed the title #[IgnoreDeprecations] attribute to ignore E_(USER_)DEPRECATE issues on test class and test method level #[IgnoreDeprecations] attribute to ignore E_(USER_)DEPRECATED issues on test class and test method level Oct 6, 2023
@sebastianbergmann
Copy link
Owner

I have this working locally already, just need to implement more end-to-end tests for it. Should be ready for me to push to main next week.

@nicolas-grekas
Copy link
Contributor

nicolas-grekas commented Oct 6, 2023

There's one more thing on the topic that's important to the use case : being able to exclude legacy tests.
On the CI of Symfony, we run tests with --exclude-group legacy when testing a component at eg v6 while loading its deps at v7. Legacy tests cannot work by definition in this situation, so we have to skip them, and the fact we're using a group makes it trivial.

@sebastianbergmann
Copy link
Owner

I do not want to conflate #[IgnoreDeprecations] with #[Group].

@nicolas-grekas
Copy link
Contributor

Sure, I'm not saying using a group should be the solution, I'm just sharing the use case so we can think about the best way to make it covered.

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Oct 6, 2023

I think a test for legacy functionality should have both #[Group('legacy')] and #[IgnoreDeprecations].

@nicolas-grekas
Copy link
Contributor

Ah, one more thing just to be 100% sure this would work: a test case with #[IgnoreDeprecations] should still be able to assert that deprecations are triggered. It's actually those test cases that test deprecations.

@sebastianbergmann
Copy link
Owner

I do not know how you test deprecations, so I cannot comment on that. At this point, I would like to ask you to move Symfony-specific discussions to your issue tracker, if that is okay with you. Testing (Symfony) deprecations is out of scope for PHPUnit, sorry.

@OskarStark
Copy link
Contributor Author

What @nicolas-grekas means is not Symfony related, but test framework related.

Let's say we introduce a deprecation because of a new code path, we want to have 2 tests (for simplicity) afterwards. One testing the old code, that it still works as expected, while asserting our new deprecation is thrown in the right place with the expected message. The other (new) test should just do regular assertion.

TLDR
an "expectException()" method. If such a method is not welcome, are we able todo such assertion on our own by leveraging the new event system?

@sebastianbergmann
Copy link
Owner

We had something similar in the past, it did not work reliably and caused problems, so we removed it. I am very reluctant to add support for "assert that an E_* was triggered". If you really want to test that then my suggestion would be to disable PHPUnit's error handler for such a test, register your own, collect issues, and verify that the issue you're expecting was triggered.

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

No branches or pull requests

3 participants