Repository navigation
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
ce49f65 to
778c4ee
Compare
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
lancewillett
left a comment
There was a problem hiding this comment.
The separate configurations fit the ticket’s scope. Please address three integration gaps before landing:
- In
reusable-phpunit-tests-v3.yml, remove or version-gate--verboseand-v. PHPUnit 10 rejects these options; I reproducedUnknown option "--verbose"with PHPUnit 10.5.60 in Docker. - Pass
-c "${PHPUNIT_CONFIG}"to the xDebug test step too. It currently bypasses the new configuration selection. - Restore JUnit logging in
tests/phpunit/multisite10.xml, matching the existing multisite configuration:This preserves the output consumed by CI reporting and timing tools.<logging> <junit outputFile="build/logs/junit.xml" /> </logging>
Both XML files passed PHPUnit 10.1 schema validation, and isolated PHPUnit 10 probes confirmed that their settings fail on deprecations, notices, and warnings. Actionlint passed. CI is green, but the inspected PHP 8.5 job uses PHPUnit 9.6, so it does not exercise the new configuration.
These are focused configuration/invocation fixes; completing the broader PHPUnit 10 migration can remain separate.
AI review · gpt-6
|
Thanks for the thorough review and catching these integration gaps @lancewillett! I have addressed all three items in commit 479ae87f56:
|
|
@johnbillion, could you give this an independent review before we land it? The author addressed the PHPUnit 10 invocation and configuration issues, and my code approval stands. I’m holding this larger integration change for a second review of the approach. AI review · |
|
@robelsust this now conflicts with trunk in AI review · |
… later - Add phpunit10.xml.dist and tests/phpunit/multisite10.xml compliant with PHPUnit 10.1+ schema. - Update reusable-phpunit-tests-v3.yml with automated detection to select the PHPUnit 10+ configuration when running on PHPUnit 10+. - Fix hardcoded -c phpunit.xml.dist in reusable-phpunit-tests-v1.yml and v2.yml external-http step to use PHPUNIT_CONFIG. - Update workflow path triggers in phpunit-tests.yml, full matrix, test-coverage, and exclude from asset workflows. - Add test10 script in composer.json and test:php10 in package.json. - Add /phpunit10.xml to .gitignore and update configuration documentation in tests/phpunit/README.md. See #66210, #62004.
…ow integration
- Remove --verbose and -v options from reusable-phpunit-tests-v3.yml since PHPUnit 10 rejects them.
- Pass -c "${PHPUNIT_CONFIG}" to the Xdebug test step in reusable-phpunit-tests-v3.yml.
- Restore JUnit logging in tests/phpunit/multisite10.xml matching tests/phpunit/multisite.xml.
See #66210, #62004.
479ae87 to
b8dbe9d
Compare
|
@lancewillett Rebased cleanly onto latest trunk and resolved the conflict in .github/workflows/reusable-phpunit-tests-v3.yml with the r65292 PCOV changes. Ready for your re-check! |
lancewillett
left a comment
There was a problem hiding this comment.
Thanks for the rebase. Re-checked at ed34d10: the changes match what I approved at 479ae87. In reusable-phpunit-tests-v3.yml, the "Select PHPUnit configuration" step now follows "Install WordPress", since r65292 removed the PCOV check that used to come before it. CI is green.
One nit: the new step is missing the blank line before "Run external HTTP tests" that separates the other steps.
This still needs an independent review from @johnbillion before it lands.
AI review · claude-opus-5-5
Introduces separate PHPUnit 10.1+ configuration files (
phpunit10.xml.distandtests/phpunit/multisite10.xml) alongside the existing PHPUnit 9 configuration files (phpunit.xml.distandtests/phpunit/multisite.xml), fulfilling Problem 1 of Trac #62004 as scoped in Trac #66210.Background & Context
<phpunit>element (convertErrorsToExceptions,convertWarningsToExceptions,convertNoticesToExceptions,convertDeprecationsToExceptions) were removed.failOnDeprecation="true",failOnNotice="true",failOnWarning="true",failOnRisky="true", and the correspondingdisplayDetailsOnTestsThatTrigger*="true"attributes.<filter><whitelist>element was replaced by<source><include>and<source><exclude>.<logging><log type="junit" target="..."/></logging>element was replaced by<logging><junit outputFile="..."/></logging>.Changes
Added
phpunit10.xml.dist:https://schema.phpunit.de/10.1/phpunit.xsd).failOnDeprecation="true",failOnNotice="true",failOnWarning="true",failOnRisky="true", and details display.<source><include>and<source><exclude>.<logging><junit outputFile="tests/phpunit/build/logs/junit.xml" />.Added
tests/phpunit/multisite10.xml:tests/phpunit/multisite.xml.WP_TESTS_MULTISITE="1"andWP_RUN_CORE_TESTS="1".ajax,ms-files,ms-excluded,external-http,oembed-headers,html-api-web-platform-tests).<source>paths relative totests/phpunit/.Workflow & CI Automation:
reusable-phpunit-tests-v3.yml: Added aSelect PHPUnit configurationstep before running tests that detects the installed PHPUnit version inside the container. If PHPUnit version >= 10, it automatically switchesPHPUNIT_CONFIGtophpunit10.xml.dist(ortests/phpunit/multisite10.xmlwhen testing multisite).reusable-phpunit-tests-v1.yml&reusable-phpunit-tests-v2.yml: Fixed theexternal-httptest step which previously hard-coded-c phpunit.xml.dist, now correctly using-c "${PHPUNIT_CONFIG}"..github/workflows/phpunit-tests.yml,phpunit-tests-full-matrix.yml, andtest-coverage.ymlto trigger onphpunit10.xml.distandmultisite10.xml, while excluding!phpunit10.xml.distfrom asset/e2e/performance workflows.Developer Tooling & Documentation:
composer.json: Added"test10": [ "Composer\\Config::disableProcessTimeout", "@php ./vendor/phpunit/phpunit/phpunit -c phpunit10.xml.dist" ].package.json: Added"test:php10": "node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpunit -c phpunit10.xml.dist"..gitignore: Added/phpunit10.xmland/phpunit-10.xmlto ignore local developer overrides.tests/phpunit/README.md: Added aConfiguration Filessection documenting which configuration file to use for PHPUnit 9 vs PHPUnit 10+.Verification
phpunit10.xml.distandtests/phpunit/multisite10.xmlwere strictly validated against official PHPUnit 10.1, 10.5, and 11.0 XSD schemas viaxmllint --noout --schemawith 0 errors.composer validateexecuted with 0 errors.package.jsonsyntax validated.Trac ticket: https://core.trac.wordpress.org/ticket/66210
Use of AI Tools
AI assistance: Yes
Tool(s): Antigravity
Model(s): Gemini
Used for: Validating schema conformity against PHPUnit 10.1/10.5/11.0 XSD specifications, updating CI workflow configuration detection and reusable test runner workflows, and authoring developer documentation. All changes were reviewed, verified, and validated locally.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.