Migrate from Tox to Github actions; add missing unit tests to pytest scope (#10)#14
Migrate from Tox to Github actions; add missing unit tests to pytest scope (#10)#14
Conversation
blms
changed the title
blms
force-pushed
the
chore/10-gh-actions
branch
from
07ced1d to
f2b7672
Compare
|
@blms we already have an organization codecov secret defined - should be able to use that, is it not picking it up? Test coverage can be addressed later, but would you make an issue and document the major sections where we're missing coverage? And then maybe we should adjust our minimum to not go below the current 86% but not yell at us for being below 95% |
rlskoeser
left a comment
rlskoeser
left a comment
There was a problem hiding this comment.
Suggesting a few small changes. Let's see if we can get codecov working with the existing token.
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| python: ["3.12"] |
There was a problem hiding this comment.
Don't we need more python versions here? We should test on all supported versions
There was a problem hiding this comment.
Should we assume it's working on all non-EOL python 3 versions (3.9+), or is there a more systematic way I should go about checking compatibility with pre-3.12 versions? I can check library requirements, and then, maybe check for breaking changes in each 3.10-3.12?
There was a problem hiding this comment.
Oh right, and 3.13 too!
There was a problem hiding this comment.
@rlskoeser I ran the unit tests using different python versions with venv:
- On 3.9, all tests error out due to an incompatibility here. Python documentation is a little unclear about how exactly this changed so I'll need to do further investigation if we want to fix.
- 3.10, 3.11, and 3.13 work fine. (I was getting weird coverage issues in 3.11 but before sending this comment I recreated the venv and now it works!)
Should we address the 3.9 incompatibilities, or set 3.10 as the minimum version? And is there anything else I should do to make sure the library is compatible?
There was a problem hiding this comment.
@blms let's drop python 3.9! Tests passing is enough for compatibility for now (barring something important not covered by unit tests; but worry about that later)
| - name: Upload test coverage to Codecov | ||
| uses: codecov/codecov-action@v3 |
There was a problem hiding this comment.
This is what Codecov says the task should look like - not sure what the indentation should be, please adjust
| - name: Upload test coverage to Codecov | |
| uses: codecov/codecov-action@v3 | |
| - name: Upload coverage reports to Codecov | |
| uses: codecov/codecov-action@v5 | |
| with: | |
| token: ${{ secrets.CODECOV_TOKEN }} | |
| slug: Princeton-CDH/neuxml |
There was a problem hiding this comment.
Once you add other python versions, let's pick a version and only run this once per build. I have an example of this somewhere if you want.
|
|
||
| - name: Install package with dependencies | ||
| run: | | ||
| pip install -e . |
There was a problem hiding this comment.
I think this line is unnecessary (redundant with the next line)
Co-authored-by: Rebecca Sutton Koeser <rlskoeser@users.noreply.github.com>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
2 participants
In this PR
Per #10:
pytest.iniQuestions