Skip to content
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

Suggestion: improve test workflows #629

Open
jrfnl opened this issue Aug 20, 2024 · 5 comments
Open

Suggestion: improve test workflows #629

jrfnl opened this issue Aug 20, 2024 · 5 comments

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Aug 20, 2024

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:

  • Enable showing deprecation notices.
  • Enable failing the build on deprecation notices.
  • Start running the tests against PHP nightly early.
  • Run against the latest + nightly PHP versions with multiple PHPUnit versions, i.e. PHPUnit 9 and 10, not just 10.

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
Copy link
Member

stof commented Aug 21, 2024

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
Copy link
Contributor Author

jrfnl commented Aug 21, 2024

@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
Copy link
Member

stof commented Aug 21, 2024

@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
Copy link
Contributor Author

jrfnl commented Aug 21, 2024

@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
Copy link
Contributor Author

jrfnl commented Aug 21, 2024

I've pulled fixes to PHPSpec now for their issues: phpspec/phpspec#1472

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants