Add Coverage workflow #623
Conversation
|
if it's a pain to run PyPy tests under coverage, I think it would be fine to do the pypy tests as a separate CI job that aren't run under coverage. None of our code is currently PyPy-specific (we had a workaround for a PyPy bug for a while but it was a tiny branch of code). |
|
Could you fix the pre-commit failures? |
|
According to https://docs.github.com/actions/how-tos/security-for-github-actions/security-guides/automatic-token-authentication, a PR from a fork can never have write access (expect the repo itself). A relevant bit:
Maintainers note from the action on the error: https://github.com/marocchino/sticky-pull-request-comment/tree/v2/#error-resource-not-accessible-by-integration
The alternative is to provide a |
|
@JelleZijlstra I think we need your decision / help here to finish this PR. |
| contents: read | ||
| pull-requests: write | ||
|
|
||
| if: ${{ always() }} |
There was a problem hiding this comment.
Why do we need this line?
There was a problem hiding this comment.
A matter of choice, I thought it might be interesting to always get the coverage output comment even if the tests failed.
The brackets {{ might not be needed on a second look. On !cancelled might even be better.
What are you thoughts? I will adjust if you feel differently (i.e. only on success).
|
Read and write permissions for workflows are already enabled, so this might work once we merge it into main? |
There was a problem hiding this comment.
Read and write permissions for workflows are already enabled, so this might work once we merge it into main?
I had the same thought as well. But couldn't find a definite answer yet.
On my fork I did made a PR to a branch with the coverage workflow already in it: Daraan#1 (comment) there it commented like expected on the PR.
However, if I add a PR too my main branch Daraan#2 it also comments -might be because it is my own PR 🤔?
If: Settings > Actions > General > Workflow permissions is read & write. Merge and test with a dummy PR as follow up if it really works?
| contents: read | ||
| pull-requests: write | ||
|
|
||
| if: ${{ always() }} |
There was a problem hiding this comment.
A matter of choice, I thought it might be interesting to always get the coverage output comment even if the tests failed.
The brackets {{ might not be needed on a second look. On !cancelled might even be better.
What are you thoughts? I will adjust if you feel differently (i.e. only on success).
This reverts commit 6c43f31.
Added a coverage workflow (resolves: #520).
Currently the logic is to upload one coverage file from each version test.
pip install coverage&coverage -m unittestA crucial point about this workflow is that it installs and runs the test with
coverage -m unittestand notpython -m unittestanymore.coveragedoes (currently) not installtyping_extensionsso I think that is a safepip install(at the moment). I tried to add a weak test that assures thattyping_extensions.__file__is indeed the one insrc/and notsite-packages, but that test fails on the pypi installationDo you prefer any changes here? e.g. run first with
pythonwhen install and run again with coverage?Does anyone know why the pypy 3.9 and 3.10 tests fail here? Some kind of code leakage, missing monkeypatch?
EDIT: