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 support with kcov. #792

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

tobinjt
Copy link

@tobinjt tobinjt commented Sep 18, 2023

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 redefining gather_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 variable KCOVERAGE_DIR.

Hopefully this will be generally useful and you'll consider merging. There are also some small cleanups, e.g. deleting trailing spaces.

Thanks,

@tobinjt tobinjt requested a review from a team as a code owner September 18, 2023 22:05
@AlexSkrypnyk
Copy link
Contributor

This is great!

It is also possible to run code coverage without this pr

kcov --clean --include-pattern=.bash --bash-parse-files-in-dir=. --exclude-path=node_modules,vendor,coverage coverage bats tests

With CodeCov enabled, this is how the PR of the project using BATS tests would look like
drevops/bats-helpers#14 (comment)

@AlexSkrypnyk
Copy link
Contributor

@tobinjt
Reading the code, I see that you are running SUT with kcov.

Here is an extensive example of running SUT with BATS wrapped in kcov

https://github.com/AlexSkrypnyk/bats-example

@AlexSkrypnyk
Copy link
Contributor

I had some extensive testing around BATS and kcov and now understand where this PR is coming from.

Running kcov from the "outside" of BATS will only capture coverage for functions within sourced files (unit tests) or SUT scripts that executed with run (functional tests), but only if the current working directory is not changed. The moment the directory is changed, like in cases where you want to run your scripts on the clone of your current codebase in the fixture directory, kcov looses context and no longer provides coverage for such scripts. I've tried different variations to overcome this, including moving the TMPDIR (the dir where BATS creates temp test dirs) to the same directory as the SUT, but kcov still refuses to see those.

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 run.

Thank you for working on this.


Only [kcov][kcov] is currently supported:

- To enable test coverage globally use `--kcoverage_dir <directory>`.
Copy link
Contributor

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

Copy link
Author

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.

@tobinjt
Copy link
Author

tobinjt commented Sep 21, 2023

This is great!

It is also possible to run code coverage without this pr

kcov --clean --include-pattern=.bash --bash-parse-files-in-dir=. --exclude-path=node_modules,vendor,coverage coverage bats tests

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 kcov --clean --bash-parse-files-in-dir=. $HOME/tmp/kcoverage bats test has been running for over 10 minutes now and appears to still be on the first test file :(

I think something is wrong with my invocation - I'm seeing an unending stream of lines to stderr like this (slightly redacted): # kcov@${HOME}/src/bats-core-mine/lib/bats-core/common.bash@5@printf '# %s\n' 'kcov@/${HOME}/src/bats-core-mine/lib/bats-core/common.bash@4@read -r line'

I'm running on MacOS if that makes any difference.

OK, adding --bash-method=DEBUG makes the tests pass in under 2 seconds - yay! The coverage for individual files is correct, the overall coverage is incorrect because it includes bats and apparently none of that code is executed?

With CodeCov enabled, this is how the PR of the project using BATS tests would look like drevops/bats-helpers#14 (comment)

@tobinjt
Copy link
Author

tobinjt commented Sep 21, 2023

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.

@AlexSkrypnyk
Copy link
Contributor

AlexSkrypnyk commented Sep 26, 2023

@tobinjt
I am on macos and can confirm that I too see a lot of lines which disappear when --bash-method=DEBUG is added.

To exclude bats (depending how you are loading it), you can use --exclude-pattern.

In my example below, I'm installing bats with NPM and excluding paths as a pattern.

kcov --include-pattern=.sh,.bash --bash-parse-files-in-dir=PATH_TO_SCRIPTS --exclude-pattern=node_modules,vendor,coverage coverage node_modules/.bin/bats test.bats

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, kcov is not fully documented - it is not clear how all the exclusions/inclusions behave when used together. In addition - different version of kcov behave differently. The docker image has a version 41 while the last version is 42, apt has version 38.

Secondly, the "functional" tests - the tests of the running of the script with run script.sh from the test - are covered with the command above by default (provided that you specify all the paths correctly).

But the "unit" tests - the tests that you would want to run for functions within script.sh - the coverage only discovered when you load "./script.sh or source it. It kinda makes sense that in order to tests a function inside of the script, you have to load it somehow. However, this means that the code of the script outside of the function will be executed when sourcing. A workaround - wrap every script into main() function and only run that like this:

# Run script only when it is executed directly.
if [ "$0" = "${BASH_SOURCE[0]}" ]; then
  main "$@"
fi

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 kcov what the current directory is, despite the fact that all paths are passed as absolute. This is not documented anywhere. This behaviour is weird since it is installed as a global bin.

@tobinjt
Copy link
Author

tobinjt commented Oct 22, 2023

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,

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.

None yet

2 participants