Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions .github/workflows/reusable-phpunit-tests-v3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ on:

env:
LOCAL_PHP: ${{ inputs.php }}-fpm
LOCAL_PHP_XDEBUG: ${{ inputs.coverage-report || false }}
LOCAL_PHP_XDEBUG_MODE: ${{ inputs.coverage-report && 'coverage' || 'develop,debug' }}
LOCAL_PHP_XDEBUG: false
LOCAL_PHP_XDEBUG_MODE: 'develop,debug'
LOCAL_DB_TYPE: ${{ inputs.db-type }}
LOCAL_DB_VERSION: ${{ inputs.db-version }}
LOCAL_PHP_MEMCACHED: ${{ inputs.memcached }}
Expand Down Expand Up @@ -113,7 +113,6 @@ jobs:
# - Install WordPress within the Docker container.
# - Run the PHPUnit tests.
# - Upload the code coverage report to Codecov.io.
# - Upload the HTML code coverage report as an artifact.
# - Ensures version-controlled files are not modified or deleted.
# - Checks out the WordPress Test reporter repository.
# - Submit the test results to the WordPress.org host test results.
Expand Down Expand Up @@ -201,15 +200,40 @@ jobs:
- name: Install WordPress
run: npm run env:install

# Installs PCOV as the code coverage driver for the PHPUnit run below.
#
# The INI directives tune PCOV for WordPress's codebase:
# - `pcov.enabled` keeps the Zend hooks active (this is the default, but
# stated explicitly for clarity).
# - `pcov.directory` restricts instrumentation to `src/`, so PCOV does not
# record hits for `vendor/`, `tests/`, or WordPress test fixtures that
# PHPUnit would discard at report time anyway.
# - `pcov.initial.files` pre-sizes the internal file tracking array for
# the thousands of files under `src/`, avoiding reallocation churn
# during test warmup. The default of 64 is far too low here.
- name: Install PCOV coverage driver
if: ${{ inputs.coverage-report }}
run: |
docker compose exec -T -u 0 php sh -c '
pecl install pcov &&
docker-php-ext-enable pcov &&
{
echo "pcov.enabled=1"
echo "pcov.directory=/var/www/src"
echo "pcov.initial.files=2000"
} >> /usr/local/etc/php/conf.d/docker-php-ext-pcov.ini &&
php -m | grep -i pcov
Comment thread
desrosj marked this conversation as resolved.
'
Comment on lines +203 to +226
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The setup-php action supports PCOV as a coverage tool. When we are finally able to explore running the unit tests outside of the Docker environment, this becomes simply supplying values to the ini-values input.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice!


Comment on lines +203 to +227
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I checked and there are 1,852 PHP files in src/, so this number should work. However, I'm concerned that this number will just end up being set and never updated over time.

What if we add a simple file counting step before this one and round the number of files up to the nearest hundred before using that value for pcov.initial.files?

