Skip to content
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

Replace codecov with python-coverage-comment-action #1649

Merged
merged 1 commit into from
Mar 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# .github/workflows/coverage.yml
name: Post coverage comment

on:
workflow_run:
workflows: ["Tests"]
types:
- completed

jobs:
test:
name: Run tests & display coverage
runs-on: ubuntu-latest
if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success'
permissions:
# Gives the action the necessary permissions for publishing new
# comments in pull requests.
pull-requests: write
# Gives the action the necessary permissions for editing existing
# comments (to avoid publishing multiple comments in the same PR)
contents: write
# Gives the action the necessary permissions for looking up the
# workflow that launched this workflow, and download the related
# artifact that contains the comment to be published
actions: read
steps:
# DO NOT run actions/checkout here, for security reasons
# For details, refer to https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
- name: Post comment
uses: py-cov-action/python-coverage-comment-action@v3
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_PR_RUN_ID: ${{ github.event.workflow_run.id }}
54 changes: 48 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
python -m pip install --upgrade pip setuptools
- name: Install dependencies
run: |
python -m pip install coverage tox tox-py unittest-xml-reporting
python -m pip install coverage[toml] tox tox-py unittest-xml-reporting
- name: Run tox
run: |
python -m pip --version
Expand All @@ -35,12 +35,54 @@ jobs:
- name: Coverage reporting
run: |
coverage combine
coverage report -m
coverage xml
coverage html
Comment on lines -39 to -40
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why coverage xml and html files were produced. xml, I'd guess it's because codecov needs it (though I would have thought it used the .coverage file itself).
.html, I really have no idea. Of course, if this serves a purpose that I didn't see, please tell me, I'll be delighted to re-add it with a comment to explain why.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a legacy thing. (Don't ask 😅) Fine to get rid of it.

- name: Publish coverage results
uses: codecov/codecov-action@v4
coverage report --show-missing
mv .coverage .coverage.${{ matrix.python-version }}
Comment on lines 35 to +39
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, I tend to use the COVERAGE_FILE env var to control the name of the coverage file, but it changes the way coverage combine works, and I was afraid I'd have to change more things for this to work, so I opted to move myself the .coverage into its own name.

- name: Store coverage file
uses: actions/upload-artifact@v4
with:
name: coverage-${{ matrix.python-version }}
path: .coverage.${{ matrix.python-version }}

coverage:
name: Coverage
runs-on: ubuntu-latest
needs: tests
permissions:
# If the author is a maintainer, the permission level is set by the
# values below.
# `pull-requests: write` is needed for publishing new comments in pull
# requests.
# `contents: write` is needed for pushing data to the
# `python-coverage-comment-action` branch, and for editing existing
# comments (to avoid publishing multiple comments in the same PR)
# In case the pull request comes from a forked repository, the maximum
# permission level is read, so the permissions below won't be acted upon
# by GitHub.
# https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token
Comment on lines +51 to +61
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried being a bit verbose because it's important that readers understand the security implications on those lines.

If you prefer without the comments or with less comments, let me know.

pull-requests: write
contents: write
steps:
- uses: actions/checkout@v4

- uses: actions/download-artifact@v4
id: download
with:
pattern: coverage-*
merge-multiple: true

- name: Coverage comment
id: coverage_comment
uses: py-cov-action/python-coverage-comment-action@v3
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
MERGE_COVERAGE_FILES: true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't changed the action settings more than necessary. In particular, if you want your badge to appear green, you'll have to tweak MINIMUM_GREEN which by default is 100 (which might be a bit overkill). Your current coverage is 98% (👏) and will appear orange with the default settings.


- name: Store Pull Request comment to be posted
uses: actions/upload-artifact@v4
if: steps.coverage_comment.outputs.COMMENT_FILE_WRITTEN == 'true'
with:
name: python-coverage-comment-action
path: python-coverage-comment-action.txt

isort:
runs-on: ubuntu-latest
Expand Down
4 changes: 2 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ add dynamic ``QuerySet`` filtering from URL parameters.

Full documentation on `read the docs`_.

.. image:: https://codecov.io/gh/carltongibson/django-filter/branch/develop/graph/badge.svg
:target: https://codecov.io/gh/carltongibson/django-filter
.. image:: https://raw.githubusercontent.com/carltongibson/django-filter/python-coverage-comment-action-data/badge.svg
:target: https://github.com/carltongibson/django-filter/tree/python-coverage-comment-action-data

.. image:: https://badge.fury.io/py/django-filter.svg
:target: http://badge.fury.io/py/django-filter
Expand Down
5 changes: 0 additions & 5 deletions codecov.yml

This file was deleted.

3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,6 @@ known_first_party = ["django_filters"]

[tool.flit.module]
name = "django_filters"

[tool.coverage.run]
relative_files = true
Comment on lines +58 to +59
Copy link
Contributor Author

@ewjoachim ewjoachim Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this was added to pyproject.toml, I had to add the extra [toml] in the placeS where coverage is installed. It wouldn't be the case if we put the coverage config in a different file, but it feels right to put it here. Feel free to indicate if you'd like it another way.

2 changes: 1 addition & 1 deletion requirements/test-ci.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
markdown
django-crispy-forms

coverage
coverage[toml]
pytz
unittest-xml-reporting