FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-40474: Updated coverage.yml to better report coverage stats by lrjball · Pull Request #19851 · python/cpython · GitHub

/ cpython Public

bpo-40474: Updated coverage.yml to better report coverage stats - #19851

Merged
brettcannon merged 6 commits into
python:masterfrom
lrjball:fix-issue-40474
May 28, 2020
Merged

bpo-40474: Updated coverage.yml to better report coverage stats#19851
brettcannon merged 6 commits into
python:masterfrom
lrjball:fix-issue-40474

Conversation

lrjball commented May 2, 2020
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Currently modules which are imported early are misreported in coverage. This has a fix which is documented in the dev guide, but the fix isn't being used in the CI. This PR adds in that fix.

https://bugs.python.org/issue40474

Currently modules which are imported early are misreported in coverage. This has a fix which is documented in the dev guide, but the fix isn't being used in the CI. This PR adds in that fix.

lrjball commented May 2, 2020

Copy link
Copy Markdown
Contributor Author

How do I add the 'skip news' label to this?

lrjball commented May 2, 2020

Copy link
Copy Markdown
Contributor Author

this seems to be working now, for example coverage for re is correctly showing as 93% when previously it was showing 60% with the global statements showing as uncovered. See https://codecov.io/gh/python/cpython/tree/64d521b5d34c25b83d0472608d1eab3a6334bf59/Lib

aeros requested a review from zware May 8, 2020 21:08
Comment thread .github/workflows/coverage.yml Outdated
source ./.venv/bin/activate
python -m pip install -U coverage
python -m test.pythoninfo
export PYTHONPATH=./.venv/lib/python3.9/site-packages/coverage/fullcoverage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks like it will break periodically, maybe we could find a way to avoid hardcoding the version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Agreed, although I couldn't find a simple alternative. I tried .../python*/... which does set the pythonpath to the right thing when echo'd, it doesn't seem to pick it up on the following command. If anyone can suggest a better alternative that would be great

Comment thread .github/workflows/coverage.yml Outdated
source ./.venv/bin/activate
python -m pip install -U coverage
python -m test.pythoninfo
export PYTHONPATH=./.venv/lib/python3.9/site-packages/coverage/fullcoverage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is how the python3.9 is built looking at https://github.com/python/cpython/blob/master/Lib/venv/__init__.py#L129-L131:

Suggested change
export PYTHONPATH=./.venv/lib/python3.9/site-packages/coverage/fullcoverage
export PYTHONPATH=./.venv/lib/$(./python -c "import sys; print('python%d.%d' % sys.version_info[:2])")/site-packages/coverage/fullcoverage

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That is a clever idea! Thinking about it, export PYTHONPATH=./.venv/lib/$(ls .venv/lib | grep python | head -n 1)/site-packages/coverage/fullcoverage would also work and might be a bit simpler as it doesn't depend on sys, it just gets the name of that python* directory.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Whatever works and is accepted by a core dev is fine by me, you can also use find .venv -name fullcoverage which may be more robust.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

yeah I like that one the best, I've updated it now

lrjball commented May 27, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner @brettcannon, looks like you have both added to the CI pipeline in the past. Do either of you have any suggestions /opinions on this update to the test coverage calculation? Thanks

brettcannon merged commit d9c1f19 into python:master May 28, 2020

Copy link
Copy Markdown

@brettcannon: Please replace # with GH- in the commit message next time. Thanks!

Copy link
Copy Markdown
Member

Thanks!

CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request May 30, 2020
* 'master' of github.com:python/cpython: (497 commits)
  bpo-40061: Fix a possible refleak in _asynciomodule.c (pythonGH-19748)
  bpo-40798: Generate a different message for already removed elements (pythonGH-20483)
  closes bpo-29017: Update the bindings for Qt information with PySide2 (pythonGH-20149)
  bpo-39885: Make IDLE context menu cut and copy work again (pythonGH-18951)
  bpo-29882: Add an efficient popcount method for integers (python#771)
  Further de-linting of zoneinfo module (python#20499)
  bpo-40780: Fix failure of _Py_dg_dtoa to remove trailing zeros (pythonGH-20435)
  Indicate that abs() method accept argument that implement __abs__(), just like call() method in the docs (pythonGH-20509)
  bpo-39040: Fix parsing of email mime headers with whitespace between encoded-words. (pythongh-17620)
  bpo-40784: Fix sqlite3 deterministic test (pythonGH-20448)
  bpo-30064: Properly skip unstable loop.sock_connect() racing test (pythonGH-20494)
  Note the output ordering of combinatoric functions (pythonGH-19732)
  bpo-40474: Updated coverage.yml to better report coverage stats (python#19851)
  bpo-40806: Clarify that itertools.product immediately consumes its inpt (pythonGH-20492)
  bpo-1294959: Try to clarify the meaning of platlibdir (pythonGH-20332)
  bpo-37878: PyThreadState_DeleteCurrent() was not removed (pythonGH-20489)
  bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time (pythonGH-20493)
  bpo-40755: Add missing multiset operations to Counter() (pythonGH-20339)
  bpo-25920: Remove socket.getaddrinfo() lock on macOS (pythonGH-20177)
  bpo-40275: Fix test.support.threading_helper (pythonGH-20488)
  ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL