Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to phpunit 11 #1252

Closed
bobstrecansky opened this issue Mar 8, 2024 · 7 comments
Closed

Upgrade to phpunit 11 #1252

bobstrecansky opened this issue Mar 8, 2024 · 7 comments
Labels
on hold waiting on a depency from something else

Comments

@bobstrecansky
Copy link
Collaborator

Before opening a feature request against this repo, consider whether the feature should/could be implemented in the other OpenTelemetry client libraries. If so, please open an issue in opentelemetry-specification first.

Is your feature request related to a problem?
We are currently using phpunit 9.6 in ourcodebase. We should upgrade to PHP 11.0. We will need to do this after we upgrade to PHP 8.2, as that is a required dependency

Describe the solution you'd like
What do you want to happen instead? What is the expected behavior?
PHPUnit tests running on PHPUnit 11.0

Describe alternatives you've considered
Which alternative solutions or features have you considered?
None

Additional context
Add any other context about the feature request here.

@bobstrecansky bobstrecansky added the on hold waiting on a depency from something else label Mar 8, 2024
@brettmc
Copy link
Collaborator

brettmc commented Mar 20, 2024

Progress update: dropping PHP 8.0 support allowed an upgrade to phpunit 10 in #1258

@dkarlovi
Copy link
Contributor

dkarlovi commented Apr 5, 2024

Going from PHPUnit 10 to 11 is simple assuming you don't have any deprecations, for example you need to actually make all data providers static, in 10 they trigger a deprecation.

@brettmc
Copy link
Collaborator

brettmc commented May 1, 2024

Upgrading tests to phpunit 11 probably is simple. Upgrading our dependencies to phpunit 11 not so much:

  • phpunit 11 requires phpunit/php-code-coverage ^11
  • phpunit/php-code-coverage requires nikic/php-parser ^5
  • vimeo/psalm latest required nikic/php-parser ^4.16

So, until psalm starts supporting nikic/php-parser ^5, we're blocked. Both psalm and phpunit are important parts of out CI pipeline, and we do need and want both.

Once that puzzle is solved, our tests do pass under phpunit11, but there are deprecation warnings for phpunit 12!

edit: we could also try installing phpunit as a phar, which would remove some dependency issues, I think.

@dkarlovi
Copy link
Contributor

dkarlovi commented May 1, 2024

Installing PHPUnit as PHAR makes running static analysis on your tests more difficult since your code (in tests) is using PHPUnit directly.

What you can do is install Psalm separately. We use a Docker image with Psalm baked in which des this for us: https://github.com/sigwinhq/infra

A more conventional way to do this (which the above Docker image uses internally) is to use https://github.com/bamarni/composer-bin-plugin

It allows you to install your tools using a separate composer.json which then doesn't need to be aligned with your project one since you're not using Psalm as a dependency, you're just using Psalm the tool.

@brettmc
Copy link
Collaborator

brettmc commented May 2, 2024

@dkarlovi using composer-bin-plugin was fairly straightforward. Thanks for the advice on not doing anything clever with phpunit, I've left it as-is.
I can now upgrade to phpunit 11, but I'll leave that as a separate PR since it will involve fixing lots of deprecations.

@bobstrecansky
Copy link
Collaborator Author

This is being worked on in #1308

@brettmc
Copy link
Collaborator

brettmc commented May 16, 2024

Closed by #1308

@brettmc brettmc closed this as completed May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold waiting on a depency from something else
Projects
None yet
Development

No branches or pull requests

3 participants