Adding Coveralls or similar to auto-check new PR for unit tests

[argerlt]

Is there a good reason for not implementing coveralls in the hexrd default PR actions?

For reference, coveralls gives the percentage of code called by unit tests, which is necessary for eventually ensuring actual code coverage, as opposed to assumed, and is super helpful for catching edge cases.

Example from Orix, a different coding project I often work on: https://github.com/pyxem/orix/issues/7

And example of where I made a pretty hefty commit: https://github.com/pyxem/orix/pull/450/commits/462b513e568cd38033c3a04fef016000703234c1 and then coverall auto-caught an untested line for me, so I made a new unittest to address it: https://github.com/pyxem/orix/pull/450/commits/15945593066500737f69f38bc6e03d18271a0356

Coveralls is also super easy to run on your own computer as a pre-flight check in command line:

https://orix.readthedocs.io/en/stable/dev/running_writing_tests.html

Its the sort of thing everyone should be doing before any PR, but often don't unless forced to. I can implement all this pretty quick, just want to check if there was a good reason not to.

Ideally, the end goal would be to completely cover hexrd, then make it so new PR cannot be merged to main unless they had unit tests, but short term, it would just be a report for users to self-check their code's completeness.

[psavery]

I think that adding some sort of coverage would definitely be helpful. I'm personally more familiar with codecov.

But I'm also aware that we probably have pretty bad coverage, frankly. It's something we need to work on.

[argerlt]

I believe codecov costs money, whereas coveralls is free as long as the repo is public. Other than that though, I believe they are functionally identical. (as in, if there is money and you would prefer codecov, I'm down to add that instead)

Also, semi-related, is there interest in adding auto-PEP-8 compliance checking, like python black? Again, since much of HEXRD is not currently compliant, wouldn't deny commits, would just give a line count for non-compliant lines.

I ask because it is easy to add both at the same time, since both can run as part of pytest

[psavery]

I believe codecov costs money, whereas coveralls is free as long as the repo is public. Other than that though, I believe they are functionally identical. (as in, if there is money and you would prefer codecov, I'm down to add that instead)

Codecov is actually free for open source projects. I'm not necessarily opposed to using coveralls, but I don't know much about it. I'm guessing it is pretty similar.

Also, semi-related, is there interest in adding auto-PEP-8 compliance checking, like python black? Again, since much of HEXRD is not currently compliant, wouldn't deny commits, would just give a line count for non-compliant lines.

Yes, it would be good to look into this as well. If we use Black, we would reduce the line length to 79 characters to match pep8, and skip string normalization since we prefer single quotes in this project (it has always bothered me that Black does not provide an option to enforce single quotes).

[argerlt]

Codecov is actually free for open source projects.

Oh, Neat, I'll try that then. It makes more sense to fit to what you are used to than what I am, based on just number of commits alone.

Okay, I'll submit a PR adding a workflow implementing both. We can see how it looks/works and go from there.

[psavery]

Sounds great, thanks Austin!

[kevindlewis23]

Seems like code coverage was never added, currently added the Code coverage PR. I don't think I have permissions to link it to this issue

[argerlt]

Sounds good, I'm glad someone had a similar thought. I started a PR but didn't like my implementation and then got pulled other directions. Plus, it seems more people are familiar with codecov, so win-win.

If #656 gets merged, feel free to close this issue as well.