GH Actions: various updates

[jrfnl]

GH Actions: version update for actions/checkout

Ref: https://github.com/actions/checkout/releases

GH Actions: enable error reporting

The default setting for error_reporting used by the SetupPHP action is error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT and display_errors is set to Off.

For the purposes of CI/test runs, I'd recommend running with -1 and display_errors=On to ensure all PHP notices are shown.

GH Actions: fix typo

PHPUnit: update configuration

PHPUnit >= 9.5.10 and >= 8.5.21 contain a particular (IMO breaking) change:

• PHPUnit no longer converts PHP deprecations to exceptions by default (configure convertDeprecationsToExceptions="true" to enable this)

Let's unpack this:

Previously (PHPUnit < 9.5.10/8.5.21), if PHPUnit would encounter a PHP native deprecation notice, it would:

1. Show a test which causes a deprecation notice to be thrown as "errored",
2. Show the first deprecation notice it encountered and
3. PHPUnit would exit with a non-0 exit code (2), which will fail a CI build.

As of PHPUnit 9.5.10/8.5.21, if PHPUnit encounters a PHP native deprecation notice, it will no longer do so. Instead PHPUnit will:

1. Show a test which causes a PHP deprecation notice to be thrown as "risky",
2. Show the all deprecation notices it encountered and
3. PHPUnit will exit with a 0 exit code, which will show a CI build as passing.

As this repo is generally used in CI by other repos, IMO it is important to fix deprecations early to prevent being the reason for workflows of users of this repo to fail.

This commit reverts PHPUnit to the previous behaviour by adding convertDeprecationsToExceptions="true" to the PHPUnit configuration.
It also adds the other related directives for consistency.

Refs:

• https://github.com/sebastianbergmann/phpunit/blob/9.5/ChangeLog-8.5.md
• https://github.com/sebastianbergmann/phpunit/blob/9.5/ChangeLog-9.5.md

GH Actions/CI: run the test suite against PHP 8.1 and PHP 8.2

I intended to just add a build against PHP 8.2, but realized the tests aren't run against PHP 8.1 yet when looking at the script.

Both the PHP 8.1 test run as well as the PHP 8.2 test run will currently fail. Some of my other open PRs are stepping stones to fix this.

Once those open PRs are merged, there should be just five failures left on PHP 8.1, which I'm leaving for someone else to address.
As for PHP 8.2: the remaining test failures there look to be mostly related to external dependencies (Prophecy, Symfony/Console). Once those have released PHP 8.2 ready versions, it can be evaluated if there is anything more that needs addressing in this repo.

🆕 GH Actions: disallow for Symfony/Flex composer plugin (updated)

As discussed in the ticket, as of July 1st 2022, Composer requires explicit permission to be given for Composer plugins to be run when using Composer 2.2+.
See: https://blog.packagist.com/composer-2-2/#more-secure-plugin-execution

On top of that the website which Symfony Flex tries to access during its install is not responding anymore when using old Flex versions in which those API calls haven't been updated yet.
Also see: https://symfony.com/blog/upgrade-flex-on-your-symfony-projects

Combining the above two issues and having done some test runs on GH Actions, I've come to the conclusion that not allowing the symfony/flex plugin and disabling the running of plugins during the install step solves the problems without any impact on the tests.

So that is what this commit now does.

🆕 Composer: limit allowed PHPUnit 9.x version

The PHPUnit 8.5.29/9.5.23 patch update removed the dependency on Prophecy, which means that projects using Prophecy now need to require it themselves:

sebastianbergmann/phpunit 5033: Do not depend on phpspec/prophecy

See: https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-9.5.md

This, however, has its own problems, what with the phpspec/propephy-phpunit layer not allowing installation with PHPUnit < 9.

So, for now, this limits PHPUnit to a version before the dependency was dropped.

[keradus]

do you know why CI is failing and how to make it green, perhaps?

[jrfnl]

do you know why CI is failing and how to make it green, perhaps?

Yes, this has to do with Composer 2.2+ which now requires that you give explicit permission for Composer plugins to be run.
See: https://blog.packagist.com/composer-2-2/#more-secure-plugin-execution

For the first six months after the release you could still ignore this, but as of July 1st, that's no longer the case.

I cannot decide for you whether or not you want to allow the plugin included with Symfony Flex to run.

