feat: make prophesize() static for compatibility with PHPUnit 10 data providers

[dunglas]

PHPUnit 10 deprecated non-static data providers, and creating stubs in data providers is common.
To allow using Prophecy in data providers, this patch makes the prophesize() method static, as PHPUnit made for its own createStub() method.

This also dramatically eases migration to PHPUnit 10 for projects using Prophecy.

Calling $this->prohesize() still works because PHP allows calling static methods as if they weren't static.

[dunglas]

I dropped support for PHPUnit 9 because it's not possible to override its existing prophesize() method to make it static. It's not a big issue because PHPUnit 9 users will just get an older version of prophecy-phpunit.

[dunglas]

@sebastianbergmann would you accept a patch backporting this change in PHPUnit 9? This would allow keeping support for PHPUnit 9 and 10 in this package (it's currently not possible because TestCase::prophesize() isn't static, and so cannot be overridden by the static method provided by the trait of this package). This would make the migration from PHPUnit 9 to 10 easier (it would be possible to support both versions at the same time without relying on hacks like this: https://github.com/api-platform/core/pull/5701/files#diff-66024aef34c148aa02abcd1f06ba101ba303aadcd9a7cd076c7e773ddae5bd8fR756)

[sebastianbergmann]

@sebastianbergmann would you accept a patch backporting this change in PHPUnit 9?

Are we talking about a single-line change that adds the static keyword to the method's declaration?

[dunglas]

It will probably be a bit more than one line, but that's basically the idea yes: 182527d

[sebastianbergmann]

It will probably be a bit more than one line, but that's basically the idea yes: 182527d

That is a patch for prophecy-phpunit, not for PHPUnit.

Just send a pull request for PHPUnit's 9.6 branch. I will likely accept it.

[sebastianbergmann]

Just send a pull request for PHPUnit's 9.6 branch. I will likely accept it.

It might not be as easy as I hoped, or even possible, because $this->addWarning() is not static and we need to call it to warn about the fact that prophesize() is deprecated. I am still open to a pull request in general, but by now I doubt that this can be done in a small enough fashion that I am comfortable with for a branch that is closed for all changes except for bug fixes and changes to make it compatible with future PHP versions.

[dunglas]

As #56 has been refused (a decision that is totally understandable), this PR is now ready to be merged.

To ease the upgrade:

1. tag a 2.0.3 release with the current content of the master branch (this will allow to use this package with both PHPUnit 8 and 9)
2. create a 2.x branch from master
3. merge this patch into master
4. tag a new 3.0.0 release

To upgrade from PHPUnit 9 to 10, the process will then be:

1. Upgrade phpspec/phpunit-prophecy to v2.0.3
2. Upgrade to PHPUnit 10 (deprecations related to static data providers will remain)
3. Upgrade phpspec/phpunit-prophecy to v3.0.0
4. Change calls to $this->prohesize() to self::prophesize() in data providers
5. Make data providers static to fix remaining deprecations

@stof if you need help on this, I can do it

[stof]

This change is broken.

Prophecy does not make a distinction between mocks (which can have expectations to check) or stubs (which cannot have them). Any double created in the data providers would not be properly associated to the Prophet object using during the test run (things could be even worse than that as the static property might make it associated to the first test being run in the class, even though it would not even be associated to that data provider).

Making prophesize static would be kind of equivalent to making createMock static in PHPUnit, which is also not the case (as it would have similar issues)

[mondrake]

We also hit this need in the Drupal community, in our journey to support PHPUnit 10. On top of the need to support a static context, there's also a problem with checking expectations on number of calls on mocks. In https://www.drupal.org/project/drupal/issues/3368713, we are discussing to create a singleton hubbing all the Prophet objects created in dataproviders, and invoking each during the teardown to abide with Prophecy's recommended implementation.

Would be good if we could have an 'official' way to do it supported here.

[stof]

Checking the number of expected calls requires the double to be associated with the test, and this is the primary reason why this is not supported in data providers.
If you look at the PHPUnit mock objects, createStub is static now, but createMock (which support such expectations) is not.

[stof]

@dunglas I don't understand your comment. You are saying that this PR is ready to be merged because some PR has been rejected. But the rejected PR you link to is this one.

[jdreesen]

I think he refers to sebastianbergmann/phpunit#5452

[BafS]

We started using Prophecy to ease the upgrade to PHPUnit 10 and I now realize it's not compatible with PHPUnit 10, what is the status of this PR?

[stof]

This PR is not about compatibility with PHPUnit 10. Prophecy does not make a distinction between mocks and stubs in its API and so should not be used in data providers just like PHPUnit mocks should not be used there (and PHPUnit's createMock is not static either)