search

phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 242 forks source link

Suggestion: improve test workflows #629

Open jrfnl opened 3 months ago

jrfnl commented 3 months ago

In my opinion, as Prophecy is used in the CI workflow of other projects, it should be ready early for new PHP versions and be free of PHP deprecation notices.

As things are, I see the following improvements which IMO should be made to the CI/PHPUnit side of things to better guarantee the stability of Prophecy for use with PHPUnit:

I'm willing to invest the time to make these changes, but would like to know if the project maintainers are open to this before I invest the time.

stof commented 3 months ago

Given that most tests are not written using PHPUnit at all, I'm not sure how useful it would be to use multiple versions of PHPUnit.

For deprecations, I agree about that. However, I'm not sure we can make phpspec fail the build on deprecations.

jrfnl commented 3 months ago

@stof I'm not very familiar with PHPSpec, so I'd need to look into that, but for PHPUnit, it would mean either composer scripts with different CLI settings for PHPUnit < 10 and PHPUnit 10+ or two different config files.

In other repos, I've normally gone with a combination - the different config files + composer scripts - to make it as obvious as possible for contributors how to get the same CI results locally.

Would that be acceptable to you ?

stof commented 3 months ago

@jrfnl I'm not asking how to use multiple versions of PHPUnit, but which benefit this would provide (especially given that this repo is not the one providing an integration between Prophecy and PHPUnit, as that happens in https://github.com/phpspec/prophecy-phpunit)

jrfnl commented 3 months ago

@jrfnl I'm not asking how to use multiple versions of PHPUnit, but which benefit this would provide

Well, you are already using multiple versions of PHPUnit, but the configuration is not set up to fail builds on deprecation notices. That's what I was referring to above.

(especially given that this repo is not the one providing an integration between Prophecy and PHPUnit, as that happens in https://github.com/phpspec/prophecy-phpunit)

That's fair enough, but even if the fourth point from the list above is dropped, the configuration difference between PHPUnit < 10 and 10+ would still need to be handled.

On another note: I've just been looking at PHPSpec and unfortunately their docs site is down (known issue), but running the PHPSpec tests locally against PHP 8.4 shows that nearly all tests would currently fail due to the implicitly nullable deprecation. Looking closer, it looks like PHPSpec itself needs fixes first.

jrfnl commented 3 months ago

I've pulled fixes to PHPSpec now for their issues: https://github.com/phpspec/phpspec/pull/1472