This is in the GH Actions log for the Install Symfony Flex step:

Error: symfony/flex (installed globally) contains a Composer plugin which is blocked by your allow-plugins config. You may add it to the list if you consider it safe.
You can run "composer global config --no-plugins allow-plugins.symfony/flex [true|false]" to enable it (true) or disable it explicitly and suppress this exception (false)
See https://getcomposer.org/allow-plugins
In PluginManager.php line 744:
                                                                               
  symfony/flex (installed globally) contains a Composer plugin which is block  
  ed by your allow-plugins config. You may add it to the list if you consider  
   it safe.                                                                    
  You can run "composer global config --no-plugins allow-plugins.symfony/flex  
   [true|false]" to enable it (true) or disable it explicitly and suppress th  
  is exception (false)                                                         
  See https://getcomposer.org/allow-plugins     

So to fix this, the Install Symfony Flex step needs to be updated. I can do so, but need to know whether you want it set to true or false:

            -   name: Install Symfony Flex
                if: startsWith(matrix.php, '5') == false
                run: |
                    composer global config allow-plugins.symfony/flex [true|false]
                    composer global require -o --no-interaction symfony/flex:${{ steps.flex-version.outputs.result }}

[jrfnl]

Note: PR #344 is also still needed to get a passing build again.

[keradus]

I would be happy with any direction for flex. would you mind fixing it, as you know the problem here?

[jrfnl]

I would be happy with any direction for flex. would you mind fixing it, as you know the problem here?

Done. I've checked the source of Symfony Flex and it is basically a Composer plugin, so the permission is needed.

However, now we run into the next problem, which is a breaking change introduced last week in the PHPUnit 9.5.23 patch update:

sebastianbergmann/phpunit#5033: Do not depend on phpspec/prophecy

See: https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-9.5.md

This means that Prophecy will either need to be added to the composer.json file with a wide range of supported versions, or we need to add it in CI for those builds running on PHPUnit 9.x.

What would you prefer ?

Note: I haven't checked if adding Prophecy when combined with PHPUnit versions which include Prophecy natively will be problematic, so that could bring yet another challenge.

[jrfnl]

And for the record: I've rebased the PR on master so the current state of things could be evaluated.

Also made two tiny tweaks to the pre-existing commits:

• "Error reporting": I've added zend.assertions=1 to the ini-values enabled as that's often used in tests, so better to include it.
• "Run tests against PHP 8.1/8.2": I've tweaked the name of the additional step a little to make it unique.

[jrfnl]

I'm also seeing an issue still with PHP 7.0, which may be a hickup in GHA or may need to be addressed:

 Package operations: 1 install, 0 updates, 0 removals
  - Installing symfony/flex (v1.6.3): Downloading (connecting...) Downloading (100%)         
Plugin installation failed (The "https://flex.symfony.com/versions.json" file could not be downloaded: php_network_getaddresses: getaddrinfo failed: Name or service not known
failed to open stream: php_network_getaddresses: getaddrinfo failed: Name or service not known), rolling back
  - Removing symfony/flex (v1.6.3)

Installation failed, reverting ./composer.json to its original content.

                                                                                                                                                      
  [Composer\Downloader\TransportException]                                                                                                            
  The "https://flex.symfony.com/versions.json" file could not be downloaded: php_network_getaddresses: getaddrinfo failed: Name or service not known  
  failed to open stream: php_network_getaddresses: getaddrinfo failed: Name or service not known      

[jrfnl]

However, now we run into the next problem, which is a breaking change introduced last week in the PHPUnit 9.5.23 patch update:

sebastianbergmann/phpunit#5033: Do not depend on phpspec/prophecy

See: https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-9.5.md

This means that Prophecy will either need to be added to the composer.json file with a wide range of supported versions, or we need to add it in CI for those builds running on PHPUnit 9.x.

Okay, so I've looked into this a little....

The (old) recommendation is to require phpspec/prophecy-phpunit. As of PHPUnit 9.5.23, the recommendation is to require phpspec/prophecy.

Using Prophecy-PHPunit is not an option at this time.

