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

Fix failing CI #943

Merged
merged 3 commits into from Nov 29, 2022
Merged

Fix failing CI #943

merged 3 commits into from Nov 29, 2022

Conversation

di
Copy link
Sponsor Member

@di di commented Nov 29, 2022

CodeCov's services seem to be getting more and more unavailable, and are causing our tests to fail (e.g., https://github.com/pypa/twine/actions/runs/3577489389/jobs/6016720780).

This PR removes CodeCov entirely in favor of running coverage in CI and asserting on some coverage percentage, currently 97%.

Fixes #658. This includes #942 to get CI to pass, closes #942. Closes #936.

@di
Copy link
Sponsor Member Author

di commented Nov 29, 2022

Looks like no_color for rich is not getting picked up on Windows now for some reason?

>       assert capsys.readouterr().out == f"Checking {sdist}: PASSED\n"
E       AssertionError: assert 'Checking C:\....gz: PASSED\n' == 'Checking C:\....gz: PASSED\n'
E         Skipping 64 identical leading characters in diff, use -v to show
E         - wn\pytest-0\test_passes_markdown_descripti1\dist\test-package-0.0.1.tar.gz: PASSED
E         + wn\pytest-0\test_passes_markdown_descripti1\dist\test-package-0.0.1.tar.gz: PASSED
E         ?           ++++ ++++                                                   ++++   ++++ ++++  ++++

tests\test_check.py:270: AssertionError

@bhrutledge
Copy link
Contributor

Looks like no_color for rich is not getting picked up on Windows now for some reason?

Yeah, I started looking into this in #936, but haven't had the time to dig into it.

@di di requested a review from bhrutledge November 29, 2022 21:02
@bhrutledge bhrutledge changed the title Drop CodeCov Fix failing CI Nov 29, 2022
Copy link
Member

@sigmavirus24 sigmavirus24 left a comment

Choose a reason for hiding this comment

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

We still care about coverage and there are ways to display it in CI. But we can add it back eventually I guess

@sigmavirus24 sigmavirus24 merged commit 717ae3d into main Nov 29, 2022
@sigmavirus24 sigmavirus24 deleted the drop-codecov branch November 29, 2022 21:10
@di
Copy link
Sponsor Member Author

di commented Nov 29, 2022

We still care about coverage and there are ways to display it in CI. But we can add it back eventually I guess

The results are still displayed in CI: https://github.com/pypa/twine/actions/runs/3577925730/jobs/6017490264#step:6:110

Do you mean adding comments on PRs like CodeCov did?

@bhrutledge
Copy link
Contributor

The results are still displayed in CI: https://github.com/pypa/twine/actions/runs/3577925730/jobs/6017490264#step:6:110

As noted in #658 (comment) (and subsequent comments), I think it'd be nice to make the HTML report available.

@di
Copy link
Sponsor Member Author

di commented Nov 29, 2022

I think once nedbat/coveragepy#1418 is released, this would be as simple as:

- python -m coverage report -m --fail-under 97
+ python -m coverage report -m --fail-under 97 --format=markdown >> $GITHUB_STEP_SUMMARY

https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/

@webknjaz
Copy link
Member

CodeCov's services seem to be getting more and more unavailable, and are causing our tests to fail

FWIW the workflow used a long-deprecated version of the action (v1). The current v3 is usually working well.

@webknjaz
Copy link
Member

once nedbat/coveragepy#1418 is released

@di FYI, it is possible to implement this right now with coverage xml, irongut/CodeCoverageSummary action and a few lines of code. Here's how I did it: https://github.com/ansible-community/ansible-test-gh-action/blob/ce3e835/action.yml#L493-L511.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Codecov for coverage reporting
4 participants