-
Notifications
You must be signed in to change notification settings - Fork 430
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 support with kcov. #792
base: master
Are you sure you want to change the base?
Conversation
This is great! It is also possible to run code coverage without this pr
With CodeCov enabled, this is how the PR of the project using BATS tests would look like |
@tobinjt Here is an extensive example of running SUT with BATS wrapped in kcov |
I had some extensive testing around BATS and kcov and now understand where this PR is coming from. Running So, while it feels a bit wrong to run coverage from inside of the testing system, there is currently no other way (unless kcov can fix this somehow), so your solution would work. If it is not accepted by maintainers, I think it is still possible to extract it as a helper and use it as a replacement from Thank you for working on this. |
|
||
Only [kcov][kcov] is currently supported: | ||
|
||
- To enable test coverage globally use `--kcoverage_dir <directory>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if kcov
is potentially replaceable with another coverage tool, should the API defined in a more generic way?
--coverage-dir
option and BATS_COVERAGE_DIR
env var
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'm happy to change the API - maybe something like this would be better?
--coverage_dir
and$BATS_COVERAGE_DIR
--coverage_tool
and$BATS_COVERAGE_TOOL
Then only gather_coverage
would need changes to support a new coverage tool.
Thanks for the info, and the link to https://github.com/AlexSkrypnyk/bats-example, I'm trying that out now. Is it expected to take a really long time? My tests, with coverage enabled, run in under 2 seconds; this command I think something is wrong with my invocation - I'm seeing an unending stream of lines to stderr like this (slightly redacted): I'm running on MacOS if that makes any difference. OK, adding
|
An alternative to merging this PR is to document what's known about gathering coverage so that people have copy'n'pastable commands and clearly described caveats to be aware of. |
@tobinjt To exclude bats (depending how you are loading it), you can use In my example below, I'm installing bats with NPM and excluding paths as a pattern.
I have already spent several days trying to make it all work together, so that both functional and unit tests would have coverage and this is really really hard. Firstly, Secondly, the "functional" tests - the tests of the running of the script with But the "unit" tests - the tests that you would want to run for functions within
All of this feels like one big hack. A coverage should be easily available from a testing system. Maybe it is worth to have BATS project maintainers looking into all of this from a different approach altogether. [UPDATE] I finally found a root of my issues - it matters for |
G'day, What approach would y'all like to take here? Is this PR useful? Would generalising the flags etc. to support multiple coverage engines be better than this PR? Would documenting how to run bats under coverage tools be a better approach? Thanks, |
G'day,
This commit adds support for gathering coverage using kcov (https://github.com/SimonKagstrom/kcov). Coverage is only collected for programs executed with
run
, not for functions that are called internally. The structure supports running other tooling for gathering coverage, either by users redefininggather_coverage
inside tests or by future changes extending the framework. Collection can be enabled with the flag--kcoverage_dir
or switched on or off more granularly by setting or clearing the environment variableKCOVERAGE_DIR
.Hopefully this will be generally useful and you'll consider merging. There are also some small cleanups, e.g. deleting trailing spaces.
Thanks,