Implement Code Coverage check logic #144
Conversation
|
|
||
| set -e | ||
|
|
||
| workdir=.cover |
There was a problem hiding this comment.
Can we add this to the .gitignore? Although the check_coverage.sh script deletes this directory, this script doesn't when run alone... which is a minor inconvenience if you're looking at your coverage profiles locally.
validate.sh
Outdated
| @@ -29,4 +29,5 @@ else | |||
| test $GOFMT_LINES -eq 0 || die "gofmt needs to be run, ${GOFMT_LINES} files have issues. Below is a list of files to review:\n`gofmt -l *.go pbs adapters`" | |||
| fi | |||
|
|
|||
| go test $(go list ./... | grep -v /vendor/) | |||
| #go test $(go list ./... | grep -v /vendor/) | |||
There was a problem hiding this comment.
Can we preserve "tests without coverage" in another scripts/* file?
Typical developer workflow is something like:
Implement feature with initial code & tests
check code coverage
while test coverage is too low:
add more tests
while tests fail:
fix code
Instrumented code is much slower than regular code... and developers only care about it for a tiny % of the workflow. So a standalone "test without coverage" script will save a ton of development time overall.
scripts/coverage.sh
Outdated
|
|
||
| push_to_coveralls() { | ||
| echo "Pushing coverage statistics to coveralls.io" | ||
| goveralls -coverprofile="$profile" |
There was a problem hiding this comment.
We should add this somewhere in the README.md as a required startup step. Developers have to install it separately in order to run this, right?
scripts/check_coverage.sh
Outdated
| trap finish EXIT ERR INT TERM | ||
|
|
||
| #start script logic | ||
| ./scripts/coverage.sh > scripts/results.tmp |
There was a problem hiding this comment.
any reason this tmp file is in scripts, rather than .cover?
Not a big deal... just wondering, cause it seems out of place and complicates things a bit.
There was a problem hiding this comment.
I've been playing around with this part of the code to have the final results.tmp file located in the same folder with the rest of the package coverage files (ie .cover). When the main script coverage script (ie coverage.sh) runs, it recreates the .cover directory so that it contains data relevant to that run. When I try to pipe the results from coverage.sh to a file within this .cover directory, it gets deleted when the directory is recreated (since the results.tmp is created first to store the results before the script runs).
So to maintain the output in the results.tmp file for the second half of the check_coverage.sh script, the results.tmp file has to be located in a different directory than where the rest of the coverage data sits.
If you have any thoughts/suggestions around this to try out, please let me know.
|
Pushed some updates based on the feedback from Dave. To clarify though, based on a discussion I had with him - I updated one additional aspect of this code. The validate.sh script now runs the coverage check only with a flag option (--cov) instead of by default (which should be more ideal for devs when doing their quick local testing). To have the framework automatically incorporate the coverage check aspect in whether it accepts code changes, I altered the .travis.yml file to add the flag when it runs the validate.sh script. |
| @@ -0,0 +1,32 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
any particular reason why there is 2 script files instead of one? Seems like both are simple enough that it could be covered with a single one (pun intended).
There was a problem hiding this comment.
I had them split mainly for organizational reasons; so that each script performed a specific task.
I've tested merging them together into one single script and got it working (passing, failing and rendering the detailed html report file); so I could implement this change if desired.
Please let me know.
Added two scripts and a change to the validate.sh script to setup an automatic coverage check on the packages within the PBS framework.
The coverage.sh file performs the actual coverage check process for each package and then collates the data together into one coverage file. From the terminal, this script can be ran separately with the --html flag to produce a more detailed coverage report (eg $ ./scripts/coverage.sh --html).
The check_coverage.sh file calls the coverage.sh file and reads through the (coverage) results for each package to detect if any area is below a certain threshold percentage. Currently this percentage is 20 (to get this implemented and allow for the chance for more unit tests to be written); later, this threshold would ideally be at 80%.
The validate.sh script is now configured to use these new scripts instead of the basic go test command, so that if a code coverage check falls below the threshold the code change will be blocked.