-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Additionally handle logstart and logfinish hooks #3189
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
Additionally handle logstart and logfinish hooks #3189
Conversation
e98ae1f to
4eccf4d
Compare
4eccf4d to
23c15f9
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.
Great work @s0undt3ch, thanks!
| with catching_logs(LogCaptureHandler(), | ||
| formatter=self.formatter, level=self.log_level) as log_handler: | ||
| if self.log_cli_handler: | ||
| self.log_cli_handler.set_when(when) |
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 would add the following code here and remove your if item checks:
if item is None:
yield
returnThere 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.
Good point.
|
Thx for working on this! |
a2369c3 to
c9123c6
Compare
|
@nicoddemus sorry, I force pushed the changes asked by @Thisch and lost your change log changes(forgot to pull first). |
No worries, just pushed my change again. 👍 |
|
Thanks! |
8abc7a4 to
a8ac48c
Compare
_pytest/logging.py
Outdated
|
|
||
| if not hasattr(item, 'catch_log_handlers'): | ||
| item.catch_log_handlers = {} | ||
|
|
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.
This newline and the next one are not really needed. Can you remove them please?
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.
Done
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.
Thx
a8ac48c to
ea06c13
Compare
Thanks for submitting a PR, your contribution is really appreciated!
Here's a quick checklist that should be present in PRs:
$issue_id.$typefor example (588.bugfix)removal,feature,bugfix,vendor,docortrivialbugfix,vendor,docortrivialfixes, targetmaster; for removals or features targetfeatures;Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please:
AUTHORS, in alphabetical order;