Add support for logging in collection-phase#3986
Conversation
| "--log-cli-level" | ||
| ) is not None or self._config.getini("log_cli") | ||
|
|
||
| @pytest.hookimpl(hookwrapper=True) |
There was a problem hiding this comment.
Try first could possibly make it more robust
There was a problem hiding this comment.
Done. Thx for the suggestion. Probably it makes sense to use tryfirst also for the other hookimpls in the logging plugin, right?
There was a problem hiding this comment.
@Thisch quite possible - i never did a formal review of hook orders proceed with caution ^^
| # so we can access the terminal reporter plugin. | ||
| self._setup_cli_logging() | ||
|
|
||
| # TODO write to file support needed? |
There was a problem hiding this comment.
@Thisch any reason not to? If there isn't a good reason, I would prefer this to go out feature-complete rather than need another iteration.
There was a problem hiding this comment.
Because adding support for log-to-file support is trickier. Simply copying the ctx manager that are used in pytest_runtestloop does not work.
There was a problem hiding this comment.
I have a working implementation now. It was necessary to remove the closing context manager.
The logging plugin does not output log messages generated during the collection-phase when live-logging is enabled. This fixes this. Fixes pytest-dev#3964
Codecov Report
@@ Coverage Diff @@
## features #3986 +/- ##
============================================
+ Coverage 94.5% 94.54% +0.04%
============================================
Files 107 107
Lines 23710 23721 +11
Branches 2353 2357 +4
============================================
+ Hits 22406 22427 +21
+ Misses 993 987 -6
+ Partials 311 307 -4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## features #3986 +/- ##
============================================
+ Coverage 94.5% 94.54% +0.04%
============================================
Files 107 107
Lines 23710 23734 +24
Branches 2353 2357 +4
============================================
+ Hits 22406 22440 +34
+ Misses 993 987 -6
+ Partials 311 307 -4
Continue to review full report at Codecov.
|
|
|
||
| if self.log_file_handler is not None: | ||
| with catching_logs(self.log_file_handler, level=self.log_file_level): | ||
| yield # run all the tests |
There was a problem hiding this comment.
this should be # perform collection 😁
There was a problem hiding this comment.
I'll simply remove this comment.
| self.log_file_handler, level=self.log_file_level | ||
| ): | ||
| yield # run all the tests | ||
| with catching_logs(self.log_file_handler, level=self.log_file_level): |
There was a problem hiding this comment.
So in the end is not really necessary to close log_file_handler explicitly?
There was a problem hiding this comment.
No, it is not necessary. An alternative to removing the closing ctx manager would be to create the log_file_handler object twice.
|
Great, thanks a lot for the work @Thisch! As far as I'm concerned this can be merged when CI passes. 👍 |
The logging plugin does not output log messages generated during the
collection-phase when live-logging is enabled. This fixes this.
Fixes #3964