Suggested change
# Installs PCOV as the code coverage driver for the PHPUnit run below.
#
# The INI directives tune PCOV for WordPress's codebase:
# - `pcov.enabled` keeps the Zend hooks active (this is the default, but
# stated explicitly for clarity).
# - `pcov.directory` restricts instrumentation to `src/`, so PCOV does not
# record hits for `vendor/`, `tests/`, or WordPress test fixtures that
# PHPUnit would discard at report time anyway.
# - `pcov.initial.files` pre-sizes the internal file tracking array for
# the thousands of files under `src/`, avoiding reallocation churn
# during test warmup. The default of 64 is far too low here.
- name: Install PCOV coverage driver
if: ${{ inputs.coverage-report }}
run: |
docker compose exec -T -u 0 php sh -c '
pecl install pcov &&
docker-php-ext-enable pcov &&
{
echo "pcov.enabled=1"
echo "pcov.directory=/var/www/src"
echo "pcov.initial.files=2000"
} >> /usr/local/etc/php/conf.d/docker-php-ext-pcov.ini &&
php -m | grep -i pcov
'
- name: Count PHP source files
id: count_php_files
if: ${{ inputs.coverage-report }}
run: |
total=$(find src -type f -name '*.php' -print | wc -l | tr -d '[:space:]')
rounded=$(( ( total + 99 ) / 100 * 100 ))
echo "count=$rounded" >> "$GITHUB_OUTPUT"
echo "::info::PHP files under src/: ${total}; pcov.initial.files=${rounded}"
# Installs PCOV as the code coverage driver for the PHPUnit run below.
#
# The INI directives tune PCOV for WordPress's codebase:
# - `pcov.enabled` keeps the Zend hooks active (this is the default, but
# stated explicitly for clarity).
# - `pcov.directory` restricts instrumentation to `src/`, so PCOV does not
# record hits for `vendor/`, `tests/`, or WordPress test fixtures that
# PHPUnit would discard at report time anyway.
# - `pcov.initial.files` pre-sizes the internal file tracking array for
# the rough number of files under `src/`, avoiding reallocation churn
# during test warmup. The default of 64 is far too low here.
- name: Install PCOV coverage driver
if: ${{ inputs.coverage-report }}
env:
PCOV_INITIAL_FILES: ${{ steps.count_php_files.outputs.count }}
run: |
docker compose exec -T -u 0 php sh -c '
pecl install pcov &&
docker-php-ext-enable pcov &&
{
echo pcov.enabled=1
echo pcov.directory=/var/www/src
echo pcov.initial.files=$PCOV_INITIAL_FILES
} >> /usr/local/etc/php/conf.d/docker-php-ext-pcov.ini &&
php -m | grep -i pcov
'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah I ran the count before deciding on 2,000. This setting reserves the given size for a hash table used during the coverage run, if it's exceeded it just means some resizing occurs. I wonder if the find might take longer than the hash table resizing. No clue.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Claude tells me just to use 10,000 which will allocate ~512KB of memory and is plenty big enough. Sound good?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ya, I think works here. But if we choose to go the "PCOV installed in Docker for us" route, we'd have to apply that number there. A number that high also negates the need for a related environment variable in the local environment, which is a positive.

- name: Run PHPUnit tests${{ inputs.phpunit-test-groups && format( ' ({0} groups)', inputs.phpunit-test-groups ) || '' }}${{ inputs.coverage-report && ' with coverage report' || '' }}
continue-on-error: ${{ inputs.allow-errors }}
run: |
node ./tools/local-env/scripts/docker.js run \
node ./tools/local-env/scripts/docker.js ${{ inputs.coverage-report && 'exec' || 'run' }} \
php ./vendor/bin/phpunit \
--verbose \
-c "${PHPUNIT_CONFIG}" \
${{ inputs.phpunit-test-groups && '--group "${TEST_GROUPS}"' || '' }} \
${{ inputs.coverage-report && '--coverage-clover "wp-code-coverage-${MULTISITE_FLAG}-${GITHUB_SHA}.xml" --coverage-html "wp-code-coverage-${MULTISITE_FLAG}-${GITHUB_SHA}"' || '' }}
${{ inputs.coverage-report && '--coverage-clover "wp-code-coverage-${MULTISITE_FLAG}-${GITHUB_SHA}.xml"' || '' }}
env:
TEST_GROUPS: ${{ inputs.phpunit-test-groups }}
MULTISITE_FLAG: ${{ inputs.multisite && 'multisite' || 'single' }}
Expand Down Expand Up @@ -244,14 +268,6 @@ jobs:
flags: ${{ inputs.multisite && 'multisite' || 'single' }},php
fail_ci_if_error: true

- name: Upload HTML coverage report as artifact
if: ${{ inputs.coverage-report }}
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
with:
name: wp-code-coverage${{ inputs.multisite && '-multisite' || '-single' }}-${{ github.sha }}
path: wp-code-coverage${{ inputs.multisite && '-multisite' || '-single' }}-${{ github.sha }}
overwrite: true

- name: Ensure version-controlled files are not modified or deleted
run: git diff --exit-code

Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/test-coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ concurrency:
permissions: {}

env:
LOCAL_PHP_XDEBUG: true
LOCAL_PHP_XDEBUG_MODE: 'coverage'
LOCAL_PHP_MEMCACHED: false
PUPPETEER_SKIP_DOWNLOAD: true

Expand All @@ -52,7 +50,7 @@ jobs:
uses: ./.github/workflows/reusable-phpunit-tests-v3.yml
permissions:
contents: read
if: ${{ github.repository == 'WordPress/wordpress-develop' }}
# if: ${{ github.repository == 'WordPress/wordpress-develop' }}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This needs to be reverted prior to merge. Can stay in place for now.

strategy:
fail-fast: false
matrix:
Expand Down
Loading