search

AtChem / AtChem2

Atmospheric chemistry box-model for the MCM
MIT License
59 stars 22 forks source link

Code coverage #501

Closed rs028 closed 10 months ago

rs028 commented 11 months ago

Fix the reporting of code coverage by Codecov (see #450).

Also requires modifications to the Makefile, as codecov works better without optimization and creation of the gcov files is not needed otherwise.

codecov[bot] commented 11 months ago

Codecov Report

Merging #501 (df049f3) into master (87e02fd) will decrease coverage by 14.67%. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AtChem/AtChem2/pull/501/graphs/tree.svg?width=650&height=150&src=pr&token=4p5Cr68G8w&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem)](https://app.codecov.io/gh/AtChem/AtChem2/pull/501?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) ```diff @@ Coverage Diff @@ ## master #501 +/- ## =========================================== - Coverage 66.66% 52.00% -14.67% =========================================== Files 17 17 Lines 2049 2096 +47 Branches 0 166 +166 =========================================== - Hits 1366 1090 -276 - Misses 683 934 +251 - Partials 0 72 +72 ``` | [Flag](https://app.codecov.io/gh/AtChem/AtChem2/pull/501/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) | Coverage Δ | | |---|---|---| | [build](https://app.codecov.io/gh/AtChem/AtChem2/pull/501/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) | `?` | | | [tests](https://app.codecov.io/gh/AtChem/AtChem2/pull/501/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) | `?` | | | [unittests](https://app.codecov.io/gh/AtChem/AtChem2/pull/501/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem#carryforward-flags-in-the-pull-request-comment) to find out more. [see 16 files with indirect coverage changes](https://app.codecov.io/gh/AtChem/AtChem2/pull/501/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AtChem/AtChem2/pull/501?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AtChem/AtChem2/pull/501?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem). Last update [87e02fd...df049f3](https://app.codecov.io/gh/AtChem/AtChem2/pull/501?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AtChem).
rs028 commented 11 months ago

@JamesAllsopp I have reorganized the ci.yml file as we discussed, but I haven't added your modifications yet (first I need to change the Makefile). Can you please have a look and see if I have missed something, when you have 10 minutes?

rs028 commented 11 months ago

Still needs some tidying up but it should more or less work. Basically it does 2 things:

  1. introduces a new variable in Makefile. If set to FALSE (default) adds -O2 to the gfortran flags. If set to TRUE, adds the gcov flags needed by codecov, but not needed for production runs (for which we want optimization instead).

  2. reorganize the ci.yml file so that there is no longer a standard run that is not part of a test and then it runs the tests twice: the first for numerical and behavioural accuracy (with -O2 flag) and the second for codecoverage (with gcov flags). The second run is done only for the latest linux and gfortran versions and uploads the results to Codecov for analysis.

I have also split the Makefile into two parts to make it easier to read and maintain.

JamesAllsopp commented 11 months ago

That's really good, should make the code easier to deal with

spco commented 11 months ago

I haven't gone through this in any depth, but it looks like a nice improvement! The key thing is - does the coverage now get measured, and change over time, in ways that make sense? :)

rs028 commented 11 months ago

The key thing is - does the coverage now get measured, and change over time, in ways that make sense? :)

That's the question! I think so, but not sure yet. I found out that I can reset the codecov info. So I could remove the tests, reset codecov and add the tests one by one to see what happens. But maybe first I should merge this, to keep things clean.