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

Run tests in different versions of PHP #544

Merged
merged 44 commits into from
Jan 29, 2024

Conversation

mukeshpanchal27
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 commented Sep 23, 2022

Summary

Fixes #399

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@mukeshpanchal27 mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release Needs Review labels Sep 23, 2022
@mukeshpanchal27 mukeshpanchal27 self-assigned this Sep 23, 2022
@swissspidy
Copy link
Member

I thought I'd pick this one up, but a couple of SVG upload tests (test_get_dominant_color_none_images) are failing on various PHP versions 🤷

@mukeshpanchal27
Copy link
Member Author

Thanks @swissspidy for picking. I also don't understood why it going fails 🤔

@swissspidy
Copy link
Member

So the failures makes sense, because WP doesn't handle/support SVG uploads, so wp_check_filetype fails. And unfiltered uploads are not enabled either. The weird part is that the test is passing on PHP 7.4...

We don't really need to test SVG upload here anyway, so we can just remove it.

@swissspidy
Copy link
Member

Tests on PHP < 7.3 are failing because the PHPUnit version (v9.6) is too new for those PHP versions, causing syntax errors.

We probably don't need something as complex as WordPress/plugin-check#321, but definitely something in that direction.

@mukeshpanchal27
Copy link
Member Author

All the unit test pass ✅ now. Looking into integration tests setup.

@swissspidy
Copy link
Member

There is no error output for failing runs, see e.g. https://github.com/WordPress/performance/actions/runs/7650368243/job/20846313421?pr=544

Do we need to log command.stderr?

@swissspidy swissspidy changed the title Run PHP unit tests in different versions of PHP Run tests in different versions of PHP Jan 25, 2024
@mukeshpanchal27
Copy link
Member Author

@swissspidy Added log in script. In local i got below error for PHP 7.2.

ℹ Starting 'vendor/bin/phpunit -c phpunit.xml --verbose --testdox' on the tests-cli container. 
This version of PHPUnit requires PHP >= 7.3.
You are using PHP 7.2.34 (/usr/local/bin/php).

@swissspidy
Copy link
Member

@swissspidy Added log in script. In local i got below error for PHP 7.2.

ℹ Starting 'vendor/bin/phpunit -c phpunit.xml --verbose --testdox' on the tests-cli container. 
This version of PHPUnit requires PHP >= 7.3.
You are using PHP 7.2.34 (/usr/local/bin/php).

Aah 💡

Now it makes sense. We need to downgrade PHPUnit in this case using composer update, just like we do for unit tests.

Maybe 7d560ae works 🤞

@swissspidy
Copy link
Member

Or maybe not 🤷

Copy link
Member

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay 🤩🥳

@swissspidy swissspidy merged commit fc9ddfb into trunk Jan 29, 2024
26 checks passed
@swissspidy swissspidy deleted the fix/399-unit-tests-in-php-versions branch January 29, 2024 21:05
Comment on lines +23 to +34
"phpstan": [
"composer --working-dir=build-cs update --no-interaction",
"build-cs/vendor/bin/phpstan analyse --memory-limit=2048M -c phpstan.neon.dist"
],
"format": [
"composer --working-dir=build-cs update --no-interaction",
"build-cs/vendor/bin/phpcbf --standard=phpcs.xml.dist --report-summary --report-source"
],
"lint": [
"composer --working-dir=build-cs update --no-interaction",
"build-cs/vendor/bin/phpcs --standard=phpcs.xml.dist"
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that calling composer update here causes problems: #961

Proposed fix: #962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run PHP unit tests in different versions of PHP
5 participants