-
Notifications
You must be signed in to change notification settings - Fork 23
chore: bump python version used for codecov in CI #520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #520 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 2096 2096
Branches 347 291 -56
=========================================
Hits 2096 2096 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -69,7 +69,7 @@ jobs: | |||
run: | | |||
pytest --runslow | |||
- name: Upload coverage to codecov | |||
if: matrix.python-version == '3.8' | |||
if: matrix.python-version == '3.9' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the coverage only runs for 3.8 (and other versions run pytest
without it), so this needs to change in the two places above as well. Have a look at #428, possibly this update means we can also remove the typing_extensions
install there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this
- name: Test with pytest
if: matrix.python-version != '3.8'
run: |
pytest --runslow
stays the same, right? I am guessing the reason to only run the slow tests on py>3.8 had a reason beyond 3.8 being the oldest version supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think they should all change. The logic is:
- run pytest with coverage for a specific version (previously 3.8, now 3.9),
- run pytest without coverage for the other versions (previously !=3.8, now !=3.9),
- upload if on the right version (previously 3.8, now 3.9).
I'm not sure what the 3.9 CI actually uploaded now because it didn't seem to fail. That surprised me since it should not have generated a coverage report in the current state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right but then for #428 to be dropped we would need to drop 3.8 all together, because the CI will still be running that (why it currently fails)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, I see now it's unrelated to coverage
In #515 the coverage was observed to be underestimated due to the use of
xfail()
to handle the issue described in #476. This reduced coverage was not observed locally (withpython 3.9.7
). It happens because the CI still usespython 3.8
to report coverage. This python version will be eventually deprecated, and it makes sense to not use it for code coverage anymore.