Change codecov from master to main #585
Conversation
Code coverage was shown incorrectly and pointed to the 'old' master branch. Changed it to main and the coverage seems to be correct.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #585 +/- ##
===========================================
- Coverage 99.31% 69.28% -30.03%
===========================================
Files 39 62 +23
Lines 3485 7244 +3759
===========================================
+ Hits 3461 5019 +1558
- Misses 24 2225 +2201 ☔ View full report in Codecov by Sentry. |
|
The change itself seems fine but our coverage is not really 99%, it is 87% now (see printout below from pytest). Should we look into it or merge this and figure it out later? |
|
By the way, our actual coverage, calculated using pytest and including the integration tests in the calculation is 92%, which is really good. This is after a few additions which will be included in a small PR soon, but they are not that significant. |
|
Where do we stand with this? Do we get the right coverage numbers with it now? (I am not sure where to check.) |
|
I have to admit the discussion around this PR leaves and the different coverage results leaves me a bit confused. No idea what to do and which numbers are reliable. The changes in this PR all make sense, but I have no idea what the coverage number means. |
|
I agree the changes themselves look fine, but our actual coverage is 92% and not 99% that the badge in this branch gives. It seems that the problem is that codecov is measuring the coverage of the tests directory rather than the actual project (see - https://app.codecov.io/gh/gammasim/simtools/tree/main?displayType=tree). Not sure how to fix it. Will see if I find a moment of boredom to dig around at some point, but I suggest we keep this open so we don't forget. |
|
I officially give up on this and think we should simply remove that badge and run coverage locally. |
|
I agree to removing the badge. Better to have no badge than a wrong one. Hopefully we can fix this in the future and return it. |
|
I removed the codecov badge and also the upload from the unit test. Not really satisfactory, but let's do not waste more time on it. It works locally. |
|
Should we close this PR, since we are not using it anymore? |
|
I am not really ready to give up on codecov to be honest. Perhaps it should be revisited in the next maintenance period. |
|
Some progress, but still stuck.
Issue: coverage is zero and this is very puzzling. Running the following command locally gives me reasonable values (average of 70%; but this value is distorted as the application directory is not excluded): The exact same command in github actions results in zero coverage, see e.g., https://github.com/gammasim/simtools/actions/runs/6815126998/job/18533564276 (this doesn't use codeov, but simply prints the coverage to the terminal). I've done several cross checks, including installing the newest python 3.11 and cross checking the version of pytest and all plugins. |
|
Progress and this looks much more reasonable.
The final trick was that the github action did a |
|
@orelgueta - this is now ready to review again. Was a bit tricky, but ideal thing to do while listening to talks... The only thing I couldn't figure out is to exclude simtools/applications from the coverage testing. And obviously codecov complaints that we have -29% coverage. |
There was a problem hiding this comment.
The badge doesn't have the correct value when looking at the README, even after selecting this branch. However, on the codecov website it seems better. So I am approving on faith that once we merge it will actually work. Then we should still look at excluding the applications directory cause right now we are failing the basic quality requirement for no real reason.
|
I don't have admin access to edit the config yaml, could you maybe try this for ignoring the application path? |
Code coverage was shown incorrectly and pointed to the 'old' master branch. Changed it to main and the coverage seems to be correct.
For review, please open the readme of the branch and looked at the badge: https://github.com/gammasim/simtools/tree/code-coverage-badge#readme