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

Add coverage job #571

Merged
merged 1 commit into from
Nov 23, 2024
Merged

Conversation

NWilson
Copy link
Member

@NWilson NWilson commented Nov 22, 2024

Fixes #555

@zherczeg
Copy link
Collaborator

This looks useful.

pcre2_jit_compile.c.html#L102: 58.9k #define MACHINE_STACK_SIZE 32768

Interesting that a const has coverage. Some macros has coverage, others don't, so probably not all code is fully "covered" by the tool :)

@NWilson
Copy link
Member Author

NWilson commented Nov 22, 2024

I'd guess that constants which are built into the binary will be counted as "code" and generate coverage.

Macros which are only used by the preprocessor won't generate coverage. They have to actually write out some text which is seen as C code by the compiler.

I'd be interested in trying out the "Coveralls" or "CodeCov" services, which look like good ways to track coverage over time, coverage diff in a PR, and so on.

@NWilson NWilson marked this pull request as ready for review November 22, 2024 13:09
.github/workflows/build.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@carenas carenas left a comment

Choose a reason for hiding this comment

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

it might be better to move this job into their own "tools.yml" in te long run, though

@zherczeg zherczeg merged commit f4c5bcf into PCRE2Project:master Nov 23, 2024
18 checks passed
@NWilson
Copy link
Member Author

NWilson commented Nov 25, 2024

it might be better to move this job into their own "tools.yml" in te long run, though

I feel like code-scanning tools (like CodeQL, or Clang Static Analyzer) should be in their own jobs.

However, code coverage should be part of the Release "Build" pipeline (not the "Dev" pipeline) because it's actually a fixed, reportable artifact of the release process: "we built the code; we ran the tests; the tests covered 85% of the code; now we release the binaries".

@carenas
Copy link
Contributor

carenas commented Nov 25, 2024

I think you are seeing too much on those names, I created the files as ways to automte the work that Philip does on each release, and build.yml was the first; with a simple objective to make sure the code actually builds in the few pltforms we could easily use through github actions. I was specially motivated to make sure macOS would work because neither Philip or Zoltan have access to it and the build was broken by a contributor merged patch and almost released once.

dev.yml came later after a lot more contributor provided patches were found to introduce problems that I thought we could have prevented with feedback at PR time. For those, the ideas was to cover as much of the code (hence why DEBUG is on) so we don't have blind spots, and probably a comparative code coverage job would be also sensible there.

Eitherway, we are a very small team, none of us doing this as "work", so our perspectives will obviously differ, and indeed some people assume that our jobs are a serious CI/CD when they are meant only as additional automation on top of developer directed tests, but feel free to tweak them for your needs as well.

@NWilson NWilson deleted the user/niwilson/lcov branch December 17, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lcov coverage report to CI jobs
3 participants