search

spatie / enum

Strongly typed enums in PHP supporting autocompletion and refactoring
https://docs.spatie.be/enum/v3/introduction
MIT License
771 stars 67 forks source link

PHPUnit assertions trait changed to class #114

Closed gbuckingham89 closed 2 years ago

gbuckingham89 commented 2 years ago

It seems that this commit changed the trait that provided PHPUnit assertions into an abstract class: https://github.com/spatie/enum/commit/45245a12d424d08b7191038f4ac7375afd4feb80

I've got a project on PHP 7.4 with spatie/enum v3.10.0. I ran acomposer update this morning and was given v3.11.1. I'm now getting a fatal error when running my tests:

PHP Fatal error:  Tests\TestCase cannot use Spatie\Enum\Phpunit\EnumAssertions - it is not a trait in /home/runner/work/example.com/example.com/tests/TestCase.php on line 19

I can obviously refactor my tests to use the static methods on what is now the class - but should this not have been documented as a breaking change and a new major version released?

brendt commented 2 years ago

Ah, I wasn't aware that people were using this trait in such a way, that's my bad.

PHP 8.1 made a change so that calling static methods on traits isn't possible anymore. I'll fix it!

brendt commented 2 years ago

@Gummibeer this is actually a difficult one to fix. We do want to support 8.1 with the current version to allow people for a more smooth upgrade path when we tag our new major. But PHP 8.1 disallowed static calls on traits.

I also don't want to tag a new major version…

Thinking about it: @gbuckingham89 why exactly are you using the trait in your test classes? These are all static methods and can't be called on the instance, so in my mind it didn't make any sense to ever use this trait?

brendt commented 2 years ago

Checking the README, it only shows examples of using this trait statically and not by importing it. That's why I don't think this should be considered a breaking change: it was never intended to work this way.

Gummibeer commented 2 years ago

So the breaking change comes from PHP itself. Because of that I would also stick to the promoted usage in the README.

gbuckingham89 commented 2 years ago

I've just checked the older versions of the README and as you say, it's always been documented that the assertions should be called statically.

It was obviously implemented wrong in our codebase - so apologies for any confusion!

brendt commented 2 years ago

No problem!