Added support for phpunit 9 #19
Conversation
|
Oh, thanks! I'll take a look ASAP. 👍
|
| ], | ||
| "require": { | ||
| "php": "^7.2", | ||
| "php": "^7.2 || ^7.3", |
There was a problem hiding this comment.
This is not necessary; ^7.2 already translates to >= 7.2 && < 8.0
There was a problem hiding this comment.
You are right. I am used to gracefully update dependencies in libraries due to backwards compatibility, but this is not necessary here.
There was a problem hiding this comment.
You are right. I am used to gracefully update dependencies in libraries due to backwards compatibility, but this is not necessary here.
Upon further inspection, I do think it is necessary as with this notation we still allow users with PHP 7.2 to make use of this package with PHPUnit 8, from 7.3 PHPUnit 9 is needed (as it is the minimum requirement).
composer.json
Outdated
| }, | ||
| "conflict": { | ||
| "phpunit/phpunit": "<8.0 || >= 9.0" | ||
| "phpunit/phpunit": "<8.0" |
There was a problem hiding this comment.
This was originally a safe-guard against this package being used with (future) versions of PHPUnit that it was not tested against. Even if we now support PHPUnit 9, the same argument can still be made for the next upcoming major versions.
Let's make this future-proof:
| "phpunit/phpunit": "<8.0" | |
| "phpunit/phpunit": "<8.0 || >= 10.0" |
| }, | ||
| "autoload-dev": { | ||
| "files": [ | ||
| "vendor/phpunit/phpunit/src/Framework/Assert/Functions.php", |
There was a problem hiding this comment.
The functional interface is still alive and well in PHPUnit 9. I don't see the need for removing this include and rewriting the tests in the later files. Can you share your thoughts on why this change was necessary?
There was a problem hiding this comment.
The functional interface is still alive and well in PHPUnit 9. I don't see the need for removing this include and rewriting the tests in the later files. Can you share your thoughts on why this change was necessary?
I removed the autoloading as the file was autoloaded twice and so it threw a redeclaration exception. I now imported the functions in the unit test suites to make use of the functional interface. I hope this is fine for you.
|
@martin-helmich As I already mentioned, we do not need to manually autoload the functions of PHPUnit anymore as they are now namespaced. This unfortunately leads to the problem that it is imho not possible to keep the next version backwards-compatible with PHPUnit 8, as I need to change the namespaces when using the functional interface. Should I get rid of PHPUnit 8 in this PR and only keep PHPUnit 9 (which also would mean to drop support of PHP 7.2 from here on, as PHPUnit 9 requires PHP 7.3)? |
|
@martin-helmich Did you find the time to look at my comments? Would be great to get this sorted. |
|
@martin-helmich Ping :) |
|
Oh sorry, I kind of lost track of this one. I'll have another look at this ASAP (and also try to think of something regarding backwards-compatibility). |
|
This is a blocker for upgrading https://github.com/martin-helmich/phpunit-psr7-assert. Perhaps there is a better way, as this disallows to upgrade PHPUnit which has a new major every year. |
|
@martin-helmich We could tag this version with the new namespaces as a new major version (4.0). Everything related to PHPUnit 8 will be a minor version release in 3.x. In https://github.com/martin-helmich/phpunit-psr7-assert, we do the same, introducing a new major release version for PHPUnit 9. |
|
So! Finally getting to this. I've forked this pull request into my own branch in #20 so that I could put some of my own changes on top before merging. When commenting, I did not realize that PHPUnit's functional interface is now namespaced, which essentially requires a breaking change -- thanks for pointing that out. I've decided to forego using PHPUnit's functional assertions (so I've reverted aa56581) to keep compatibility with PHPUnit 8 and 9. |
3 participants
@martin-helmich I added support for phpunit 9.