-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[7431] Add --suppress-logger CLI option #10371
Conversation
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.
Thank you, indeed your test-cases cover the functionality.
Cannot merge this very useful PR, not closely involved with this project to take responsibility |
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.
Thanks @itxasos23!
Besides the comments, we also should add this new option to https://github.com/pytest-dev/pytest/blob/main/doc/en/how-to/logging.rst.
e9d8982
to
a0012ed
Compare
Added docs entry; I'd like a review on wording and clarity, if possible 🙏 |
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.
Thanks @itxasos23!
Looks good, thanks! |
@@ -297,6 +297,13 @@ def add_option_ini(option, dest, default=None, type=None, **kwargs): | |||
default=None, | |||
help="Auto-indent multiline messages passed to the logging module. Accepts true|on, false|off or an integer.", | |||
) | |||
group.addoption( |
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.
Why is the new option just a command line option and not an ini
option? I guess that it can't be specified in e.g. pytest.ini
, right?
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 is possible to use addopts
to set that for every invocation if one chooses, is that what you are asking?
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.
Yes, kind of. In the logging plugin we have the add_option_ini
function which is used for most parameters of the logging plugin. I think that whenever we don't use this function, we should explain in a comment why we don't use it. Even better would be an explanation in the docstring of pytest_addoption
in logging.py that explains if new options should be added as ini options, as cli options or as both.
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.
There's still time to change this, as 7.2.0 was not released yet. 😁
Issue
Users want to suppress logger by name when invoking pytest.
Proposed solution
Based off https://github.com/pytest-dev/pytest/pull/7873/files from @symonk
Closes #7431