Support older versions of PHPUnit

[drzraf]

My symfony-based project depends on symfony/phpunit-bridge which download & install phpunit dynamically at runtime.
I'd rather prefer not to pull phpunit/phpunit when requiring phpunit-arraysubset-asserts.

[m4rcinkowski]

What is more, PhpUnit is currently required in version 9 which breaks my current test suite.
This makes use of this extension impossible for me.

[spaceemotion]

We also wanted to use this package, but we're still on PHPUnit 7. Since we know about the deprecation we didn't want to write "outdated" tests.

[jrfnl]

I'm running into the same problem.

Instead of dropping the requirement, I'd like to suggest widening it instead - I'd suggest 5.7 - 9 - and using a custom autoloader instead of the Composer autoloader (the custom autoloader would still be loaded via autoload - files) and basically loading "empty" versions of the classes/trait provided when the polyfill is not needed.

That way test suites which use this package can add the import and trait use statements at will and the PHPUnit native functionality will be used when available, and if not, this package will kick in.

The PHP 7 features (strict types, type declarations) would not need to be removed as those "versions" of the classes would only be loaded when the functionality isn't natively available in PHPUnit, i.e. when on PHPUnit 9 or higher, which requires PHP 7.3 anyway.

Just the "empty" classes and the autoloader would need to be compatible with older PHP versions.

[rdohms]

I think we have 2 separate issues here. One is the original report of using this with symfony/phpunit-bridge for which the only solution is to not rely on the package.

And 2: widening its ability to work with earlier versions of PHPunit.

Regarding 1: I think phpunit-bridge should report provides phpunit/phpunit correctly otherwise we maintainers of plugins are forced to not declare a dependency which we have to fit their model. Would love to see if we can find another way to solve this.

Regarding 2: @jrfnl can you show me an example of what this autoloader would look like? Also there are some changes to Constraint that are only available for 8+. So we would really be making this a polyfill not just a specific plugin for the asserts. Is there any way in which we can rollout a separate thing package that requires this one and provides the polyfilling?

Widening the dependencies here would need to applied to PHP and PHPUnit and I'm not really comfortable with having to keep older code supported, especially PHP versions as that's not something I have on my day-to-day, so would need extra testing and effort, and might hamper the new releases I'm planning around expanding support for indexed arrays.

[jrfnl]

can you show me an example of what this autoloader would look like?

What I envision would basically come down to something along the lines of (simplified, but you get my drift I hope):

spl_autoload_register(function($class)) {
    if ($class === 'DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts') {
        if (method_exists( 'PHPUnit\Framework\Assert', 'assertArraySubset' ) === false) {
            require_once __DIR__ . '/src/ArraySubsetAsserts.php';
        } else {
            require_once __DIR__ . '/src/ArraySubsetAsserts_Empty.php';
        }
    }
});

where the content of src/ArraySubsetAsserts_Empty.php would be something like:

<?php

namespace DMS\PHPUnitExtensions\ArraySubset;

trait ArraySubsetAsserts {}

Widening the dependencies here would need to applied to PHP and PHPUnit and I'm not really comfortable with having to keep older code supported

That is actually not much of an issue if the actual polyfill code is only loaded if the functionality is not available natively in PHPUnit.
As I stated above, the widened PHP version requirement would in reality only apply to the autoloader, not to the actual polyfill code.

Also there are some changes to Constraint that are only available for 8+. So we would really be making this a polyfill not just a specific plugin for the asserts. Is there any way in which we can rollout a separate thing package that requires this one and provides the polyfilling?

Not 100% sure what you mean by this, but I'm currently working on a package which will polyfill nearly all other other changed assertions [Edit: and exception methods] in PHPUnit 5.7 - 9, which will use the same principle as I outline above. Does that help ?

[jrfnl]

@rdohms Had a chance to think things over ?

[jrfnl]

FYI: the other repo I was talking about is now public and expected on Packagist later today: https://github.com/Yoast/PHPUnit-Polyfills

[rdohms]

@jrfnl ok let's give it a go, if we run into stuff that gets harder down the line, we can see how to best spend efforts.

Can you get started with that PR?

[torotil]

In PHPUnit 8 assertArraySubset() is still available but triggers a deprecation warning. It would be great to remove the warning as well.

[jrfnl]

Can you get started with that PR?

@rdohms Will do, may be a little while before I get to it now.

[jrfnl]

In PHPUnit 8 assertArraySubset() is still available but triggers a deprecation warning. It would be great to remove the warning as well.

@torotil That's quite a different issue. I think that should be addressed separately (if at all).

[torotil]

@torotil That's quite a different issue. I think that should be addressed separately (if at all).

I don’t think so. Let me explain: If I have code using assertArraySubset() and I’m currently using PHPUnit 8 then I get this deprecation warnings. Once I’m using a/this polyfill (which will kick in as soon as I upgrade to PHPUnit 9) the warnings are actually false positives: The asserts will still work after the upgrade. So if this Polyfill was backported to earlier PHPUnit versions (including 8.*) it should also remove the deprecation warning there.

[jrfnl]

So if this Polyfill was backported to earlier PHPUnit versions

And that's exactly why this is a different issue and should have its own discussion.

What's being discussed here is the ability for the package to be installed in combination with earlier PHPUnit versions, not for the polyfills to be active in earlier versions, which would effectively overrule the PHPUnit native implementation.

[jrfnl]

@rdohms Ok, so, in contrast to what I said earlier, this issue stayed on my mind yesterday, so to prevent it from distracting me further, I have got a proof-of-concept ready.

More than anything, I think I need some feedback on naming preferences for the "empty" versions of files, but I got it all working, including "availability tests", with the current minimums set to PHP 5.5 / PHPUnit 4.8.36.

I'll tidy up the commits and then I'll open a draft PR for discussion on the implementation details.

[torotil]

And that's exactly why this is a different issue and should have its own discussion

Ah, sorry. Then I misinterpreted the title of the issue (“Support older versions of PHPUnit”). Should I open a new one? What should be its title?

[drzraf]

Can't the composer.json requirement of PHPUnit be simply dropped and make it a suggest instead? Seems simple and would just work.