PHPUnit 10 deprecates use of non-static @dataProvider methods. This means that @dataProvider methods can no longer make calls to non-static methods. Any non-static call ($this->someMethod()) needs to be replaced by a call to a static alternative (static::someOtherMethod()), that would have to be implemented if not available.
In this issue, focus on replacing $this->prophesize()
In the data providers, replace $this->prophesize() with (new Prophet())->prophesize() or similar so to get the mock object directly from Prophecy and not via the trait that is meant to be included in test objects.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !3830
Comments
Comment #3
mondrakeComment #4
smustgrave commentedThis looks very interesting, as I've gotten more into Unit testing vs just functional. Don't mind marking this.
Comment #8
catchThis looks good - I wonder if it's something we should try to put into phpstan/rector for 11.x readiness, will ask in slack.
Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!
Comment #9
mondrakephpstan/phpstan-phpunit already checks that... but only if PHPUnit 10 is in use, which does not help much in preparation to adopt PHPUnit 10.
See upstream https://github.com/phpstan/phpstan-phpunit/issues/178
Comment #11
catchSpoke to @bbrala and @mglaman in slack a bit about how to communicate this (via phpstan/rector and in general).
We came up with something like - adding a new PHPUnit 10 change record, which we can relate to all of the PHPUnit 10 changes, and then that can include the kinds of changes contrib needs to make.
For example for this issue we could talk about static data providers and
$prophet = new Prophet(); $prophet->prothesize();instead of$this->prothesize()- even though it's not Drupal API that we're changing, it'd be a useful cheat sheet for people.Re-opening and marking needs work for that, leaving the commit in.
Comment #12
mondrakeStarted a draft CR @ https://www.drupal.org/node/3365413
Comment #13
catchThat looks like a great start!
Comment #14
quietone commentedI am working through the unpublished change records. The title of the CR here is 'PHPUnit 10' which at first I thought meant that PHPUnit 10 was being used, which I also knew was not true. So, to avoid confusion I think the title needs to be changed.
And I read that the CR is intended to be used for multiple issues, which is fine. However, there are several issues for PHPUnit and they will be committed to different versions and there is only one version field. By that, we actually do need multiple CRs. It could be made simple by having one CR per version. But to have all the data in one place, what do you think of having a wiki page for the PHPUnit changes that the individual CRs can point to. The CR's would then be very short. The wiki page could be in PHPUnit in Drupal.
Comment #15
catchI've changed the title to "Changes required for PHPUnit 10 compatibility".
I'm not sure this is necessary. While it's true that there are multiple issues committed to different versions, we're not changing APIs in those issues (or not the vast majority), instead we're just changing individual tests so that they don't use APIs deprecated by PHPUnit. It's useful to document the changes that core had to do to make it easier for contrib when they're doing the same things, but we're not responsible for those API changes ourselves, and we'll only update to PHPUnit 10 in Drupal 11.