Repository navigation
List skipped and incomplete tests with their reasons in the test output - #361
softglazee wants to merge 2 commits into
Conversation
PHPUnit lists failed and risky tests at the end of a run, but it shows skipped and incomplete tests only as a count unless it runs with --verbose. junit.xml marks those tests without a reason, so a host could not see anywhere why a test did not run. Run PHPUnit with --verbose, as WordPress core's own PHPUnit workflow does, and describe the lists in the README. The report sent to WordPress.org does not change. Fixes WordPress#47.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. 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. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
✅ Container Tests: successResults for 94c9ac9 from workflow run #11. Logs, PHPUnit Test Runner: ✅ passed
Test results
PHPUnit Test Runner: ✅ passed
Test results
|
ekamran
left a comment
There was a problem hiding this comment.
Thanks for this one.
I checked the claims against PHPUnit 9.6.38: DefaultResultPrinter::printResult() prints the incomplete and skipped lists only when verbose, the JUnit logger writes an empty <skipped/>, and it leaves risky tests out when useless tests are not reported. Core trunk still requires yoast/phpunit-polyfills ^1.1.0, so PHPUnit stays on 9.x, where --verbose is a valid option, and core's own workflow passes it. In the container test artifacts for a3aa891, both jobs ran with --verbose and printed the list, and report.php reads only junit.xml, so the upload does not change.
One suggestion on the README below, for hosts with a custom WPT_PHPUNIT_CMD. Happy to approve after that.
A side effect, not a blocker: when tests fail, docker/summarize.php adds the last 40 lines of test.log, which may now include skipped-list entries, since PHPUnit prints that list after the failures. The failed tests table it builds from junit.xml still lists the failures (the first 50) with their messages.
|
@ekamran Can you help me to mention here other reviewer too for reveiwing it too and then merge? |
PHPUnit lists the failed and risky tests at the end of a run, but it shows skipped and incomplete tests only as a count (
Skipped: 62) unless it runs with--verbose.junit.xmldoes not have the reasons either: PHPUnit 9.6 writes a skipped or incomplete test as an empty<skipped/>element. So a host could not see anywhere which tests were skipped or why. This PR runs PHPUnit with--verbose, the same flag WordPress core's own PHPUnit workflow uses, and describes the lists in the README.Fixes #47.
Changes
test.php: add--verboseto the default PHPUnit command. A customWPT_PHPUNIT_CMDis not changed.R(risky) symbol, and a short paragraph about the lists at the end of the run, with an example from a real run.The report sent to WordPress.org does not change. I checked the receiving plugin (
phpunit-test-reporter/src/class-restapi.phpin the WordPress/wordpress.org repository). When a result has failures, it treats every test case undertestsuitesas a failure in the "new failures" email, so skipped tests should not go into the payload.Risky tests: PHPUnit already lists them without
--verbose. They are not injunit.xml, because--dont-report-useless-testsalso turns off risky reporting in the PHPUnit 9.6 JUnit logger (JUnit::addRiskyTest()returns early whenreportRiskyTestsis false).Testing Instructions
Environment
docker/DockerfilewithPHP_VERSION=8.4, run as the non-rootwptestsuserSteps to Test
cp .env.default .env, set theWPT_DB_*values, setWPT_TEST_DIRto the same value asWPT_PREPARE_DIR, thensource .env.php prepare.phpphp test.phpphp report.phpwithoutWPT_REPORT_API_KEY, so it prints the results instead of uploading them.php cleanup.phpmasteras well, against the same prepared checkout, before step 5.Expected Results
test.phplogs ends with--dont-report-useless-tests --verbose.There were 62 skipped tests:.junit.xmlis the same as onmaster: the same totals and the same status for every test case.report.phpprints (theresultsfield of the upload) are the same as onmaster, apart fromtime.Results
Before (
masterat 1f276b2), the end of thetest.phpoutput:After (this branch), the same run adds the list. Three of the 62 entries:
The XSL entry is how I found that the image built from
docker/Dockerfiledoes not have thexslextension, so 6 sitemap tests are skipped in the container runs. That is a separate change.junit.xmlfrom both runs:tests=30867 assertions=4573915 failures=0 errors=0 warnings=86 skipped=62, and the sorted list of test cases with their status is identical (same SHA-1 over 30867 lines).report.phpresults from both runs, apart fromtime:{"tests":"30867","failures":"0","errors":"0","testsuites":[]}.php -l test.phpandvendor/bin/phpcs test.phpwith the repository ruleset are clean.Not tested: the multisite flavor and an SSH setup. In both cases the flag is part of the same command string.
My connection to GitHub is slow, so
prepare.phpcloned wordpress-develop trunk from a local mirror (a gitinsteadOfrule), and the Gutenberg build was downloaded once beforehand. No runner file was changed for that.Notes
--verbose(it has--display-skippedand--display-incompleteinstead). Core trunk requiresyoast/phpunit-polyfills ^1.1.0, which keeps PHPUnit on 9.x, and core's own workflow passes--verbose, so this line would change together with core when it moves to PHPUnit 10.test.php, so whichever of the two is merged second needs a one-line rebase. This branch merges cleanly with the other open PRs (Add host-tools/testrunner.sh to run the full test cycle in one step #346, Add host-tools/setup.sh to create .env with prompts #348, Document the host tools #349, Ensure PHPCS cache directory is tracked and fix docblock typos #357, Check the minimum PHP version from the WordPress checkout, before the build #360).Use of AI
AI assistance: Yes
Tool(s): Claude Code
Used for: Investigation, implementation, the end-to-end test runs and PR wording. I reviewed the reasoning and the test results, and I take responsibility for the contribution.