-
Notifications
You must be signed in to change notification settings - Fork 214
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
Make sure the CLI precision is used when creating report. Fixes #674. #684
Conversation
src/pytest_cov/engine.py
Outdated
@@ -67,13 +67,16 @@ def _data_suffix(name): | |||
class CovController: | |||
"""Base class for different plugin implementations.""" | |||
|
|||
def __init__(self, cov_source, cov_report, cov_config, cov_append, cov_branch, config=None, nodeid=None): | |||
cov: coverage.Coverage | None |
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.
Perhaps move this into the initializer? Any reason to have it right here?
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.
It's just a type annotation - ":" doesn't assign anything to the "cov" class attribute. I added it mostly to make some things easier in my editor.
src/pytest_cov/engine.py
Outdated
def __init__(self, cov_source, cov_report, cov_config, cov_append, cov_branch, config=None, nodeid=None): | ||
cov: coverage.Coverage | None | ||
|
||
def __init__(self, cov_source, cov_report, cov_config, cov_append, cov_branch, cov_precision, config=None, nodeid=None): |
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.
It's probably best to make the new arg last + maybe kw-only so that it isn't a breaking change in the signature.
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.
Well now that you mention it, it does look odd doesn't it. Such a large number of positional always leads to some trouble. Funny thing, the config and nodeid are always passed. It was like that since the first commit where this was set to be a common lib for both pytest and nose: 7c6448d
…ore clarity. The config/nodeid were never meant to be optional for this internal class - the intention there was to signal that they can be None values.
30dcf1d
to
2e9d704
Compare
No description provided.