Possible improvements to coverage task

[bluejekyll]

So, I've noticed a few issues with the kcov-coverage task that is in the default library. I don't feel comfortable putting up a PR against the base makefile toml, but would be happy to if you see it as desirable.

The first is that I believe coverage data is overwritten when multiple tests are run. The next is that the current grep match statement for collecting the tests isn't general enough. And finally, macOS is now supported, though I've personally run into issues with it, so continue to leave it disabled.

1. Overlapping coverage reports: I have two strategies for this. Create a new directory for each kcov run. First is a top coverage report per crate, then also a coverage directory for each test executable run. You'll notice that to facilitate this I broke out a script for kcov as well.
   a) See here for the per crate target: https://github.com/bluejekyll/trust-dns/blob/master/Makefile.toml#L390-L396
   b) See here for the per test crate: https://github.com/bluejekyll/trust-dns/blob/master/scripts/kcov.sh#L12-L13

2. For the test matching, I grabbed a project cargo-with which uses the metadata from cargo to get all the tests. I was thinking, we might want to grab all targets in cargo-make and build some ways of filtering those, and offering the ability to wrap those in the same way as cargo-with, as I think it would be useful. Now I customized the cargo-with project to run all tests with a given command, and that hasn't landed in that project yet.
   a) see install here: https://github.com/bluejekyll/trust-dns/blob/master/Makefile.toml#L98-L103
   b) see usage here: https://github.com/bluejekyll/trust-dns/blob/master/Makefile.toml#L431-L432
   c) the PR oncargo-with: Allow multiple executions for test targets cbourjau/cargo-with#29

3. And finally, mac build, here's the install code I used for that. Now I found it beneficial to split the installer from the kcov task itself to ease testing the build of kcov, not sure if that's something you want: https://github.com/bluejekyll/trust-dns/blob/master/Makefile.toml#L137-L144

Also, thank you so much for this project. I think it's definitely the missing automation tool for Rust, and really fit my needs perfectly. I really appreciate your work on this. Thanks!

[sagiegurari]

thanks a lot. ya, seems like a lot of changes.

1. interesting point. usually i get coverage using unit tests mostly so didn't notice that integration tests coverage are getting overwritten. will check the issue and your solution.

2. prefer not to introduce cargo-with to the default build flow due to the fact that they seem to really slow down the build.
   I'm aware of caching and such, but its not always used.
   for example i also added cargo outdated to the CI flow and it added over 10 minutes in travis so i enable it only for master branch nightly builds.
   but coverage is something you need to run for all rust versions and platforms.
   that is why i enable you to customize the pattern via env. I know its not great.
   I'm not sure at this point what to do with this item.

3. really great news its working for mac. when doesn't it work?
   i'm working offline on some big change to enable a real cross platform scripting support which could help making the whole thing more portable to mac, although i'm pretty sure the bash itself should already be ok. so is the issue is kcov?
   as for the installation script, I would love to have your change Incorporated into cargo make
   So if you could public a PR for that part, it would be great (don't worry about changing the internal makefile, most PRs are for changing that :) )

[bluejekyll]

prefer not to introduce cargo-with

Totally understood. I do think the functionality might be something interesting to just pull into cargo-make... though we'd need to think about how that would work.

really great news its working for mac. when doesn't it work?

So I get a hang with my stuff, it might be something about the complexity of the tests, etc.

so is the issue is kcov?

yes, this is just for the kcov build, it's really just the brew installs (requires brew, but most devs on mac use this now).

[sagiegurari]

the functionality might be something interesting to just pull into cargo-make... though we'd need to think about how that would work.

if you want to investigate and maybe provide a pull request, that would be great.
we can then set that value as a default cargo make env (like other default env vars that are set in the code) and use it in the coverage or use it in the coverage pattern...
PR?

just the brew installs (requires brew, but most devs on mac use this now).

i saw your script code, it looks good to me to have brew install it for mac.
i'll be happy to get a PR for that. if some mac developer wants it differently, it can change later on.
for now, i see no harm in having it.
i'm not talking about enabling it by default to mac though if kcov hangs, but that people can enable easily by changing the condition or maybe i can add some env var specifically for that... but thats a different issue.

[sagiegurari]

@bluejekyll sorry for the really late handling.

1. mac installation - i used your code few release ago so thats in
2. Overlapping coverage reports - i think i resolved this one. each invocation of kcov goes to a new sub folder. codecov knows how to pick up all reports and merge so its fine and with this no report is overwriting the other. this will be in the 0.30.0 release which i'm planning to release real soon.

[bluejekyll]

Cool!