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

feat: Integrate automated test coverage comments on pull requests #419

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

1cu
Copy link
Contributor

@1cu 1cu commented Feb 9, 2025

ℹ️ Description

This Pull Request enhances the automated test coverage reporting for the kleinanzeigen-bot project. It uses the pytest-cov package to provide detailed coverage metrics during testing as xml.

📋 Changes Summary

  • Coverage Reporting Configuration: Updated addopts in pytest.ini_options to include XML coverage reporting.
  • Workflow Enhancements: Modified the GitHub Actions workflow in .github/workflows/test-coverage.yml to utilize the new coverage reporting features.

⚙️ Type of Change

Select the type(s) of change(s) included in this pull request:

  • ✨ New feature (adds new functionality without breaking existing usage)

✅ Checklist

Before requesting a review, confirm the following:

  • I have reviewed my changes to ensure they meet the project's standards.
  • I have tested my changes and ensured that all tests pass (pdm run test).
  • I have verified that linting passes (pdm run lint).
  • I have run security scans and addressed any identified issues (pdm run audit).
  • I have updated documentation where necessary.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This commit implements the integration of the Python Coverage Comment Action into the GitHub workflow for the kleinanzeigen-bot project. This enhancement addresses the lack of automated test coverage reports on pull requests, making it easier for reviewers to assess the impact of proposed changes on code coverage.

Changes made:
- Modified the `addopts` in `pytest.ini_options` to include XML coverage reporting.

This enhancement is in response to issue Second-Hand-Friends#418.
@1cu 1cu marked this pull request as draft February 9, 2025 22:18
1cu added 13 commits February 10, 2025 08:08
- Removed obsolete coverage workflow file and configuration.
- Updated build workflow to run unit and integration tests with coverage.
- Added steps to store coverage files and combine them in a new merge-coverage job.
- Configured coverage settings in `pyproject.toml` to enable parallel runs and specify source files.
- Ensured coverage reports are generated and uploaded as artifacts for pull requests.
enh: set thresholds lower for coverage checks.


###########################################################
merge-coverage:
Copy link
Contributor

@sebthom sebthom Feb 10, 2025

Choose a reason for hiding this comment

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

I am not sure if that even is required. I would expect the coverage is the same in all jobs. maybe just use coverage from one of the jobs and control which job via a matrix parameter?

Copy link
Contributor Author

@1cu 1cu Feb 10, 2025

Choose a reason for hiding this comment

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

Yes, the coverage currently is the same - but we need to merge pdm utest and pdm itest or just run pdm test.

The merge of all isn't any more work than just itest and utest.

@@ -141,23 +141,36 @@ jobs:
run: pdm run lint


- name: Run unit tests
Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding " with coverage" to the title is not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a mess at this point and definitely needs cleanup.

# SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot
#
# .github/workflows/coverage.yml
name: Post coverage comment
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be a separate workflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well as you can see workflows aren't my expertise :-)

I followed this advisory to separate it into a new workflow: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

But I can't get it to trigger in this repo.

Copy link
Contributor

@sebthom sebthom Feb 10, 2025

Choose a reason for hiding this comment

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

We are not relying on pull_request_target and we set token permissions on job level so I don't think this setup is actually required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as far as I can recreate these issues on test repos the workflow_run is only triggered on the main branch. The workflow will not trigger on this PR as long as it isn't pushed to the main branch.

@github-actions github-actions bot added the enhancement New feature or request label Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants