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

Generate coverage reports using only GitHub Actions, without codecov.io #447

Merged
merged 17 commits into from Nov 6, 2022
Merged
Show file tree
Hide file tree
Changes from 11 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
2 changes: 0 additions & 2 deletions .github/CODEOWNERS

This file was deleted.

80 changes: 80 additions & 0 deletions .github/scripts/pr_comment.js
@@ -0,0 +1,80 @@
/*
Have a single comment on a PR, identified by a comment marker.

Create a new comment if no comment already exists.
Update the content of the existing comment.


https://octokit.github.io/rest.js/v19
*/
module.exports = async ({github, context, process}) => {
var octokit_rest = github
if (context.eventName != "pull_request") {
// Only PR are supported.
return
}

var sleep = function(second) {
return new Promise(resolve => setTimeout(resolve, second * 1000))
}

/*
Perform the actual logic.

This is wrapped so that we can retry on errors.
*/
var doAction = async function() {

console.log(context)

fs = require('fs');

const body = fs.readFileSync(
process.env.GITHUB_WORKSPACE + "/" + process.env.COMMENT_BODY, 'utf8');
var comment_id = null
var comment_marker = '\n' + process.env.COMMENT_MARKER
var comment_body = body + comment_marker
adiroiban marked this conversation as resolved.
Show resolved Hide resolved

var comments = await octokit_rest.issues.listComments({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.number,
})

console.log(comments)

comments.data.forEach(comment => {
if (comment.body.endsWith(comment_marker)) {
comment_id = comment.id
}
})

if (comment_id) {
// We have an existing comment.
// update the content.
await octokit_rest.issues.updateComment({
owner: context.repo.owner,
repo: context.repo.repo,
comment_id: comment_id,
body: comment_body,
})
return
}

// Create a new comment.
await octokit_rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.number,
body: comment_body,
})

}

try {
await doAction()
} catch (e) {
await sleep(5)
await doAction()
}
}
82 changes: 81 additions & 1 deletion .github/workflows/ci.yml
Expand Up @@ -96,8 +96,21 @@ jobs:

- run: nox --python ${{ matrix.python.action }} -e ${{ matrix.task.nox }} -- --use-wheel dist/*.whl

- name: Store coverage file
uses: actions/upload-artifact@v3
with:
# We have a single artifact shared for each workflow run.
# in this way we should intefere with coverage reports from other PRs
# or previous runs for the same PR.
name: coverage-${{ github.run_id }}
# Is important to keep the unique names for the coverage files
# so that when uploaded to the same artifact, they don't overlap.
path: .coverage*
Copy link
Member

Choose a reason for hiding this comment

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

See comments in duplicated section below.

Side note... I really like cross-platform matrices to avoid this. :]

Copy link
Member Author

Choose a reason for hiding this comment

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

I am missing the comment.

I am happy to see less duplication, but I think is best to have it in separate PR.

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

This is copy pasted from the other similar section and contains the same typos that were present there and have been fixed. They should be fixed here as well.

But, I'm going to go ahead and trigger a run here, cancel it partially completed, rerun the cancelled jobs and see if my concern about using the github.run_id seems accurate.


- name: Codecov
run: |
ls -al .coverage*
coverage combine
codecov -n "GitHub Actions - ${{ matrix.task.name}} - ${{ matrix.os.name }} ${{ matrix.python.name }}"


Expand Down Expand Up @@ -137,8 +150,21 @@ jobs:

- run: nox --python ${{ matrix.python.action }} -e ${{ matrix.task.nox }} -- --use-wheel dist/*.whl

- name: Store coverage file
uses: actions/upload-artifact@v3
with:
# We have a single artifact shared for each workflow run.
# in this way we should not interfere with coverage reports from other PRs
# or previous runs for the same PR.
name: coverage-${{ github.run_id }}
# It is important to keep the unique names for the coverage files
# so that when uploaded to the same artifact, they don't overlap.
path: .coverage*

- name: Codecov
run: |
ls -al .coverage*
coverage combine
codecov -n "GitHub Actions - ${{ matrix.task.name}} - ${{ matrix.os.name }} ${{ matrix.python.name }}"

check:
Expand Down Expand Up @@ -232,6 +258,59 @@ jobs:
password: ${{ secrets.PYPI_TOKEN }}
verbose: true

coverage-report:
name: Coverage report
runs-on: ubuntu-latest
needs:
# We are waiting only for test jobs.
- test-linux
- test-windows
steps:
- uses: actions/checkout@v3
adiroiban marked this conversation as resolved.
Show resolved Hide resolved
with:
fetch-depth: 0

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install --upgrade coverage[toml] diff_cover

- name: Download coverage reports
uses: actions/download-artifact@v3
with:
name: coverage-${{ github.run_id }}
path: .

- name: Combine coverage
run: coverage combine

- name: Report coverage
run: |
coverage xml
coverage report --no-skip-covered --show-missing
# Wrap in
echo '```' > coverage-report.txt
coverage report --show-missing >> coverage-report.txt
diff-cover --compare-branch origin/trunk coverage.xml >> coverage-report.txt
echo '```' >> coverage-report.txt

# Use the generic JS script to call our custom script
# for sending a comment to a PR.
- uses: actions/github-script@v3
env:
COMMENT_MARKER: "<!--- automatic-coverage-report -->"
COMMENT_BODY: coverage-report.txt
with:
script: |
const script = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/pr_comment.js`)
// Only pass top level objects as GHA does dependecy injection.
await script({github, context, process})

- name: Enforce diff coverage
run: |
diff-cover --fail-under=100 --compare-branch origin/trunk coverage.xml


# This is a meta-job to simplify PR CI enforcement configuration in GitHub.
# Inside the GitHub config UI you only configure this job as required.
# All the extra requirements are defined "as code" as part of the `needs`
Expand All @@ -250,8 +329,9 @@ jobs:
- test-windows
- check
- pypi-publish
- coverage-report
steps:
- name: Require all successes
uses: re-actors/alls-green@3a2de129f0713010a71314c74e33c0e3ef90e696
uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe
with:
jobs: ${{ toJSON(needs) }}
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -11,6 +11,7 @@ dist/
venv/
htmlcov/
.coverage
coverage.xml
*~
*.lock
apidocs/
Expand Down
3 changes: 1 addition & 2 deletions noxfile.py
Expand Up @@ -36,9 +36,8 @@ def tests(session: nox.Session) -> None:
session.run("coverage", "run", "--module", "twisted.trial", *posargs)

if os.environ.get("CI") != "true":
# When running the test locally, show the coverage report.
session.notify("coverage_report")
else:
session.run("coverage", "combine")


@nox.session
Expand Down
8 changes: 6 additions & 2 deletions pyproject.toml
Expand Up @@ -91,7 +91,12 @@ branch = true
source = ["towncrier"]

[tool.coverage.paths]
source = ["src", ".nox/*/site-packages"]
source = [
"src",
"*.nox/*/site-packages",
# required until coverage 6.6.0 is used: https://github.com/nedbat/coveragepy/issues/991
"*.nox\\*\\*\\site-packages"
]

[tool.coverage.report]
show_missing = true
Expand All @@ -102,5 +107,4 @@ exclude_lines = [
]
omit = [
"src/towncrier/__main__.py",
"src/towncrier/test/*",
]
Empty file.