Skip to content

Fix coverage checking - #8057

Merged
esc merged 7 commits into
numba:mainfrom
gmarkall:fix-coverage
May 18, 2022
Merged

esc merged 7 commits into
numba:mainfrom
gmarkall:fix-coverage

Conversation

@gmarkall

Copy link
Copy Markdown
Member

run_coverage.py seemed to be broken in a couple of ways - these updates allow me to run it locally again - for example, invoking

./run_coverage.py numba.cuda.tests

results in running the CUDA tests, with a coverage report produced in the htmlcov directory.

Comment thread run_coverage.py Outdated
Comment on lines +38 to +39
os.path.dirname(__file__),
'coverage.conf')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment thread coverage.conf Outdated
@@ -0,0 +1,15 @@
# configuration file used by run_coverage.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's already a .coveragerc

@sklam sklam added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels May 17, 2022
gmarkall added 3 commits May 18, 2022 12:37
Due to various slight oddities in `numba._runtests._main()`'s handling
of arguments, it's actually hard to make this script work robustly
across platforms and different invocation methods. Instead it would be
better to document how to use the `coverage` CLI tool, as is done in
practice by maintainers when investigating coverage.
@gmarkall

gmarkall commented May 18, 2022 •

Copy link
Copy Markdown
Member Author

@sklam thanks for the feedback. WIth testing across Linux and Windows, and different ways of invoking it, e.g.

python run_coverage.py
python ./run_coverage.py
./run_coverage.py
# etc.

I found there's a bunch of fiddly issues still to resolve that seem a bit pointless to plough much time into. Instead, I think it's better to document the workflow using the coverage CLI tool, so I've updated this PR accordingly.

  • Do you follow these exact steps?
  • Do you have any other coverage-related tips?

@sklam

sklam commented May 18, 2022 •

Copy link
Copy Markdown
Member

Yes, that's how I use it mostly. (referring to the added docs)

The only other tip is to only get a report on changed files when reviewing PRs to speed things up. I use something like:

#! /usr/bin/env ipython
changed_files = !git diff --name-only origin/main...
print("changed files...")
for f in changed_files:
    print("  ", f)
changed_files = ','.join(changed_files)
!coverage xml -o cov.xml --include=$changed_files

@sklam sklam removed the 4 - Waiting on author Waiting for author to respond to review label May 18, 2022

@sklam sklam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the patch!

@sklam sklam added the 5 - Ready to merge Review and testing done, is ready to merge label May 18, 2022
@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label May 18, 2022
@esc
esc merged commit 88b6949 into numba:main May 18, 2022
@esc

esc commented May 18, 2022

Copy link
Copy Markdown
Member

Thank you for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants