The danger of tunnel vision

on
0

Upgrading your library: a neccessary evil!

on


Recently I have finally upgraded my library to the latest Symfony and Laravel versions. I want to get to the first release candidate of Apie and I can not afford coming with a new library if it is not up to date. PHP 8.4 also has a few nice new additions that could be very interesting to add in Apie.

Personally I thought the upgrade would be an easy one as my library relies very little on framework-specific code, but I've been wrong here. The CI is very well set up and strict and I can be sure everything works the moment everything it succeeds, but it did take a few days to update everything. So let's see how it went. To update I needed to update these packages on git:

PHP minimum requirements

My library has a PHP8.1 minimum requirement. The latest versions of Symfony and Laravel require a higher PHP version (8.3). So I had 2 options: support both versions or increase my PHP version requirements. I decided to increase the minium requirement. As I was busy with the upgrade it seems to be the best solution as many things have changed that would otherwise result in a very messy code business. Previously I only had extra conditionals on readonly classes and true typehints.

Upgrading PHP

PHP upgrade was quite straight forward. I changed the monorepo(see my article in how I set up the monorepo) to support PHP 8.4 to start with. This went quite smooth. It resulted in a few php deprecation notices, but they were ignored in the tests for now. Because I dropped PHP 8.1 and 8.2 the workflow also becomes faster as it now only tests in PHP 8.3 and PHP 8.4.
However in the end I did end up with a struggle with PHP 8.4. I test my code with the lowest version constraints and the latest version constraints in Composer with the matrix option in Github actions, but no matter what I did I could not get it to work to run all the tests in PHP 8.4 with the minimum version constraints, because most of them threw deprecation notices in PHP 8.4. I could fix most of them with setting up conflict constraints in composer.json (so your package has no direct dependency, but it can not coexist with this version in the same project). Even though there was luckily not a conflict with PHP 8.3 and PHP 8.4 and a specific library version, I decided to discard the combination PHP 8.4 with lowest version settings as it made very little sense to me.

Upgrading Symfony components

Upgrading Symfony components proved to be more a hassle than I expected. Many composer packages support both 6.* and 7.*, so often if Apie did not have a dependency to the 7.* dependency directly it would end up installing 6.* which could result in some weird version combinations.
Once I added a few conflicts in the composer.json I could end up with working symfony components.

Upgrading Laravel

I was expecting very little as I use practically nothing from Laravel specific and most things were already fixed when I updated Symfony components which Laravel depends on. The biggest change was the ApieUserDecorator class that implements Laravel's UserInterface as it adds 2 useless methods to the interface (again). The problem is that my decorator has no public properties so I returned an empty string there and that seems to work just fine!

Upgrading PHPUnit

The tests were still running in PHPUnit 9, but the latest version is in 11. I'm forced to update because of Laravel's test library orchestra/testbench, but it's good to update anyway. Many upgrades are improvements or fixes of misdesigns in PHPUnit. PHPUnit itself is mainly developed by one developer. Even though Sebastian Bergmann is a brilliant programmer, some of his choices were not smart choices when introduced. Then when he finds out this was a wrong solution/paradigm, he switches it in the next major version of phpunit., which means a large refactoring for anyone using phpunit.

Static data providers

It never made sense that data providers are not static, but in version 11 making it non-static results in a phpunit deprecation notice. You could fix it by hand or by using a rector script. I did it by hand as the number of data providers were limited. I did have to change apie/phpunit-matrix-data-provider as the methods were not static.

PHP Attributes

PHP Attributes were introduced to add metadata to classes, properties and methods so a PHP library no longer has to parse PHP docblocks for additional metadata (which was the norm with libraries like PHPUnit and Doctrine). Still PHPUnit still took some time to replace them. In version 12 only attributes will work, so I had to change them. I used rector to modify all tests instantly from docblocks to PHP attributes. Rector did a great job in converting data providers and methods with test docblocks. However it seems it ignored a few tests that had docblocks to indicate a required php version or extension, so I had to manually fix those.

Before:

class MyTest extends TestCase
{
    /**
     * @test
     * @dataProvider provideSomething
     */
    public function testSomething(string $input)
    {
    }
    
    public function provideSomething(): Generator
    {
         yield 'empty string as input' => [''];
    }
}

After:

class MyTest extends TestCase
{
    #[\PHPUnit\Framework\Attributes\Test]
    #[\PHPUnit\Framework\Attributes\DataProvider('provideSomething')]
    public function testSomething(string $input)
    {
    }
    
    public static function provideSomething(): Generator
    {
         yield 'empty string as input' => [''];
    }
}

PHPUnit sees if an error handler is changed

Symfony and Laravel both use their own global error handler. And that is not fine when Phpunit expects its own error handler being active. Since it's global, there can only be one error handler active at the same time. Previously this was not causing any issues, except a possible error when all tests ran or that an error that PHPUnit should have retrieved was caught by the framework error handler. In some other cases you will end up with a test that depends on the error handler of a previous testcase, but luckily I did not run into any issues here.

Symfony and Laravel fixed it in their own webtestcase, but we have not used this to make the same test run in Laravel as we run it in Symfony (see a previous article how I tackled this), so I had to tackle this myself. For one test the error was always thrown, so I added the attribute WithoutErrorHandler to handle this test. For the rest I could get away by adding a teardown method that is being called because it has a After attribute:


#[\PHPUnit\Framework\Attributes\After()]
public function __internalDisableErrorHandler(): void
{
    restore_exception_handler();
}

PHPUnit XML format update

Oh boy, this one took me long to figure out. PHPUnit is good at throwing cryptic error messages. After running the tests, the test failed because the coverage configuration was not set, but if you check phpunit.xml you get PHPUnit telling me it's valid and you do see a <coverage>-part?
After some googling I found out that I had to migrate the phpunit XML configuration to the format for 11. This was clearly PHPUnit on his worst.
Luckily there is a migrate configuration command, but it still requires some manual modification because there were some changes related to php notices and deprecation notices.
The main reason for the change is valid: it allows you to mark which folders should contain your code and which contains vendor code, so it could do better detection what deprecation notices come from your code.
The downside is that PHPUnit will not complain when you use a version 9 version of the XML. It will run XML validation on it, but then it misses required fields leading to very confusing error messages.

Doctrine

Doctrine is used internally to run the database migrations automatically. And I can conclude the integration tests should be better as the tests succeeded, but in reality there were some problems with the update. The biggest one is that column type 'string' should always have a length. In case it's infinite length you have to use 'text'. There was also a small issue in the configuration in prioritizing the driver option over the DSN/url option. Those were luckily the only things that broke.

Conclusion

Nobody likes upgrades, but it is needed even if it could take you more time then you want. In case you do not want to spend time on it, then the only option is not adding external dependencies, which is much more time consuming. I'm happy with the CI of Apie as the moment the tests succeeded I could even update it in existing projects without much hassle. I'd rather fix everything before and not after you find a bug and have to fix it ASAP in multiple projects. Don't be afraid for updates and make sure you stay up to date.

Some guidelines:
  • make sure you have a good test coverage. It does not have to be 100%, but good enough to give you confidence your changes will not break existing functionality.
  • Try to write your code as framework agnostic. Framework updates often break stuff even when documented. Try to avoid extending classes from a framework library.
  • Add conflicts in composer.json for indirect dependencies to avoid that an old version is loaded.
  • Don't just add a new version number of a library or PHP version and think that's enough. Make sure you really do support multiple versions by testing the lowest version constraints as wall.

Comments

Post a Comment