Fix Codecov badge by adding OIDC permission for tokenless upload#856
Fix Codecov badge by adding OIDC permission for tokenless upload#856nimesh-xecurify wants to merge 15 commits intoWordPress:masterfrom
Conversation
The codecov-action v5 requires `id-token: write` to authenticate via OIDC when no CODECOV_TOKEN is provided. Without it the upload silently fails (fail_ci_if_error: false) leaving the badge as "unknown". The Codecov GitHub App is already installed on the WordPress org, so OIDC-based tokenless uploads work once this permission is granted.
|
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. |
Configures the Codecov GitHub Action to leverage OpenID Connect (OIDC) for authentication. This enhances security by utilizing GitHub's OIDC provider, removing the need for static tokens when uploading coverage reports.
Adds necessary repository read permissions to the test job. Creates a dedicated directory for storing test logs and coverage reports. These adjustments are preparatory steps for integrating code coverage reporting.
Introduces a new workflow step to extract the `clover.xml` code coverage report from the isolated test environment. This ensures the report is accessible on the GitHub Actions runner, allowing the `codecov-action` to successfully upload it. This step runs for PHP 8.3 and the latest WordPress versions.
The removed step incorrectly attempted to retrieve and process the code coverage report. The Codecov action can directly locate and upload coverage reports, making this step unnecessary and potentially confusing. Removing it streamlines the workflow.
The tests-cli container (ephemeral, runs PHPUnit) and tests-wordpress container (persistent) share the same named Docker volume at /var/www/html. PHPUnit writes clover.xml into this volume. Use docker exec on the running tests-wordpress container to cat the file to stdout and redirect it to the host — this bypasses the unreliable bind-mount passthrough and avoids the ephemeral-container state-sharing problem that caused the previous approach to fail.
masteradhoc
requested review from
Copilot,
georgestephanis,
kasparsd and
masteradhoc
There was a problem hiding this comment.
Pull request overview
Updates the CI workflow to restore Codecov uploads (and the repo badge) when using codecov/codecov-action without a CODECOV_TOKEN, by enabling OIDC authentication and adjusting the coverage upload inputs.
Changes:
- Add
id-token: write(and explicitcontents: read) permissions to the PHP test job to allow OIDC-based Codecov auth. - Ensure the coverage directory exists and attempt to retrieve
clover.xmlfrom the test container before uploading. - Update Codecov action inputs to use OIDC (
use_oidc: true) andfiles:for the coverage path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Temporary diagnostic: prints clover.xml size, first 30 lines, and xdebug mode from tests-cli to identify why CI coverage shows 0%.
Refines the workflow to reliably retrieve and upload `clover.xml` coverage reports from the Docker container. Adds detailed error checking for container existence and report file integrity. Clarifies the rationale behind creating `tests/logs` inside the container and using `docker exec` for retrieval. Removes `continue-on-error` to enforce stricter validation during report retrieval. Ensures Codecov upload only proceeds if a valid coverage report file exists.
Removes the explicit `docker exec` command for retrieving the `clover.xml` coverage report. The `tests/logs` directory is bind-mounted from the workspace into the Docker container, making the `clover.xml` file directly accessible on the host runner after PHPUnit completes. This makes the manual retrieval step redundant.
The `clover.xml` coverage report is generated inside the `tests-wordpress` container, but the plugin directory is not bind-mounted to the host. Previously, this meant the report was inaccessible for upload. This update uses `docker cp` to retrieve the `clover.xml` report from the container to the host filesystem after tests complete. This ensures the report is available for the Codecov upload step. Includes additional checks to confirm XDebug mode and verify the integrity of the retrieved coverage report.
Updates Composer install to use `--ignore-platform-req=php` on PHP 8.x, ensuring PHPUnit 9.6 is installed for compatible code coverage. Explicitly enables Xdebug coverage mode and passes `--coverage-clover` to PHPUnit within `wp-env` for the coverage matrix cell, guaranteeing reliable report generation. Refines comments regarding atomic file retrieval via `docker cp`.
Refactors the `wp-env` test command for PHP 8.3 and `latest` WordPress to ensure `XDEBUG_MODE=coverage` is correctly applied. Directly calls `wp-env` instead of using `npm run` to prevent argument re-splitting, which previously hindered accurate coverage generation.
Removes a conditional test execution path, allowing `npm run test` to consistently handle code coverage collection across all test matrix configurations, including PHP 8.3 with the latest WordPress. Eliminates unnecessary debugging output for `xdebug.mode` and `clover.xml` content to declutter CI logs. Refines comments for improved clarity regarding Composer platform requirements and `docker cp` usage.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runs-on: ubuntu-24.04 | ||
| permissions: | ||
| contents: read | ||
| id-token: write |
There was a problem hiding this comment.
id-token: write is granted at the job level, which means every matrix run (and every step in those runs) can mint OIDC tokens. Since only the Codecov upload needs OIDC, consider moving coverage upload into a separate job (or otherwise restricting when it runs, e.g., only on trusted push events) so untrusted PR code paths don’t receive id-token: write unnecessarily.
| id-token: write |
| # The composer.json platform override (php: 7.2.24) installs PHPUnit 8.5, which does | ||
| # not support code coverage on PHP 8. Use --ignore-platform-req=php on PHP 8+ so | ||
| # Composer installs PHPUnit 9.6, which supports coverage on PHP 8.x. | ||
| if [[ "${WP_ENV_PHP_VERSION}" == 8.* ]]; then | ||
| npm run composer -- install --ignore-platform-req=php | ||
| else | ||
| npm run composer install | ||
| fi |
There was a problem hiding this comment.
The --ignore-platform-req=php install path will run for all PHP 8.x matrix entries, changing the dependency set relative to the PHP 7.x runs (and relative to what the config.platform override is intended to enforce). If the goal is specifically to enable coverage generation for the single Codecov upload build, consider narrowing this condition to that build (e.g., the same matrix.php == '8.3' && matrix.wp == 'latest' gate) to reduce variability across the test matrix.
.github/workflows/test.yml
Outdated
| echo "Error: tests-wordpress container not found" | ||
| exit 1 | ||
| fi | ||
| mkdir -p tests/logs | ||
| docker cp "$CONTAINER:/var/www/html/wp-content/plugins/two-factor/tests/logs/clover.xml" tests/logs/clover.xml | ||
| if [ ! -s tests/logs/clover.xml ]; then | ||
| echo "Error: clover.xml is empty — coverage was not generated" | ||
| exit 1 |
There was a problem hiding this comment.
The “Retrieve coverage report from container” step fails the job if the container isn’t found or if clover.xml is empty. That makes CI more brittle even though the subsequent Codecov upload is configured with fail_ci_if_error: false. If the intent is to keep coverage/upload best-effort (badge-only), consider turning these hard failures into a warning + skip upload (or gating the failure to pushes on the default branch only).
| echo "Error: tests-wordpress container not found" | |
| exit 1 | |
| fi | |
| mkdir -p tests/logs | |
| docker cp "$CONTAINER:/var/www/html/wp-content/plugins/two-factor/tests/logs/clover.xml" tests/logs/clover.xml | |
| if [ ! -s tests/logs/clover.xml ]; then | |
| echo "Error: clover.xml is empty — coverage was not generated" | |
| exit 1 | |
| echo "Warning: tests-wordpress container not found; skipping coverage retrieval" | |
| exit 0 | |
| fi | |
| mkdir -p tests/logs | |
| if ! docker cp "$CONTAINER:/var/www/html/wp-content/plugins/two-factor/tests/logs/clover.xml" tests/logs/clover.xml; then | |
| echo "Warning: Failed to copy clover.xml from container; skipping coverage upload" | |
| exit 0 | |
| fi | |
| if [ ! -s tests/logs/clover.xml ]; then | |
| echo "Warning: clover.xml is empty — coverage was not generated; skipping coverage upload" | |
| exit 0 |
The coverage report (clover.xml) is now directly accessible on the runner environment. This eliminates the need for an explicit step to copy it from the Docker container, simplifying the CI workflow.
The codecov-action v5 requires
id-token: writeto authenticate via OIDC when no CODECOV_TOKEN is provided. Without it the upload silently fails (fail_ci_if_error: false) leaving the badge as "unknown".The Codecov GitHub App is already installed on the WordPress org, so OIDC-based tokenless uploads work once this permission is granted.
AI Usage
Used Claude Sonnet 4.6 AI for troubleshooting.