From 30a282b22ac891518f5094ba90b8d4731e643758 Mon Sep 17 00:00:00 2001 From: Lance Willett Date: Thu, 20 Aug 2026 16:52:41 -0700 Subject: [PATCH 1/3] Build/Test Tools: Flag slow PHPUnit tests with annotations. Parse the JUnit report from the canonical PHP 8.5 report job and emit GitHub Actions warning annotations plus a run-summary table for tests over a threshold, on pull requests and pushes to trunk. Advisory only: the step runs even on failed test runs and never fails the build itself. This names the slow tests, complementing the aggregate CodeVitals trend that stores them without naming any one test. See #65887. --- .../workflows/reusable-phpunit-tests-v3.yml | 16 ++ .../phpunit/prepare-slow-test-annotations.php | 268 ++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 tests/phpunit/prepare-slow-test-annotations.php diff --git a/.github/workflows/reusable-phpunit-tests-v3.yml b/.github/workflows/reusable-phpunit-tests-v3.yml index 8a225134a2c38..95bbbe76ca0c5 100644 --- a/.github/workflows/reusable-phpunit-tests-v3.yml +++ b/.github/workflows/reusable-phpunit-tests-v3.yml @@ -307,6 +307,22 @@ jobs: fi echo 'Published six PHPUnit timing metrics to CodeVitals.' + - name: Flag slow PHPUnit tests + # Runs even when the test step failed (always()), so the signal still + # surfaces on red runs, and never fails the job itself (continue-on-error), + # because this is advisory only. + if: >- + always() && inputs.report && inputs.php == '8.5' && + ( github.event_name == 'pull_request' || + ( github.event_name == 'push' && github.ref == 'refs/heads/trunk' ) ) + continue-on-error: true + run: | + if [ -f tests/phpunit/build/logs/junit.xml ]; then + php tests/phpunit/prepare-slow-test-annotations.php tests/phpunit/build/logs/junit.xml + else + echo 'PHPUnit JUnit report not found; skipping slow-test annotations.' + fi + - name: Run AJAX tests if: ${{ ! inputs.phpunit-test-groups && ! inputs.coverage-report }} continue-on-error: ${{ inputs.allow-errors }} diff --git a/tests/phpunit/prepare-slow-test-annotations.php b/tests/phpunit/prepare-slow-test-annotations.php new file mode 100644 index 0000000000000..8ed28786b5f24 --- /dev/null +++ b/tests/phpunit/prepare-slow-test-annotations.php @@ -0,0 +1,268 @@ +#!/usr/bin/env php + \ + * [threshold-seconds] [max-annotations] + * + * @package WordPress + * @subpackage UnitTests + */ + +/** + * Escapes a GitHub Actions workflow command message. + * + * @param string $value Message to escape. + * @return string Escaped message. + */ +function wp_phpunit_escape_command_message( $value ) { + return str_replace( + array( '%', "\r", "\n" ), + array( '%25', '%0D', '%0A' ), + $value + ); +} + +/** + * Escapes a GitHub Actions workflow command property. + * + * @param string $value Property value to escape. + * @return string Escaped property value. + */ +function wp_phpunit_escape_command_property( $value ) { + return str_replace( + array( ',', ':' ), + array( '%2C', '%3A' ), + wp_phpunit_escape_command_message( $value ) + ); +} + +/** + * Escapes text for a Markdown table cell. + * + * @param string $value Cell value to escape. + * @return string Escaped cell value. + */ +function wp_phpunit_escape_markdown_cell( $value ) { + return str_replace( + array( '|', "\r", "\n" ), + array( '\\|', ' ', ' ' ), + $value + ); +} + +/** + * Converts a container-absolute test path to a repository-relative one. + * + * PHPUnit records absolute paths (the repository is mounted at /var/www in the + * Docker environment). GitHub annotations need repository-relative paths to + * resolve to a line, so a known workspace prefix is stripped when present. + * + * @param string $file Path recorded in the JUnit report. + * @return string Repository-relative path, or the input unchanged. + */ +function wp_phpunit_relative_path( $file ) { + if ( '' === $file ) { + return ''; + } + + $prefixes = array( '/var/www/' ); + $workspace = getenv( 'GITHUB_WORKSPACE' ); + + if ( is_string( $workspace ) && '' !== $workspace ) { + $prefixes[] = rtrim( $workspace, '/' ) . '/'; + } + + foreach ( $prefixes as $prefix ) { + if ( 0 === strncmp( $file, $prefix, strlen( $prefix ) ) ) { + return substr( $file, strlen( $prefix ) ); + } + } + + return $file; +} + +/** + * Appends a summary to GitHub Actions or writes it to standard output. + * + * @param string $summary Markdown summary. + * @return void + * @throws RuntimeException If the GitHub Actions summary cannot be written. + */ +function wp_phpunit_write_summary( $summary ) { + $summary_file = getenv( 'GITHUB_STEP_SUMMARY' ); + + if ( false === $summary_file || '' === $summary_file ) { + echo $summary; + return; + } + + if ( false === file_put_contents( $summary_file, $summary, FILE_APPEND ) ) { + throw new RuntimeException( 'The GitHub Actions step summary could not be written.' ); + } +} + +if ( $argc < 2 || $argc > 4 ) { + fwrite( + STDERR, + "Usage: php tests/phpunit/prepare-slow-test-annotations.php " + . "[threshold-seconds] [max-annotations]\n" + ); + exit( 1 ); +} + +try { + $file = $argv[1]; + $threshold_value = $argv[2] ?? '1.0'; + $max_annotations = $argv[3] ?? '20'; + + if ( ! is_numeric( $threshold_value ) || (float) $threshold_value < 0 ) { + throw new RuntimeException( 'The slow-test threshold must be a non-negative number.' ); + } + + if ( ! ctype_digit( $max_annotations ) || (int) $max_annotations < 1 ) { + throw new RuntimeException( 'The maximum annotation count must be a positive integer.' ); + } + + if ( ! is_readable( $file ) ) { + throw new RuntimeException( 'The JUnit report could not be read.' ); + } + + $threshold = (float) $threshold_value; + $max_annotations = (int) $max_annotations; + $reader = new XMLReader(); + $previous_libxml_state = libxml_use_internal_errors( true ); + $reader_is_open = false; + + libxml_clear_errors(); + + try { + if ( ! $reader->open( $file, null, LIBXML_NONET | LIBXML_COMPACT ) ) { + throw new RuntimeException( 'The JUnit report could not be opened.' ); + } + + $reader_is_open = true; + $slow_tests = array(); + + while ( $reader->read() ) { + if ( XMLReader::ELEMENT !== $reader->nodeType || 'testcase' !== $reader->name ) { + continue; + } + + $time = $reader->getAttribute( 'time' ); + + // A testcase without numeric timing (for example a skipped test) carries + // no slow-test signal, so it is ignored rather than treated as an error. + if ( ! is_numeric( $time ) ) { + continue; + } + + if ( (float) $time <= $threshold ) { + continue; + } + + $slow_tests[] = array( + 'name' => (string) $reader->getAttribute( 'name' ), + 'class' => (string) $reader->getAttribute( 'class' ), + 'file' => wp_phpunit_relative_path( (string) $reader->getAttribute( 'file' ) ), + 'line' => (string) $reader->getAttribute( 'line' ), + 'time' => (float) $time, + 'time_display' => $time, + ); + } + + $xml_errors = libxml_get_errors(); + } finally { + if ( $reader_is_open ) { + $reader->close(); + } + + libxml_clear_errors(); + libxml_use_internal_errors( $previous_libxml_state ); + } + + foreach ( $xml_errors as $xml_error ) { + if ( LIBXML_ERR_WARNING < $xml_error->level ) { + throw new RuntimeException( 'The JUnit report contains invalid XML.' ); + } + } + + usort( + $slow_tests, + static function ( $left, $right ) { + if ( $left['time'] === $right['time'] ) { + return strcmp( $left['class'] . '::' . $left['name'], $right['class'] . '::' . $right['name'] ); + } + + return $right['time'] <=> $left['time']; + } + ); + + $slow_tests = array_slice( $slow_tests, 0, $max_annotations ); + + if ( ! $slow_tests ) { + wp_phpunit_write_summary( "No PHPUnit tests exceeded {$threshold_value}s.\n" ); + exit( 0 ); + } + + // GitHub Actions renders at most 10 warning annotations per step, so the inline + // annotations are capped there while the summary table below can list more. + foreach ( array_slice( $slow_tests, 0, 10 ) as $test ) { + $properties = array(); + + if ( '' !== $test['file'] ) { + $properties[] = 'file=' . wp_phpunit_escape_command_property( $test['file'] ); + } + + if ( '' !== $test['line'] ) { + $properties[] = 'line=' . wp_phpunit_escape_command_property( $test['line'] ); + } + + $properties[] = 'title=' . wp_phpunit_escape_command_property( 'Slow PHPUnit test' ); + $message = sprintf( + '%s::%s took %ss', + $test['class'], + $test['name'], + $test['time_display'] + ); + + printf( + "::warning %s::%s\n", + implode( ',', $properties ), + wp_phpunit_escape_command_message( $message ) + ); + } + + $summary = "### Slowest PHPUnit tests (main suite, over {$threshold_value}s)\n\n"; + $summary .= "| Test | Time (s) | File:line |\n"; + $summary .= "| --- | ---: | --- |\n"; + + foreach ( $slow_tests as $test ) { + $location = $test['file']; + + if ( '' !== $test['line'] ) { + $location .= ( '' !== $location ? ':' : 'Line ' ) . $test['line']; + } + + if ( '' === $location ) { + $location = '—'; + } + + $summary .= sprintf( + "| %s::%s | %s | %s |\n", + wp_phpunit_escape_markdown_cell( $test['class'] ), + wp_phpunit_escape_markdown_cell( $test['name'] ), + $test['time_display'], + wp_phpunit_escape_markdown_cell( $location ) + ); + } + + wp_phpunit_write_summary( $summary ); +} catch ( Throwable $error ) { + fwrite( STDERR, $error->getMessage() . "\n" ); + exit( 1 ); +} From 4d8c86524752ff3eab8d5bf183da7dad0c3f4ccf Mon Sep 17 00:00:00 2001 From: Lance Willett Date: Thu, 27 Aug 2026 18:01:24 -0700 Subject: [PATCH 2/3] Build/Test Tools: Address slow-test reporting review feedback --- .../workflows/reusable-phpunit-tests-v3.yml | 15 ++++--- .../phpunit/prepare-slow-test-annotations.php | 40 ++++++++++++------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.github/workflows/reusable-phpunit-tests-v3.yml b/.github/workflows/reusable-phpunit-tests-v3.yml index 95bbbe76ca0c5..6aa6c9850744b 100644 --- a/.github/workflows/reusable-phpunit-tests-v3.yml +++ b/.github/workflows/reusable-phpunit-tests-v3.yml @@ -127,6 +127,7 @@ jobs: # - Install WordPress within the Docker container. # - Run the PHPUnit tests. # - Publish PHPUnit timing metrics to CodeVitals. + # - Flags slow PHPUnit tests with GitHub Actions annotations and a run summary. # - Upload the code coverage report to Codecov.io. # - Ensures version-controlled files are not modified or deleted. # - Checks out the WordPress Test reporter repository. @@ -308,13 +309,15 @@ jobs: echo 'Published six PHPUnit timing metrics to CodeVitals.' - name: Flag slow PHPUnit tests - # Runs even when the test step failed (always()), so the signal still - # surfaces on red runs, and never fails the job itself (continue-on-error), - # because this is advisory only. + # Runs when the test step succeeds or fails, but skips cancelled jobs. + # This step never fails the job itself (continue-on-error), because the + # slow-test signal is advisory only. if: >- - always() && inputs.report && inputs.php == '8.5' && - ( github.event_name == 'pull_request' || - ( github.event_name == 'push' && github.ref == 'refs/heads/trunk' ) ) + ${{ + ! cancelled() && inputs.report && inputs.php == '8.5' && + ( github.event_name == 'pull_request' || + ( github.event_name == 'push' && github.ref == 'refs/heads/trunk' ) ) + }} continue-on-error: true run: | if [ -f tests/phpunit/build/logs/junit.xml ]; then diff --git a/tests/phpunit/prepare-slow-test-annotations.php b/tests/phpunit/prepare-slow-test-annotations.php index 8ed28786b5f24..d3f947f055849 100644 --- a/tests/phpunit/prepare-slow-test-annotations.php +++ b/tests/phpunit/prepare-slow-test-annotations.php @@ -7,7 +7,7 @@ * Usage: * * php tests/phpunit/prepare-slow-test-annotations.php \ - * [threshold-seconds] [max-annotations] + * [threshold-seconds] [max-summary-tests] * * @package WordPress * @subpackage UnitTests @@ -109,23 +109,23 @@ function wp_phpunit_write_summary( $summary ) { if ( $argc < 2 || $argc > 4 ) { fwrite( STDERR, - "Usage: php tests/phpunit/prepare-slow-test-annotations.php " - . "[threshold-seconds] [max-annotations]\n" + 'Usage: php tests/phpunit/prepare-slow-test-annotations.php ' + . "[threshold-seconds] [max-summary-tests]\n" ); exit( 1 ); } try { - $file = $argv[1]; - $threshold_value = $argv[2] ?? '1.0'; - $max_annotations = $argv[3] ?? '20'; + $file = $argv[1]; + $threshold_value = $argv[2] ?? '1.0'; + $max_summary_tests_value = $argv[3] ?? '20'; if ( ! is_numeric( $threshold_value ) || (float) $threshold_value < 0 ) { throw new RuntimeException( 'The slow-test threshold must be a non-negative number.' ); } - if ( ! ctype_digit( $max_annotations ) || (int) $max_annotations < 1 ) { - throw new RuntimeException( 'The maximum annotation count must be a positive integer.' ); + if ( ! ctype_digit( $max_summary_tests_value ) || (int) $max_summary_tests_value < 1 ) { + throw new RuntimeException( 'The maximum summary test count must be a positive integer.' ); } if ( ! is_readable( $file ) ) { @@ -133,7 +133,7 @@ function wp_phpunit_write_summary( $summary ) { } $threshold = (float) $threshold_value; - $max_annotations = (int) $max_annotations; + $max_summary_tests = (int) $max_summary_tests_value; $reader = new XMLReader(); $previous_libxml_state = libxml_use_internal_errors( true ); $reader_is_open = false; @@ -202,13 +202,14 @@ static function ( $left, $right ) { } ); - $slow_tests = array_slice( $slow_tests, 0, $max_annotations ); - if ( ! $slow_tests ) { wp_phpunit_write_summary( "No PHPUnit tests exceeded {$threshold_value}s.\n" ); exit( 0 ); } + $total_slow_tests = count( $slow_tests ); + $summary_tests = array_slice( $slow_tests, 0, $max_summary_tests ); + // GitHub Actions renders at most 10 warning annotations per step, so the inline // annotations are capped there while the summary table below can list more. foreach ( array_slice( $slow_tests, 0, 10 ) as $test ) { @@ -216,10 +217,10 @@ static function ( $left, $right ) { if ( '' !== $test['file'] ) { $properties[] = 'file=' . wp_phpunit_escape_command_property( $test['file'] ); - } - if ( '' !== $test['line'] ) { - $properties[] = 'line=' . wp_phpunit_escape_command_property( $test['line'] ); + if ( '' !== $test['line'] ) { + $properties[] = 'line=' . wp_phpunit_escape_command_property( $test['line'] ); + } } $properties[] = 'title=' . wp_phpunit_escape_command_property( 'Slow PHPUnit test' ); @@ -238,10 +239,19 @@ static function ( $left, $right ) { } $summary = "### Slowest PHPUnit tests (main suite, over {$threshold_value}s)\n\n"; + + if ( $total_slow_tests > count( $summary_tests ) ) { + $summary .= sprintf( + "Showing the %d slowest of %d tests above the threshold.\n\n", + count( $summary_tests ), + $total_slow_tests + ); + } + $summary .= "| Test | Time (s) | File:line |\n"; $summary .= "| --- | ---: | --- |\n"; - foreach ( $slow_tests as $test ) { + foreach ( $summary_tests as $test ) { $location = $test['file']; if ( '' !== $test['line'] ) { From da3eb42025bcfaf2129b26a51b1c6a527b7ec31b Mon Sep 17 00:00:00 2001 From: Lance Willett Date: Mon, 31 Aug 2026 10:22:43 -0700 Subject: [PATCH 3/3] Build/Test Tools: Reuse the PHPUnit timing report traversal. --- .../workflows/reusable-phpunit-tests-v3.yml | 43 ++++---- .../class-wp-phpunit-timing-metrics.php | 89 ++++++++++++----- .../phpunit/prepare-slow-test-annotations.php | 79 ++++----------- tests/phpunit/prepare-timing-results.php | 38 ++++++- .../tests/includes/junitTimingMetrics.php | 99 ++++++++++++++++--- 5 files changed, 232 insertions(+), 116 deletions(-) diff --git a/.github/workflows/reusable-phpunit-tests-v3.yml b/.github/workflows/reusable-phpunit-tests-v3.yml index 6aa6c9850744b..6485f2aa3f9d8 100644 --- a/.github/workflows/reusable-phpunit-tests-v3.yml +++ b/.github/workflows/reusable-phpunit-tests-v3.yml @@ -126,8 +126,8 @@ jobs: # - Logs debug information about what's installed within the WordPress Docker containers. # - Install WordPress within the Docker container. # - Run the PHPUnit tests. - # - Publish PHPUnit timing metrics to CodeVitals. # - Flags slow PHPUnit tests with GitHub Actions annotations and a run summary. + # - Publish PHPUnit timing metrics to CodeVitals. # - Upload the code coverage report to Codecov.io. # - Ensures version-controlled files are not modified or deleted. # - Checks out the WordPress Test reporter repository. @@ -274,6 +274,28 @@ jobs: TEST_GROUPS: ${{ inputs.phpunit-test-groups }} MULTISITE_FLAG: ${{ inputs.multisite && 'multisite' || 'single' }} + - name: Flag slow PHPUnit tests + # Runs when the test step succeeds or fails, but skips cancelled jobs. + # This step never fails the job itself (continue-on-error), because the + # slow-test signal is advisory only. + if: >- + ${{ + ! cancelled() && inputs.report && inputs.php == '8.5' && + ( github.event_name == 'pull_request' || + ( github.event_name == 'push' && github.ref == 'refs/heads/trunk' ) ) + }} + continue-on-error: true + run: | + if [ -f tests/phpunit/build/logs/junit.xml ]; then + php tests/phpunit/prepare-slow-test-annotations.php \ + tests/phpunit/build/logs/junit.xml \ + 1.0 \ + 20 \ + "$RUNNER_TEMP/phpunit-timing-metrics.json" + else + echo 'PHPUnit JUnit report not found; skipping slow-test annotations.' + fi + - name: Publish PHPUnit timing metrics continue-on-error: true if: | @@ -296,6 +318,7 @@ jobs: trunk \ "$GITHUB_SHA" \ "$COMMITTED_AT" \ + "$RUNNER_TEMP/phpunit-timing-metrics.json" \ | curl --fail-with-body --silent --show-error \ --request POST \ --header 'Content-Type: application/json' \ @@ -308,24 +331,6 @@ jobs: fi echo 'Published six PHPUnit timing metrics to CodeVitals.' - - name: Flag slow PHPUnit tests - # Runs when the test step succeeds or fails, but skips cancelled jobs. - # This step never fails the job itself (continue-on-error), because the - # slow-test signal is advisory only. - if: >- - ${{ - ! cancelled() && inputs.report && inputs.php == '8.5' && - ( github.event_name == 'pull_request' || - ( github.event_name == 'push' && github.ref == 'refs/heads/trunk' ) ) - }} - continue-on-error: true - run: | - if [ -f tests/phpunit/build/logs/junit.xml ]; then - php tests/phpunit/prepare-slow-test-annotations.php tests/phpunit/build/logs/junit.xml - else - echo 'PHPUnit JUnit report not found; skipping slow-test annotations.' - fi - - name: Run AJAX tests if: ${{ ! inputs.phpunit-test-groups && ! inputs.coverage-report }} continue-on-error: ${{ inputs.allow-errors }} diff --git a/tests/phpunit/includes/class-wp-phpunit-timing-metrics.php b/tests/phpunit/includes/class-wp-phpunit-timing-metrics.php index 1276154a32854..3d7b6e61d693f 100644 --- a/tests/phpunit/includes/class-wp-phpunit-timing-metrics.php +++ b/tests/phpunit/includes/class-wp-phpunit-timing-metrics.php @@ -8,50 +8,93 @@ final class WP_PHPUnit_Timing_Metrics { /** * Extracts timing metrics from a JUnit XML file. * - * @param string $file Path to the JUnit XML file. + * @param string $file Path to the JUnit XML file. + * @param callable|null $testcase_callback Optional callback invoked for each timed testcase. * @return array Timing metrics keyed for CodeVitals. - * @throws RuntimeException If the file cannot be read or contains invalid timing data. + * @throws RuntimeException If the file cannot be read or contains invalid XML. */ - public static function from_file( $file ) { + public static function from_file( $file, $testcase_callback = null ) { if ( ! is_readable( $file ) ) { throw new RuntimeException( 'The JUnit timing report could not be read.' ); } - $reader = new XMLReader(); - if ( ! $reader->open( $file, null, LIBXML_NONET | LIBXML_COMPACT ) ) { - throw new RuntimeException( 'The JUnit timing report could not be opened.' ); + if ( null !== $testcase_callback && ! is_callable( $testcase_callback ) ) { + throw new InvalidArgumentException( 'The testcase callback must be callable.' ); } - $suite_time = null; - $test_times = array(); + $reader = new XMLReader(); + $previous_libxml_state = libxml_use_internal_errors( true ); + $reader_is_open = false; + $suite_time = null; + $test_times = array(); + $xml_errors = array(); - while ( $reader->read() ) { - if ( XMLReader::ELEMENT !== $reader->nodeType ) { - continue; + libxml_clear_errors(); + + try { + if ( ! $reader->open( $file, null, LIBXML_NONET | LIBXML_COMPACT ) ) { + throw new RuntimeException( 'The JUnit timing report could not be opened.' ); } - if ( null === $suite_time && 'testsuite' === $reader->name ) { + $reader_is_open = true; + + while ( $reader->read() ) { + if ( XMLReader::ELEMENT !== $reader->nodeType ) { + continue; + } + + if ( null === $suite_time && 'testsuite' === $reader->name ) { + $time = $reader->getAttribute( 'time' ); + if ( is_numeric( $time ) ) { + $suite_time = (float) $time; + } + continue; + } + + if ( 'testcase' !== $reader->name ) { + continue; + } + $time = $reader->getAttribute( 'time' ); - if ( is_numeric( $time ) ) { - $suite_time = (float) $time; + + // A testcase without numeric timing, such as a skipped test, carries + // no timing signal and is excluded from the aggregate metrics. + if ( ! is_numeric( $time ) ) { + continue; } - continue; - } - if ( 'testcase' !== $reader->name ) { - continue; + $test_time = (float) $time; + $test_times[] = $test_time; + + if ( null !== $testcase_callback ) { + $testcase_callback( + array( + 'name' => (string) $reader->getAttribute( 'name' ), + 'class' => (string) $reader->getAttribute( 'class' ), + 'file' => (string) $reader->getAttribute( 'file' ), + 'line' => (string) $reader->getAttribute( 'line' ), + 'time' => $test_time, + 'time_display' => (string) $time, + ) + ); + } } - $time = $reader->getAttribute( 'time' ); - if ( ! is_numeric( $time ) ) { + $xml_errors = libxml_get_errors(); + } finally { + if ( $reader_is_open ) { $reader->close(); - throw new RuntimeException( 'A JUnit testcase is missing numeric timing data.' ); } - $test_times[] = (float) $time; + libxml_clear_errors(); + libxml_use_internal_errors( $previous_libxml_state ); } - $reader->close(); + foreach ( $xml_errors as $xml_error ) { + if ( LIBXML_ERR_WARNING < $xml_error->level ) { + throw new RuntimeException( 'The JUnit timing report contains invalid XML.' ); + } + } if ( ! $test_times ) { throw new RuntimeException( 'The JUnit timing report contains no testcases.' ); diff --git a/tests/phpunit/prepare-slow-test-annotations.php b/tests/phpunit/prepare-slow-test-annotations.php index d3f947f055849..27ae6af2cd6bf 100644 --- a/tests/phpunit/prepare-slow-test-annotations.php +++ b/tests/phpunit/prepare-slow-test-annotations.php @@ -7,12 +7,14 @@ * Usage: * * php tests/phpunit/prepare-slow-test-annotations.php \ - * [threshold-seconds] [max-summary-tests] + * [threshold-seconds] [max-summary-tests] [timing-metrics-file] * * @package WordPress * @subpackage UnitTests */ +require_once __DIR__ . '/includes/class-wp-phpunit-timing-metrics.php'; + /** * Escapes a GitHub Actions workflow command message. * @@ -106,11 +108,11 @@ function wp_phpunit_write_summary( $summary ) { } } -if ( $argc < 2 || $argc > 4 ) { +if ( $argc < 2 || $argc > 5 ) { fwrite( STDERR, 'Usage: php tests/phpunit/prepare-slow-test-annotations.php ' - . "[threshold-seconds] [max-summary-tests]\n" + . "[threshold-seconds] [max-summary-tests] [timing-metrics-file]\n" ); exit( 1 ); } @@ -119,6 +121,7 @@ function wp_phpunit_write_summary( $summary ) { $file = $argv[1]; $threshold_value = $argv[2] ?? '1.0'; $max_summary_tests_value = $argv[3] ?? '20'; + $timing_metrics_file = $argv[4] ?? null; if ( ! is_numeric( $threshold_value ) || (float) $threshold_value < 0 ) { throw new RuntimeException( 'The slow-test threshold must be a non-negative number.' ); @@ -128,66 +131,26 @@ function wp_phpunit_write_summary( $summary ) { throw new RuntimeException( 'The maximum summary test count must be a positive integer.' ); } - if ( ! is_readable( $file ) ) { - throw new RuntimeException( 'The JUnit report could not be read.' ); - } - - $threshold = (float) $threshold_value; - $max_summary_tests = (int) $max_summary_tests_value; - $reader = new XMLReader(); - $previous_libxml_state = libxml_use_internal_errors( true ); - $reader_is_open = false; - - libxml_clear_errors(); - - try { - if ( ! $reader->open( $file, null, LIBXML_NONET | LIBXML_COMPACT ) ) { - throw new RuntimeException( 'The JUnit report could not be opened.' ); - } - - $reader_is_open = true; - $slow_tests = array(); - - while ( $reader->read() ) { - if ( XMLReader::ELEMENT !== $reader->nodeType || 'testcase' !== $reader->name ) { - continue; + $threshold = (float) $threshold_value; + $max_summary_tests = (int) $max_summary_tests_value; + $slow_tests = array(); + $timing_metrics = WP_PHPUnit_Timing_Metrics::from_file( + $file, + static function ( $testcase ) use ( &$slow_tests, $threshold ) { + if ( $testcase['time'] <= $threshold ) { + return; } - $time = $reader->getAttribute( 'time' ); - - // A testcase without numeric timing (for example a skipped test) carries - // no slow-test signal, so it is ignored rather than treated as an error. - if ( ! is_numeric( $time ) ) { - continue; - } - - if ( (float) $time <= $threshold ) { - continue; - } - - $slow_tests[] = array( - 'name' => (string) $reader->getAttribute( 'name' ), - 'class' => (string) $reader->getAttribute( 'class' ), - 'file' => wp_phpunit_relative_path( (string) $reader->getAttribute( 'file' ) ), - 'line' => (string) $reader->getAttribute( 'line' ), - 'time' => (float) $time, - 'time_display' => $time, - ); - } - - $xml_errors = libxml_get_errors(); - } finally { - if ( $reader_is_open ) { - $reader->close(); + $testcase['file'] = wp_phpunit_relative_path( $testcase['file'] ); + $slow_tests[] = $testcase; } + ); - libxml_clear_errors(); - libxml_use_internal_errors( $previous_libxml_state ); - } + if ( null !== $timing_metrics_file ) { + $timing_metrics_json = json_encode( $timing_metrics, JSON_THROW_ON_ERROR ); - foreach ( $xml_errors as $xml_error ) { - if ( LIBXML_ERR_WARNING < $xml_error->level ) { - throw new RuntimeException( 'The JUnit report contains invalid XML.' ); + if ( false === file_put_contents( $timing_metrics_file, $timing_metrics_json . "\n" ) ) { + throw new RuntimeException( 'The PHPUnit timing metrics file could not be written.' ); } } diff --git a/tests/phpunit/prepare-timing-results.php b/tests/phpunit/prepare-timing-results.php index 46a97b3596f39..7cef109048237 100644 --- a/tests/phpunit/prepare-timing-results.php +++ b/tests/phpunit/prepare-timing-results.php @@ -10,12 +10,44 @@ require_once __DIR__ . '/includes/class-wp-phpunit-timing-metrics.php'; -if ( 5 !== $argc ) { - fwrite( STDERR, "Usage: prepare-timing-results.php \n" ); +if ( 5 !== $argc && 6 !== $argc ) { + fwrite( STDERR, "Usage: prepare-timing-results.php [timing-metrics-file]\n" ); exit( 1 ); } try { + if ( 6 === $argc ) { + if ( ! is_readable( $argv[5] ) ) { + throw new RuntimeException( 'The prepared PHPUnit timing metrics could not be read.' ); + } + + $timing_metrics_json = file_get_contents( $argv[5] ); + + if ( false === $timing_metrics_json ) { + throw new RuntimeException( 'The prepared PHPUnit timing metrics could not be read.' ); + } + + $timing_metrics = json_decode( $timing_metrics_json, true, 512, JSON_THROW_ON_ERROR ); + $metric_keys = array( + 'phpunit-suite-time', + 'phpunit-p95-test-time', + 'phpunit-p99-test-time', + 'phpunit-max-test-time', + 'phpunit-tests-over-500ms', + 'phpunit-tests-over-1s', + ); + + if ( + ! is_array( $timing_metrics ) + || array_keys( $timing_metrics ) !== $metric_keys + || count( $timing_metrics ) !== count( array_filter( $timing_metrics, 'is_numeric' ) ) + ) { + throw new RuntimeException( 'The prepared PHPUnit timing metrics are invalid.' ); + } + } else { + $timing_metrics = WP_PHPUnit_Timing_Metrics::from_file( $argv[1] ); + } + $timestamp = new DateTimeImmutable( $argv[4] ); $payload = array( 'branch' => $argv[2], @@ -23,7 +55,7 @@ 'baseHash' => $argv[3], 'baseMetrics' => new stdClass(), 'timestamp' => $timestamp->format( DATE_ATOM ), - 'metrics' => WP_PHPUnit_Timing_Metrics::from_file( $argv[1] ), + 'metrics' => $timing_metrics, ); echo json_encode( $payload, JSON_THROW_ON_ERROR ) . "\n"; diff --git a/tests/phpunit/tests/includes/junitTimingMetrics.php b/tests/phpunit/tests/includes/junitTimingMetrics.php index 993f5d84da22b..c6465615c55ea 100644 --- a/tests/phpunit/tests/includes/junitTimingMetrics.php +++ b/tests/phpunit/tests/includes/junitTimingMetrics.php @@ -61,6 +61,55 @@ public function test_uses_testcase_time_when_suite_time_is_missing() { $this->assertSame( 0.6, $metrics['phpunit-suite-time'] ); } + public function test_passes_timed_testcase_details_to_callback() { + $file = $this->create_junit_file( array( 0.25, 1.5 ), 1.75 ); + $testcases = array(); + + WP_PHPUnit_Timing_Metrics::from_file( + $file, + static function ( $testcase ) use ( &$testcases ) { + $testcases[] = $testcase; + } + ); + + $this->assertSame( + array( + array( + 'name' => 'test_0', + 'class' => 'Tests_Example', + 'file' => '/var/www/tests/phpunit/tests/example.php', + 'line' => '100', + 'time' => 0.25, + 'time_display' => '0.25', + ), + array( + 'name' => 'test_1', + 'class' => 'Tests_Example', + 'file' => '/var/www/tests/phpunit/tests/example.php', + 'line' => '101', + 'time' => 1.5, + 'time_display' => '1.5', + ), + ), + $testcases + ); + } + + public function test_ignores_testcase_without_numeric_timing() { + $file = $this->create_junit_file( array( 0.5, null, 1.5 ), 2.0 ); + $testcases = array(); + + $metrics = WP_PHPUnit_Timing_Metrics::from_file( + $file, + static function ( $testcase ) use ( &$testcases ) { + $testcases[] = $testcase; + } + ); + + $this->assertCount( 2, $testcases ); + $this->assertSame( 1500.0, $metrics['phpunit-max-test-time'] ); + } + public function test_rejects_report_without_testcases() { $file = $this->create_junit_file( array(), 0.0 ); @@ -70,14 +119,48 @@ public function test_rejects_report_without_testcases() { WP_PHPUnit_Timing_Metrics::from_file( $file ); } + public function test_rejects_invalid_xml() { + $file = $this->create_temporary_file( '' ); + + $this->expectException( RuntimeException::class ); + $this->expectExceptionMessage( 'The JUnit timing report contains invalid XML.' ); + + WP_PHPUnit_Timing_Metrics::from_file( $file ); + } + /** * Creates a JUnit XML file for a test. * - * @param float[] $times Testcase times in seconds. - * @param float|int $suite_time Optional testsuite time in seconds. + * @param array $times Testcase times in seconds. Null omits the timing attribute. + * @param float|int|null $suite_time Optional testsuite time in seconds. * @return string Path to the temporary file. */ private function create_junit_file( $times, $suite_time = null ) { + $suite_time_attribute = null === $suite_time ? '' : sprintf( ' time="%s"', $suite_time ); + $testcases = ''; + + foreach ( $times as $index => $time ) { + $time_attribute = null === $time ? '' : sprintf( ' time="%s"', $time ); + $testcases .= sprintf( + '', + $index, + 100 + $index, + $time_attribute + ); + } + + return $this->create_temporary_file( + sprintf( '%3$s', count( $times ), $suite_time_attribute, $testcases ) + ); + } + + /** + * Creates a temporary file containing the provided data. + * + * @param string $data File contents. + * @return string Path to the temporary file. + */ + private function create_temporary_file( $data ) { $file = tempnam( sys_get_temp_dir(), 'junit-timing-' ); if ( false === $file ) { @@ -85,17 +168,7 @@ private function create_junit_file( $times, $suite_time = null ) { } $this->temporary_files[] = $file; - $suite_time_attribute = null === $suite_time ? '' : sprintf( ' time="%s"', $suite_time ); - $testcases = ''; - - foreach ( $times as $index => $time ) { - $testcases .= sprintf( '', $index, $time ); - } - - file_put_contents( - $file, - sprintf( '%3$s', count( $times ), $suite_time_attribute, $testcases ) - ); + file_put_contents( $file, $data ); return $file; }