Skip to content

Add test coverage job #8161

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

Merged
merged 1 commit into from
Jun 24, 2022
Merged

Conversation

iluuu1994
Copy link
Member

@iluuu1994 iluuu1994 commented Mar 1, 2022

Coverage was previously executed in nightly and honestly I don't think most people even knew it was there. It takes a bit longer than the other jobs but it's a useful metric and might as well be more visible.

Targeting PHP 8.0 not because it's super useful there but to avoid conflicts when merging other CI changes upstream.

TODO:

  • Since we already have a --enable-debug --disable-zts job we probably might as well just combine the two, unless the job takes a significantly longer amount of time (I'll check after the jobs have finished)
  • Does the coverage change significantly with only the OpCache tests? It not, skip non-OpCache tests
  • Ignore coverage for zpp and other undesired macros We can do that on master
@iluuu1994 iluuu1994 force-pushed the github-actions-coverage branch from fc20957 to c4e6796 Compare March 1, 2022 22:36
@cmb69
Copy link
Member

cmb69 commented Mar 2, 2022

Thank you for working on this! I don't have a strong opinion about running for each push or nightly only, but one problem I see with coverage reports is that they may encourage users to provide potentially useless or even harmful tests for the sake of increased coverage. We should probably add some info about that to https://github.com/php/php-src/blob/master/CONTRIBUTING.md#writing-tests.

@iluuu1994
Copy link
Member Author

Maybe it's possible to ignore certain lines like ZPP to avoid encouraging useless tests, like adding LCOV_EXCL_START/STOP to the given macros.

https://stackoverflow.com/a/23264375/1320374

@iluuu1994
Copy link
Member Author

I changed my mind and moved this to nightly. It's also probably enough to do this for master, coverage of support branches shouldn't change much.

@iluuu1994 iluuu1994 force-pushed the github-actions-coverage branch from 4c8cfda to b7f0950 Compare June 23, 2022 22:21
@iluuu1994 iluuu1994 merged commit b7f0950 into php:PHP-8.0 Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment