-
Notifications
You must be signed in to change notification settings - Fork 361
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
Add coverage reporting to tests #917
Conversation
- Make sure the ignores we have are relevant to the project. - Break up ignores into topical sections - Switch most directory ignores to relative and directory only rather than absolute but matching files or directories - Remove macOS and editor specific ignores (should be managed by the user)
469f3b2
to
84c6e81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -11,3 +11,7 @@ addopts = | |||
--durations 10 | |||
--durations-min 1 | |||
--strict-markers | |||
--cov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samdoran I also recommend adding an explicit -p pytest_cov
to make sure that this plugin is pre-loaded earlier (this is sometimes important) rather than later (on the plugin discovery stage).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another comment is that you should measure the coverage for both the project and the tests. For this, you'd need to use a few subsequent args: --cov=ansible_runner --cov=test/
. It may be really helpful to find out if some of the tests never run in CI, for example.
Here's what the coveragepy author posted last year: https://nedbatchelder.com/blog/202008/you_should_include_your_tests_in_coverage.html.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plus here's my example of the coveragepy config https://github.com/ansible/pylibssh/blob/devel/.coveragerc. Hope you'll find it useful (although, some parts are related to having C-extensions so you may not need those).
skip_install = True | ||
allow_external = /bin/sh | ||
commands = | ||
/bin/sh -c "rm -rf {toxinidir}/test/coverage/*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samdoran I have a few comments here:
- You could've used a combination of
git clean
commands which is easier to maintain, here's an example: https://github.com/ansible/pylibssh/blob/924771a/tox.ini#L369-L380. - Instead of calling sh/rm, you could do
{envpython} -c 'import shutil; shutil.rmdir("{toxinidir}/test/coverage/")'
and avoid having to useallow_externals
.
@@ -0,0 +1,16 @@ | |||
[run] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may need parallel = True
and branch = True
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
branch = True
, yes I should add that. parallel = True
is being set by pytest-cov
.
@@ -11,3 +11,7 @@ addopts = | |||
--durations 10 | |||
--durations-min 1 | |||
--strict-markers | |||
--cov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another comment is that you should measure the coverage for both the project and the tests. For this, you'd need to use a few subsequent args: --cov=ansible_runner --cov=test/
. It may be really helpful to find out if some of the tests never run in CI, for example.
Here's what the coveragepy author posted last year: https://nedbatchelder.com/blog/202008/you_should_include_your_tests_in_coverage.html.
Enable and configure test coverage.
Add a
tox
environment for cleaning up coverage reports and docs builds.