• Their 1.x version which supports PHP < 7.3 has a significantly different implementation than the 2.x version needed for PHPUnit 9.x.
• Version 1.x basically provides a TestCase to let your tests extend from, while version 2.x provides a trait to use in the tests, but doesn't allow installation unless combined with PHPUnit 9.x.
• I have a long-standing PR open to widen the PHPUnit installation restrictions for the package, but there is no movement on it, even though I've made all the updates requested by the maintainer.

Using Prophecy itself does seem to work, but will still generate PHPUnit warnings about the use of prophesize() and 5 failing tests on PHP 8.1/8.2. Those will need further investigation.

While all of this is nice and dandy, IMO it feels very above and beyond to expect a random contributor to solve all of this for you.

Maybe it is time for maintainers to start maintaining again ?

[keradus]

Big thanks for all the effort added here!

While all of this is nice and dandy, IMO it feels very above and beyond to expect a random contributor to solve all of this for you.
Maybe it is time for maintainers to start maintaining again ?

I am definitely not expecting you or anyone else to solve everything for me.
Not random ppl, not all, and not for me - if you want to contribute to improve the project, please go ahead, but if you do it only for me, then don't.

Both creators of this project dropped it quite a few years ago. I took over the rights to click "merge" and "release" to help the project not going dead, but I'm far from putting the maintenance of it as my main duty, due to lack of time.

If anyone wants to contribute, that's really appreciated ;)

[jrfnl]

if you want to contribute to improve the project

I'm happy to do so, but it is a bit difficult to show that you're improving something if even the most basic CI is not working and needs multiple layers of fixes to get it running again (which is what I started to do here, but after 4 months of the PR not getting any attention, yet more fixes are needed).

I took over the rights to click "merge" and "release" to help the project not going dead, but I'm far from putting the maintenance of it as my main duty, due to lack of time.

I appreciate that you took it on and I understand where you are coming from (what with me maintaining quite some projects myself).

All the same, from a contributor perspective, a project always getting a failing build and PRs being left open for a long time does not encourage continued contributions.

[keradus]

Send me a priv if you would be interested in having merge rights ;)
if not really and you prefer to focus on concrete PR, I'm happy with most directions, probably - eg Adapter TestCase that works differently depends on installed PHPUnit version or temporarily forbidding too-new PHPUnit version

[jrfnl]

Send me a priv if you would be interested in having merge rights ;)

I really don't Have enough of those already, thanks.

if not really and you prefer to focus on concrete PR, I'm happy with most directions, probably - eg Adapter TestCase that works differently depends on installed PHPUnit version or temporarily forbidding too-new PHPUnit version

I suppose the most minimal solution for now would be to limit to PHPUnit < 9.5.23 in the composer.json.

That should get most builds passing, though PHP 7.0 still needs looking into and PHP 8.1 and 8.2 still have some test failures which need investigation, but it gets you 80% of the way to a passing build again.

[jrfnl]

Okay, so I've spend some more time on the PHP 7.0 build. The problems there are with the website which is being called during install no longer responding.

Also see: https://symfony.com/blog/upgrade-flex-on-your-symfony-projects

I've now done a number of test runs with the plugin not allowed + the --no-plugins option on the install and that seems to get round these problems. I'll update the previous commit which added the permission to instead disallow the plugin.

To be honest, as I'm not a maintainer of this package, I have no idea why Symfony Flex is even needed for the test suite.... ?

[jrfnl]

Okay, so PHP 5.5-8.0 are now all passing again. PHP 8.1 and 8.2 still have test failures.

I could remove the commit which added those builds from this PR (and pull it separately) to allow this PR to be merged.

I'm not keen to start investigating and fixing those last test failures. I've done my fair share for now I'd say.

[keradus]

Agree. big thanks for all the changes! Feel free to raise more if having a mood for it again ;)

[jrfnl]

Agree. big thanks for all the changes! Feel free to raise more if having a mood for it again ;)

So do you want me to remove the commit adding PHP 8.1/8.2 for now so this can be merged ? Or is it going to stay open until someone fixes those tests at some point ?

[keradus]

Indeed, let's drop 8.1/8.2 from this PR and add it back when anyone would be up for fixing it.
kudos for all the great work here!

[jrfnl]

Thanks for the merge @keradus! At least the project is a couple of steps closer to compatibility with my earlier PRs and the CI is passing again, so hopefully that will lower the barrier for others to contribute.