Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests lack code coverage reporting and @covers annotations #1284

Open
westonruter opened this issue Jun 6, 2024 · 5 comments · May be fixed by #1653
Open

Unit tests lack code coverage reporting and @covers annotations #1284

westonruter opened this issue Jun 6, 2024 · 5 comments · May be fixed by #1653
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

Feature Description

In looking at plugins/webp-uploads/tests/test-load.php, I noticed most of the tests lack @covers annotations. We should add those throughout our unit tests to better keep track of the code being tested. We should also add code coverage reporting for the repo (with Codecov). The phpunit calls in composer.json should also add --strict-coverage to report warnings when @covers tags are wrong.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure labels Jun 6, 2024
@westonruter westonruter added this to the performance-lab n.e.x.t milestone Jun 6, 2024
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Aug 13, 2024
@westonruter westonruter moved this from Not Started/Backlog 📆 to To Do 🔧 in WP Performance 2024 Aug 13, 2024
@westonruter westonruter removed this from the performance-lab n.e.x.t milestone Sep 20, 2024
@westonruter westonruter moved this from To Do 🔧 to In Progress 🚧 in WP Performance 2024 Nov 19, 2024
@westonruter
Copy link
Member Author

@sarthak-19 @thelovekesh Since the Performance repo is a monorepo of plugins, and since PHPUnit is going to be run separately for each plugin and thus there will be multiple coverage files, does this mean there could be multiple coverage reports? I think that would be somewhat helpful if each plugin's coverage was reported separately, although this for me is only a nice-to-have.

@thelovekesh
Copy link
Member

Yes @westonruter, that should be achievable.

@sarthak-19
Copy link
Contributor

sarthak-19 commented Nov 26, 2024

Problem

I managed to run the test locally and created a coverage-report.xml, previously report was not getting generated that's why one test was failing.

But for testing purpose when I pushed the code to my draft PR, all the test is failing.

Reason

For PHP 8.1^ it's failing because PHPUnit 10 is getting used instead of 9, and for below 8.1 PHP I'm not able to understand why it's failing.

  • When PHP Env is ^8.0, required PHPUnit should be 9.0^ for generating coverage report (PHPUnit ^10.0 throwing errors)
    Previously I was getting

This version of PHPUnit does not support code coverage on PHP 8
PHPUnit Version - 8.5.39
PHP Version -8.2.24

  • When PHP Env. is ^7.0, then PHPUnit ^9.0 will work or not, not sure.

Questions

  1. When different test case runs for different PHP versions in GH actions, how can I set PHPUnit versions for each test?
  2. For generating coverage report, xdebug must be set else we'll get No coverage driver available.
    So for testing purpose I've modified package.json as --> "wp-env": "wp-env start --xdebug=develop,coverage",, But I assume this is not the correct way to do it, so how can I enable xdebug only for env where code coverage is needed.
  3. I think that would be somewhat helpful if each plugin's coverage was reported separately.

When testing locally, each plugin report was getting generated but was getting replaced due to same file name, so how can I set different file name for each plugin?

cc : @westonruter @thelovekesh

@westonruter
Copy link
Member Author

  • When different test case runs for different PHP versions in GH actions, how can I set PHPUnit versions for each test?

Here's how the AMP plugin configures specific PHPUnit versions:

https://github.com/ampproject/amp-wp/blob/d101379ce2044924b38834b873638547f54bcc82/.github/workflows/build-test-measure.yml#L293-L344

https://github.com/ampproject/amp-wp/blob/d101379ce2044924b38834b873638547f54bcc82/.github/workflows/build-test-measure.yml#L382-L388

https://github.com/ampproject/amp-wp/blob/d101379ce2044924b38834b873638547f54bcc82/.github/workflows/build-test-measure.yml#L398-L403

I'm not sure that's what we should do here as well.

@westonruter
Copy link
Member Author

When testing locally, each plugin report was getting generated but was getting replaced due to same file name, so how can I set different file name for each plugin?

Currently all the tests are being run together via npm run test-php:

- name: Running single site unit tests
run: |
if [ "${{ matrix.coverage }}" == "true" ]; then
npm run test-php -- -- -- --coverage-clover=coverage-${{ github.sha }}.xml
else
npm run test-php
fi
- name: Running multisite unit tests
run: |
if [ "${{ matrix.coverage }}" == "true" ]; then
npm run test-php-multisite -- -- --coverage-clover=coverage-multisite-${{ github.sha }}.xml
else
npm run test-php-multisite
fi

This npm run test-php command, however, is just a shortcut for running multiple composer commands:

"test-php": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:plugins",

performance/composer.json

Lines 117 to 127 in 83b2117

"test:plugins": [
"@test:auto-sizes",
"@test:dominant-color-images",
"@test:embed-optimizer",
"@test:image-prioritizer",
"@test:optimization-detective",
"@test:performance-lab",
"@test:speculation-rules",
"@test:web-worker-offloading",
"@test:webp-uploads"
],

So I think the workflow just needs to invoke each of the plugins' test commands separately:

performance/package.json

Lines 57 to 65 in 83b2117

"test-php:performance-lab": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:performance-lab",
"test-php:auto-sizes": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:auto-sizes",
"test-php:dominant-color-images": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:dominant-color-images",
"test-php:embed-optimizer": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:embed-optimizer",
"test-php:image-prioritizer": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:image-prioritizer",
"test-php:optimization-detective": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:optimization-detective",
"test-php:speculation-rules": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:speculation-rules",
"test-php:web-worker-offloading": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:web-worker-offloading",
"test-php:webp-uploads": "wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/performance composer test:webp-uploads",

@eclarke1 eclarke1 moved this from In Progress 🚧 to Code Review 👀 in WP Performance 2024 Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: Code Review 👀
3 participants