iris icon indicating copy to clipboard operation
iris copied to clipboard

Add coverage testing

Open lbdreyer opened this issue 2 years ago • 1 comments

🚀 Pull Request

Description

This PR enables coverage testing using pytest-cov and coverage reporting using codacy.

There are a few different coverage reporting tools we can choose from:

  • coveralls (used in cartopy),
  • codecov (used in cf-units, iris-esmg-regrid, nc-time-axis, etc), and
  • codacy (not currently used although we seem to have it enabled but not running for cf-units)

I don't have a particularly strong preference between these tools, but a benefit of codacy (as @bjlittle pointed out) is that it provides extra features such as code quality inspection. It also has security scanning with bandit but that only seems to be available on the paid tiers unfortunately. Another benefit I have found when trialing these tools is that I have had difficulty with the codecov uploader and it would often fail silently, whereas codacy was quite simple to get working.

To implement codacy in this PR only required adding a single line to the .cirrus.yml file. Replacing this with a different coverage reporting tool (e.g. codecov) should be a simple change.

Outside of the changes in this PR, in order to get codacy working, we would also need to do the following:

  • Add the Iris repo to codacy (requires admin permissions which I don't have)
  • Add the CODACY_PROJECT_TOKEN to the Cirrus-CI settings (I tested this in my branch)

To see what it looks like on my branch you can see the main dashboard here and a report of coverage per files here

lbdreyer avatar May 24 '22 18:05 lbdreyer

@lbdreyer Fancy rebasing to pickup the migration of CI from cirrus-ci to GHA?

bjlittle avatar Jun 17 '22 16:06 bjlittle

This would be a really nice feature to have!

In my experience, Codacy is good for linting pull requests (we've been using it for several years in the ESMValCore project now, see here), but for code coverage, Codecov has been more useful to us because it makes the coverage and coverage changes much more visible by writing a comment on the pull request (example). Since we started using Codecov a bit over a year ago, the amount of code without coverage we have has halved (see here).

bouweandela avatar Oct 06 '22 15:10 bouweandela

Great to see some action again on this pull request!

bouweandela avatar Feb 21 '23 09:02 bouweandela

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@11da71b). Click here to learn what that means. Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4765   +/-   ##
=======================================
  Coverage        ?   89.25%           
=======================================
  Files           ?       88           
  Lines           ?    22197           
  Branches        ?     4858           
=======================================
  Hits            ?    19811           
  Misses          ?     1641           
  Partials        ?      745           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Feb 21 '23 11:02 codecov[bot]

This would be a really nice feature to have!

In my experience, Codacy is good for linting pull requests (we've been using it for several years in the ESMValCore project now, see here), but for code coverage, Codecov has been more useful to us because it makes the coverage and coverage changes much more visible by writing a comment on the pull request (example). Since we started using Codecov a bit over a year ago, the amount of code without coverage we have has halved (see here).

Thanks @bouweandela that is helpful to know about your experiences with codacy!

With that in mind I propose we use codecov and if at a later date we wish to explore codacy a bit more, we can look at implementing that. But for now codecov gives us what we want: coverage reporting on the PR

lbdreyer avatar Feb 21 '23 11:02 lbdreyer

@trexfeathers Thanks for reviewing this previously!

I have hopefully now addressed your comments.

I have also adopted codecov instead, which you can see from this report Note the report isn't showing anything yet as it doesn't have anything to compare to. Once this is merged, future PRs based of main should show the coverage change.

Do you mind taking another look?

lbdreyer avatar Feb 21 '23 11:02 lbdreyer

Thanks @trexfeathers !

lbdreyer avatar Feb 22 '23 17:02 lbdreyer

Very nice! Now I can see which of the lines are covered by tests: https://github.com/SciTools/iris/pull/5015#issuecomment-1442499808. That will make it much easier to understand if I can assume everything is fine or if I need to write extra tests when contributing.

bouweandela avatar Feb 24 '23 07:02 bouweandela