search

StoutLogic / acf-builder

An Advanced Custom Field Configuration Builder
GNU General Public License v2.0
792 stars 61 forks source link

Upgrade unit tests to work with php 8 #168

Closed michaelw85 closed 1 year ago

michaelw85 commented 1 year ago

This upgrade works with PHP 7.4, 8.0, 8.1 and 8.2

PHP unit versions + supported PHP version: https://phpunit.de/supported-versions.html

What has been changed: \PHPUnit_Framework_TestCase Needs to be replaced with namespaced TestCase. assertArraySubset has been removed see ticket, I've added a package re-adding it via trait. prophesize is not included in phpunit anymore, as of phpunit 9 you will have to install it yourself and use a trait, which I did but this library supports PHP 7.3 as a minimum. assertInternalType Is deprecated expectedException Should be written with an assertion

michaelw85 commented 1 year ago

Scrutinizer fails on composer install since it's running PHP 7.2, for this branch it should at least run 7.4. I would like to update the setting but I don't think the configuration is part of the project or is it @stevep ?

stevep commented 1 year ago

@michaelw85 Thank you for your work on this. This is huge. I've added a scrutinizer config file to the repo, so that works now.

Trying to decide if the library should just require 7.4 going forward or not. And if so, it probably warrants a 2.0 version number since it's breaking backwards compatibility. I have similar thoughts on the #167 PR, with NamedBuilder being removed. I don't know why a user's or lib's code would be dependent on it directly, but I can't be sure that they aren't.

That being said I'll move this to master, as well as #167 to master, and think about the release number. Any thoughts on the matter?

Thanks again. (and sorry for the delay)

michaelw85 commented 1 year ago

@stevep Thank you for merging! I wasn't sure you would be happy with such a big change 😅 For PHP version support I would even consider dropping 7.4, officially it's been end of life since November. Even 8.0 only has 9 months left. I think PHP is pushing harder on updates. Also seeing the support on PHP Unit I think it's good to move in the same direction.

On the versioning, I think it makes sense to bump the major version number when dropping support for older PHP versions. If you still want to support the older PHP versions you can branch, patch, and release version 1.x.y. I don't think you should but you can (better to have the ability than to get stuck in my opinion).

Technically I think a minor release would be enough, I don't think composer would update the package if your PHP version does not meet the new constraints. I think it always gets the latest possible version unless you start forcing or when it's a dependency of another package.

Hope this helps.

P.S I added some packages for unit testing to add support for things that were dropped. Do you want to keep it running with these packages or would you rather refactor the tests and remove the added packages? I'm referring to assertArraySubset and prophesize. For me, these packages feel like temporary bandaids.