-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Upgrade to PHPUnit 11 #1549
base: main
Are you sure you want to change the base?
Upgrade to PHPUnit 11 #1549
Conversation
- name: "Run tests" | ||
run: vendor/bin/simple-phpunit ${{ env.PHPUNIT_FLAGS }} | ||
run: bin/phpunit |
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.
shouldn't this be vendor/bin/phpunit
?
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.
The phpunit/phpunit recipe also installs bin/phpunit
. This is done to make migrating away from our simple-phpunit wrapper easy and without impacting documentation (as both use the same helper script).
I'm not sure why this project used the vendor binary in the CI (as the bin/phpunit
file is mentioned in the README).
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.
but you have not replaced the bin/phpunit
file in this PR, so it still tries to use the phpunit-bridge
csrf_protection: | ||
token_id: submit | ||
|
||
csrf_protection: |
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.
this looks like a separate change
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.
Yes, this was just to get it working. I'm not really understanding the set-up of this project, because it has a symfony.lock but no composer.lock (although composer.lock is not gitignored). So I couldn't install phpunit/phpunit without also getting these recipe updates.
This is in a separate commit, which probably should be removed from this PR before merging.
This PR upgrades to PHPUnit 11, including using it's deprecation handling features. Starting with PHPUnit 11, all deprecations features from the bridge are implemented in PHPUnit.
This PR also stops using the PhpunitBridge wrapper, which is recommended for applications in the documentation for at least the past 2 years.
I've added the configuration to make PHPUnit not report indirect deprecations (those caused by 3rd party code), and to fail on a test that triggers a direct deprecation. I think this is the most sensible default configuration that we should promote. If we agree, I'll create a recipe to do this automatically.
This PR has mostly been created as a test to see what we need to fix in recipes/code and add to the documentation about PHPUnit 11.