improve coverage configuration #66
Conversation
There was a problem hiding this comment.
Hi! Thanks a lot for this PR.
I have asked a lot of questions, because I want to learn new things 🙂
Basically, this project was generated by our template: https://github.com/wemake-services/wemake-python-package Currently we have quite defined workflow.
So, in case we will adjust something here: we would need to adjust other projects as well.
I am open to improvements! 👍
| @@ -88,9 +88,25 @@ addopts = | |||
| # Why do we exclude this file from coverage? | |||
| # Because coverage is not calculated correctly for pytest plugins. | |||
| # And we completely test it anyway. | |||
| branch = True | |||
There was a problem hiding this comment.
We already have that here:
django-test-migrations/setup.cfg
Lines 82 to 83 in 686d03a
There was a problem hiding this comment.
ahh, sorry I didn't notice usage of this flags.
However I think it's more intuitive to set all coverage related settings in setup.cfg coverage section and use --cov* only if something cannot be configured by using coverage configuration (like for instance --cov or --cov-report).
I know pytest-cov has something like below in its docs:
Note that this plugin controls some options and setting the option in the config file will have no effect. These include specifying source to be measured (source option) and all data file handling (data_file and parallel options).
But in most cases --cov* flags will override coverage settings only if they are actually used.
| omit = | ||
| django_test_migrations/contrib/pytest_plugin.py | ||
|
|
||
| [coverage:report] | ||
| skip_covered = True |
There was a problem hiding this comment.
The same is configured here:
django-test-migrations/setup.cfg
Line 81 in 686d03a
| show_missing = True | ||
| sort = Cover | ||
|
|
||
| [coverage:xml] |
There was a problem hiding this comment.
I guess that this can be added like --cov-report=xml
There was a problem hiding this comment.
yes, but this will also create xml and here I just wanted to set output option so if developers want to create xml report, they can add --cov-term=xml and xml reported will be saved under file path set in output setting
| @@ -32,7 +32,7 @@ script: | |||
|
|
|||
| after_success: | |||
| - pip install coveralls | |||
| - coveralls | |||
| - coveralls --rcfile=setup.cfg | |||
There was a problem hiding this comment.
Can you please explain why is this required? It seemed to me like it was working before quite well.
There was a problem hiding this comment.
I prefer to be explicit if I am not using default values and default value for coverage rc file is .coveragerc (however in reality it just search for this files by checking several ones).
I can remove it if you want
| @@ -199,3 +199,4 @@ fabric.properties | |||
| ### Custom ### | |||
| ex.py | |||
| *.sqlite3 | |||
| .tests_reports/ | |||
There was a problem hiding this comment.
What is the reason behind this change?
There was a problem hiding this comment.
The only reason is to not clutter project's root with coverage reports files: .coverage, htmlcov/, coverage.xml or coverage.json (by default we are generating 2 reports - default one and html).
Sometimes (but not in our case) it's also useful to have dedicated directory for tests reports, because there is more types of reports (not only coverage) or there is multiple coverage's reports and it has to be merged etc.
Use dedicated directory - `.tests_reports/` - for all tests reports, including `coverage` html, xml and json reports. Update `coverage` configuration to: + measure `branch` coverage, refer to documentation: https://coverage.readthedocs.io/en/coverage-5.0.4/branch.html + show only not covered files and lines + sort report's results by coverage value
skarzi
force-pushed
the
feature/improve-coverage-config
branch
from
686d03a
to
19cf789
Compare
|
Hi @skarzi! Thanks again for your proposal. But, I have decided not to include this PR into the project. There are several key points:
However, if you believe that this change is really helpful and has better DX, feel free to reopen this discussion at https://github.com/wemake-services/wemake-python-package This way we can change all our projects to look the same way. Best, |
|
Sure, I will double check if it's worth to change it and eventually add PR to https://github.com/wemake-services/wemake-python-package 👍 |
Use dedicated directory -
.tests_reports/- for all tests reports,including
coveragehtml, xml and json reports.Update
coverageconfiguration to:branchcoverage, refer to documentation:https://coverage.readthedocs.io/en/coverage-5.0.4/branch.html
I found following configuration more developers friendly because it provide more concise and helpful terminal report and all things from points above.