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

CI/QA: start recording code coverage #144

Merged
merged 4 commits into from
Dec 9, 2023

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 9, 2023

Description

Unit tests: add @covers annotation to all sniff tests

Unit tests: add @covers annotation to all utility method tests

PHPUnit config: set up to allow for recording code coverage

The forceCoversAnnotation attribute will prevent code coverage from being recorded for tests without a @covers tag.

As for recording the code coverage:
By default, a code coverage text summary will be shown and a clover.xml file will be generated in a build/logs directory.
The clover file can be used to generate code coverage reports via a service like Coveralls.

For local development, the clover.xml is not very human readable-friendly and the summary is a little too minimal, so an HTML report would be better.

To that end, a coverage-local script has been added to the composer.json to make it straight forward for contributors to generate the HTML report.
The HTML report will be placed in a build/coverage-html directory.

The build directory is now excluded via the .gitignore file.

GH Actions: start recording code coverage

This commit reworks the test workflow to start running the tests with code coverage for low/medium/high PHP and upload the generated reports to Coveralls.

  • It adds an extra "coverage" job which runs only the unit tests with code coverage against low/medium/high PHP.
    This job does not run the CS check as an integration test.
  • Makes minor tweaks to the pre-existing "test" job to prevent duplicate test runs for the exact same PHP/custom ini combinations.
    No need to run the same check twice.

The recorded code coverage reports will be available on: https://coveralls.io/github/PHPCSStandards/PHP_CodeSniffer

Includes adding a code coverage badge to the README..

Suggested changelog entry

N/A

@jrfnl jrfnl added this to the 3.x Next milestone Dec 9, 2023
@jrfnl jrfnl force-pushed the feature/unit-tests-add-covers-annotations branch from 04bfe05 to 432e6b9 Compare December 9, 2023 17:44
Copy link

@echoix echoix left a comment

Choose a reason for hiding this comment

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

I read through all the changes of the files. Seems ok. But my opinion might only be worth for the GitHub actions part ;)

@jrfnl
Copy link
Member Author

jrfnl commented Dec 9, 2023

I read through all the changes of the files. Seems ok. But my opinion might only be worth for the GitHub actions part ;)

@echoix Thank you for looking this over! I'm still fiddling a bit with the GH Actions script as the test script runs a combination of tasks, only one of which is running the PHPUnit tests.
Xdebug being on makes the other tasks much slower, while those don't need Xdebug in the first place. This is particularly noticeable on PHP 5.4, for which the run now took 6 minutes.

Unfortunately, XDEBUG_MODE=off cannot be used for those tasks as that's an Xdebug 3 feature and wouldn't work on PHP 5.4.

@echoix
Copy link

echoix commented Dec 9, 2023

I'm maintaining https://github.com/oxsecurity/megalinter. The biggest job was around 1h. In the second repo I follow, https://github.com/OSGeo/grass , the longest job (when they can run, since they often get queued), it's 1h30 of runtime (but we want to have it faster).

If 6 min doesn't bother you yet, you can do other nice things in the meantime

@jrfnl
Copy link
Member Author

jrfnl commented Dec 9, 2023

I'm maintaining GitHub.com/oxsecurity/megalinter. The biggest job was around 1h. In the second repo I follow, https://github.com/OSGeo/grass , the longest job (when they can run, since they often get queued), it's 1h30 of runtime (but we want to have it faster).

If 6 min doesn't bother you yet, you can do other nice things in the meantime

😁 Yeah, we have some of those 1 hour+ jobs for the Composer install plugin too.

I don't mind slow jobs if there is good reason for it (more tests, end-to-end tests etc), I do mind when I know the cause and it's preventable ;-)

@jrfnl jrfnl force-pushed the feature/unit-tests-add-covers-annotations branch from 432e6b9 to 8cbd19f Compare December 9, 2023 21:48
The `forceCoversAnnotation` attribute will prevent code coverage from being recorded for tests without a `@covers` tag.

As for recording the code coverage:
By default, a code coverage text summary will be shown and a `clover.xml` file will be generated in a `build/logs` directory.
The clover file can be used to generate code coverage reports via a service like Coveralls.

For local development, the `clover.xml` is not very human readable-friendly and the summary is a little too minimal, so an HTML report would be better.

To that end, a `coverage-local` script has been added to the `composer.json` to make it straight forward for contributors to generate the HTML report.
The HTML report will be placed in a `build/coverage-html` directory.

The `build` directory is now excluded via the `.gitignore` file.
This commit reworks the `test` workflow to start running the tests with code coverage for low/medium/high PHP and upload the generated reports to Coveralls.

* It adds an extra "coverage" job which runs only the unit tests with code coverage against low/medium/high PHP.
    This job does not run the CS check as an integration test.
* Makes minor tweaks to the pre-existing "test" job to prevent duplicate test runs for the exact same PHP/custom ini combinations.
    No need to run the same check twice.

The recorded code coverage reports will be available on: https://coveralls.io/github/PHPCSStandards/PHP_CodeSniffer

Includes adding a code coverage badge to the README.
@jrfnl jrfnl force-pushed the feature/unit-tests-add-covers-annotations branch from 8cbd19f to 3f29c24 Compare December 9, 2023 22:24
@jrfnl jrfnl merged commit 5a2a376 into master Dec 9, 2023
45 checks passed
@jrfnl jrfnl deleted the feature/unit-tests-add-covers-annotations branch December 9, 2023 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants