Skip to content

gh-106690: Add a .coveragerc file to the CPython repository #8150

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

Merged
merged 11 commits into from
Jul 13, 2023

Conversation

ammaraskar
Copy link
Member

@ammaraskar ammaraskar commented Jul 7, 2018

This was requested by Terry Reedy, the coveragerc file is theirs. It ignores code blocks that don't really matter and raise false alarms in terms of missed coverage.

(Can be marked with skip issue and skip news as it pertains to the build process)

@ammaraskar ammaraskar changed the title Add a coveragerc file that ignores common untaken paths Add a coveragerc file that ignores code blocks that don't matter for coverage Jul 7, 2018
@ammaraskar
Copy link
Member Author

ammaraskar commented Jul 7, 2018

@terryjreedy https://codecov.io/gh/python/cpython/pull/8150

This is probably a good place to start a discussion on if there's any other common patterns in the python code that should also be excluded.

Getting a review from someone knowledgeable with the build process like Brett or Zach would be good for this.

@ammaraskar
Copy link
Member Author

Also, branch = True caused some reliably reproducible test errors, will make bpo bugs for them later after investigating.

@zware
Copy link
Member

zware commented Sep 9, 2019

@ammaraskar Can you rebase this please? I think this is good to merge.

@ammaraskar
Copy link
Member Author

@zware Sorry for the late response. I needed some time to context-switch back to this, it didn't work last time I tried and I believe it's because codecov itself needs to be aware of the .coveragerc file and can't use it if it's not present in the root of the project. It seems to work at a quick glance:

@csabella
Copy link
Contributor

@zware, do you want to take another look before merging? Thanks!

@csabella csabella requested a review from zware January 12, 2020 13:48
@csabella csabella requested review from zware and removed request for zware February 3, 2020 12:22
@zware
Copy link
Member

zware commented Feb 3, 2020

I've not had a chance to swap this back into my brain lately and don't foresee such an opportunity in the near future. I'm in favor of adding a .coveragerc even if it's not complete, to be iterated on as we go; if @terryjreedy is good with this one, so am I :)

@zware zware requested review from terryjreedy and removed request for zware February 3, 2020 16:26
@arhadthedev
Copy link
Member

I'm in favor of adding a .coveragerc even if it's not complete, to be iterated on as we go; if @terryjreedy is good with this one, so am I :)

Pinging @terryjreedy to resolve this PR stuck for five years total.

@terryjreedy
Copy link
Member

I requested this back when coverage was run with CI and, as I remember, it was proposed that decreased coverage should block merging. Without exclusion of code that cannot be covered by unittests, improvements can decrease 'coverage'.

While removal of CI coverage and withdrawal of the gateway proposal removed he urgency of this patch, the accuracy point remains true. My (biased) feeling is that this should still be added (with the comment I added).

It is unclear to me whether or when the addition would affect a particular coverage run. The way I run IDLE coverage, it needs to be in the directory that contains the repository. But others cannot, if needed, copy to another place without something in the repository to copy.

@terryjreedy terryjreedy self-assigned this Mar 6, 2023
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

Since CI is green without branch = True, let us see what happens now with it added.

EDIT: If codecov is not run even unofficially (not sure), this will not do any good. I am running tests sequentially on a fresh local build and will try to manually run with my .coveragerc.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@terryjreedy
Copy link
Member

@Yhg1s Should I backport this to 3.12? It is a new feature, but does not affect any code that does not run coverage or look at repository directory.

@terryjreedy
Copy link
Member

Tests / Docs failure are bogus due to the low PR number (8150). Reported on #106689.

@terryjreedy terryjreedy changed the title Add a coveragerc file that ignores code blocks that don't matter for coverage gh-106690: Add a .coveragerc file to the CPython repository Jul 12, 2023
@erlend-aasland
Copy link
Contributor

erlend-aasland commented Jul 12, 2023

@terryjreedy, sorry for intruding, but I took the liberty to rename the NEWS file; it used the PR number, not the issue number, in its file name.

@terryjreedy
Copy link
Member

@erlend-aasland Intrude? Thank you for the fix. I just opened the issue to enable that.

@terryjreedy
Copy link
Member

terryjreedy commented Jul 13, 2023

Bogus failure in required test: test_threading changed environment: rerunning.
https://github.com/python/cpython/actions/runs/5536029870/jobs/10103154036?pr=8150

@terryjreedy terryjreedy merged commit 2f3ee02 into python:main Jul 13, 2023
kgdiem pushed a commit to kgdiem/cpython that referenced this pull request Jul 14, 2023
…thon#8150)

The added file is the coverage default at some point in time + checking branches both ways + IDLE additions, labelled as such and somewhat designed to be unlikely to affect other files.  Located in the CPython repository directory, it can be used where it is or copied elsewhere, depending on how one runs coverage.
---------

Co-authored-by: Terry Jan Reedy <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
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.