Skip to content

Conversation

@mikespub
Copy link
Contributor

Support phpunit 11+

@oscarotero oscarotero requested a review from filisko May 10, 2025 17:36
@filisko
Copy link
Member

filisko commented May 10, 2025

@mikespub thanks! Will check it tomorrow 👍

@mikespub
Copy link
Contributor Author

@mikespub thanks! Will check it tomorrow 👍

Thanks. No changes were needed to support the tests with phpunit 10, 11 & 12 here, unlike middlewares/cache which needed a few updates

@filisko
Copy link
Member

filisko commented May 11, 2025

@mikespub Hi, thanks for your input.

Let me share my thoughts though on why it's v10.

I don't see any problem supporting v10 only as it exactly matches our PHP constraint: >=8.1 and the last update was 1 week ago.

My idea was to use the phpunit version that does exactly that, and then when the time comes to drop 8.1 we upgrade phpunit to v11 (>=8.2) which would again match our version.

So, I don't see any benefit on doing what you did. I feel like it's anticipating what will go next (specially on the other package that requires changes to code).

Also, I would feel different about it if the change would be for production which would block (in our case) the phpunit version of the project using this lib.

I think it adds complexity to have 3 versions of the same lib. Imagine that the TestCase interface changes in each version ... That actually happens. Check the middleware that uses the jwt package of this org or the form-manager/form-manager package where I had to implement Symfony's validator interface twice that has the exact same code except for interface's return types (more strict in php 8.3)

@mikespub
Copy link
Contributor Author

mikespub commented May 11, 2025

Hi @filisko

I understand your point of view, but I thought we might as well be a bit pro-active in checking & supporting newer versions of dependencies. The last times we went through this, it was to support psr/http-message 2.x and PHP 8.4.

Now I wanted to check the new major releases of phpunit for all supported PHP versions, and as it turns out both middlewares/cache and its required mikespub/micheh-psr7-cache had minor compatibility issues.

And since I updated this package for psr/http-message 2.x too, I wanted to do the same thing here, and it turns out that the tests are fine for all versions. So we might as well allow them now, so that none of us need to come back in 6 months or a year again :-)

Note: I'm perfectly fine if we keep v10 for PHP 8.1, but we also support PHP 8.2, 8.3 and 8.4, and phpunit/phpunit has new major releases for each PHP version. Relaxing the dev dependency like I did allows us to test this package with the phpunit version most appropriate for each PHP version, while keeping compatibility for all versions.

@filisko
Copy link
Member

filisko commented May 11, 2025

I agree that perhaps in this case it's ok but as I mentioned earlier, TestCase could have 3 different implementations.

I upgraded all production required packages of the libs of this org. to their latest. Perhaps only 1 was missing and I have it spotted.

What where the compatibility issues? Why package's version was not changed?

We will have to come anyways to drop 8.1, 8.2, 8.3 .. :)

As I said, it's anticipating work, it doesn't improve anything (proved by the tests) so now we have 2 more versions
without anything really changing .. it's like adding dead code? Sorry for being that extreme

Will check the other package with the changes

@mikespub
Copy link
Contributor Author

Hi @filisko as there are no other changes here, feel free to close this PR if you prefer. The package version is based on tags, so I'll leave that decision to the maintainer :-)

@filisko
Copy link
Member

filisko commented May 12, 2025

@mikespub thank you though, I'm willing to see you more around in our other middlewares too! :)

I'm the main maintainer now,
Thanks!

@filisko filisko closed this May 12, 2025
@mikespub mikespub deleted the relax-dev branch January 29, 2026 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants