-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Prepare IntegrationTestCase for static data providers #4266
Prepare IntegrationTestCase for static data providers #4266
Conversation
The failures in the extra packages happen because we pull a version of Twig as dependency that does not include my changes. How do we deal with this kind of failure on this repo? |
@derrabus accept it for now, as it will be resolved after the merge-up in the 4.x branch. |
ae35b7a
to
a57f35e
Compare
Having PHPUnit 11 support only in 4.0 sounds good to me. |
a57f35e
to
18f4203
Compare
Thank you @derrabus. |
The documentation example is missing an update https://github.com/twigphp/Twig/blob/4.x/doc/advanced.rst#testing-an-extension |
instead, which will be abstract in 4.0. | ||
|
||
* The data providers ``getTests()`` and ``getLegacyTests()`` on | ||
``Twig\Test\IntegrationTestCase`` are considered final als of Twig 3.13. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
@williamdes Would you like to open a PR to improve documentation? |
🇫🇷 I will try soon :) |
Data providers need to be static in PHPUnit 11. Because of this, I'd like to declare the two methods we use as data providers in
IntegrationTestCase
as static in 4.0. This PR prepares that change:getFixturesDir()
method is replaced with a staticgetFixturesDirectory()
.getTests()
andgetLegacyTests()
are marked as final, so we can declare them static in the next major.This however means that we're delaying PHPUnit 11 compatibility of integration tests to Twig 4.0. If that's too late for us, we could deprecate the whole
IntegrationTestCase
in favor of a compatible replacement. That's a bigger change, but I would work on it if you think it's worth it.