-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Unit testing: Allow Composer to auto-detect PHP version #73358
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
Conversation
2131388 to
8d4b14b
Compare
.github/workflows/unit-test.yml
Outdated
| # On PRs: Only test PHP 8.3, single-site, latest WP | ||
| - event: 'pull_request' | ||
| php: '7.2' | ||
| # TEMP: PHP 7.2 enabled for testing the Composer fix - do not merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this needs to be undone before merging; it's only there to trigger the PHP 7.2 tests in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excluded it again since PHP 7.2 tests pass in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually feel that this change should be reverted. But that's a discussion for another ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you say more about that, so that we spin a follow-up PR?
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| "platform": { | ||
| "php": "7.4" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of removing this entirely, let's change it to `"php": ">=7.2.24"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are version ranges supported in this context? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this should be specified under require.php, not config.platform.
Ref: https://getcomposer.org/doc/articles/composer-platform-dependencies.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Platform doesn't accept >=, only specific versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@priethor That works for me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 0fe9d04, with composer validate running successfully
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PHP 7.2 Unit Tests job successfully passes with the require.php setting.
5f9946f to
5d5922e
Compare
aduth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@desrosj I'm going ahead and merging this to get the tests fixed on trunk, since we were all on the same page about the blocker issue, and it's already addressed. Let me know if there is any other adjustment needed from your perspective, and I'll gladly follow up 🙏 |
The feedback was addressed as agreed.
Co-authored-by: priethor <priethor@git.wordpress.org> Co-authored-by: desrosj <desrosj@git.wordpress.org> Co-authored-by: aduth <aduth@git.wordpress.org> Co-authored-by: swisspidy <swisspidy@git.wordpress.org>
|
Cherry-picked to |
Changes can be found at https://github.com/WordPress/gutenberg/commits/wp/6.9/. * [Fix isNewLink value with entity binding (#73368)](WordPress/gutenberg#73368) * [Unit testing: Allow Composer to auto-detect PHP version (#73358)](WordPress/gutenberg#73358) * [Move the Edit Navigation button to the last item in the block toolbar to ensure that the styling is consistent and simple (#73436)](WordPress/gutenberg#73436) * [Block Support: Change block visibility support key (#73432)](WordPress/gutenberg#73432) * [Accordion: add box-sizing:border-box rule (#73507)](WordPress/gutenberg#73507) * [Accordion Block: Trigger panel opening from URL hash or anchor link (#73357)](WordPress/gutenberg#73357) * [iAPI: Backport of "Return a deep-clone object from `getServerState` and `getServerContext` functions" (#73514)](WordPress/gutenberg#73514) * [Reuse wp_script_attributes filter for adding data-wp-router-options attribute to script module (#72489) (#73512)](WordPress/gutenberg#73512) * [Accordion Item: Don't use grid layout (#73501)](WordPress/gutenberg#73501) * [Drag and drop: remove grab cursor for multi-selection (#73521)](WordPress/gutenberg#73521) * [Math block: fix accessibility (#73508)](WordPress/gutenberg#73508) * [iAPI: Fix using `getServerContext` in derived state getters (#73518) (#73531)](WordPress/gutenberg#73531) * [Drag: hide block tools popovers (#73539)](WordPress/gutenberg#73539) Developed in #10549. See https://make.wordpress.org/core/handbook/about/release-cycle/block-editor-release-process-for-major-releases/#package-updates-and-core-patches. Props priethor. See #64301. git-svn-id: https://develop.svn.wordpress.org/branches/6.9@61304 602fd350-edb4-49c9-b593-d223f7449a82
Changes can be found at https://github.com/WordPress/gutenberg/commits/wp/6.9/. * [Fix isNewLink value with entity binding (#73368)](WordPress/gutenberg#73368) * [Unit testing: Allow Composer to auto-detect PHP version (#73358)](WordPress/gutenberg#73358) * [Move the Edit Navigation button to the last item in the block toolbar to ensure that the styling is consistent and simple (#73436)](WordPress/gutenberg#73436) * [Block Support: Change block visibility support key (#73432)](WordPress/gutenberg#73432) * [Accordion: add box-sizing:border-box rule (#73507)](WordPress/gutenberg#73507) * [Accordion Block: Trigger panel opening from URL hash or anchor link (#73357)](WordPress/gutenberg#73357) * [iAPI: Backport of "Return a deep-clone object from `getServerState` and `getServerContext` functions" (#73514)](WordPress/gutenberg#73514) * [Reuse wp_script_attributes filter for adding data-wp-router-options attribute to script module (#72489) (#73512)](WordPress/gutenberg#73512) * [Accordion Item: Don't use grid layout (#73501)](WordPress/gutenberg#73501) * [Drag and drop: remove grab cursor for multi-selection (#73521)](WordPress/gutenberg#73521) * [Math block: fix accessibility (#73508)](WordPress/gutenberg#73508) * [iAPI: Fix using `getServerContext` in derived state getters (#73518) (#73531)](WordPress/gutenberg#73531) * [Drag: hide block tools popovers (#73539)](WordPress/gutenberg#73539) Developed in WordPress/wordpress-develop#10549. See https://make.wordpress.org/core/handbook/about/release-cycle/block-editor-release-process-for-major-releases/#package-updates-and-core-patches. Props priethor. See #64301. Built from https://develop.svn.wordpress.org/branches/6.9@61304 git-svn-id: http://core.svn.wordpress.org/branches/6.9@60616 1a063a9b-81f0-0310-95a4-ce76da25c4cd
What?
Fixes composer settings and unit test workflow so that Composer can auto-detect the right PHP executing the Unit Test job matrix.
Why?
It caused dependency resolution failures and broke unit tests for PHP 7.2 versions.
How?
platform.phpsettingrequire.phpsetting to >=7.2.24, the current minimum PHP version supported by WordPress