bpo-40275: Avoid importing logging in test.support#19601
Uh oh!
There was an error while loading. Please reload this page.
Conversation
* Import logging lazily in assertLogs() in unittest. * Move TestHandler from test.support to test_logging.
serhiy-storchaka commented Apr 19, 2020
It depends on #19600 because |
Lib/test/test_logging.py Outdated
| ]) | ||
| classTestHandler(logging.handlers.BufferingHandler): |
There was a problem hiding this comment.
Hm, I am not sure it's only test_logging use this common class forever.
There was a problem hiding this comment.
The whole purpose of placing TestHandler in test.support was that it could be used/useful in other tests. I understand if test.support needs to be subdivided further into subpackages, e.g. test.support.logging to hold this kind of code, but I don't think it's appropriate to move it into test_logging (even if it's the only current user of TestHandler) because it then wouldn't be obvious that it was intended for use across any tests that wanted to use logging.
I also see the microbenchmarks on the issue. but what is the practical impact on the time taken for test runs? How much does the total time for a test run reduce as a result of refactoring test.support to minimise imports?
bedevere-bot commented Apr 20, 2020
When you're done making the requested changes, leave the comment: |
serhiy-storchaka commented Apr 20, 2020
The goal of this change is not to save the import time, but to minimize possible side effects of importing the |
Lib/unittest/case.py Outdated
| logger=self.logger=logging.getLogger(self.logger_name) | ||
| formatter=logging.Formatter(self.LOGGING_FORMAT) | ||
| class_CapturingHandler(logging.Handler): |
There was a problem hiding this comment.
I dislike defining a new _CapturingHandler class each time _AssertLogsContext context manager is used :-( Maybe a lazy import could be used, something similar to PR #19600?
serhiy-storchakaforce-pushed the test-support-logging branch from f9b43f8 to 5d59d23Compare
vstinner left a comment
vstinner left a comment There was a problem hiding this comment.
Maybe moving the logging changes into a separated PR since test_logging/test.support changes and unittest changes are not directly related.
Uh oh!
There was an error while loading. Please reload this page.
| @@ -0,0 +1,29 @@ | |||
| importlogging.handlers | |||
| classTestHandler(logging.handlers.BufferingHandler): | |||
There was a problem hiding this comment.
It's only used in test_logging. Why not simply moving this class into test_logging?
We don't provide any backward compatibility warranty for test.support. It should not be used outside CPython code base.
There was a problem hiding this comment.
It is what I did originally. But then move it into a separate submodule on the request of @vsajip (logging maintainer, who added this class 10 years ago).
vstinner commented Apr 20, 2020
@vsajip: "The whole purpose of placing TestHandler in test.support was that it could be used/useful in other tests" The thing is that 10 years later, it's only used by test_logging. If tomorrow, another test wants to use it, support.logging_helper could be added. But I don't think that it's worth it. I would prefer to move TestHandler in test_logging to be able to remove "import logging" from test.support. @vsajip: "I also see the microbenchmarks on the issue. but what is the practical impact on the time taken for test runs?" |
vstinner left a comment
vstinner left a comment There was a problem hiding this comment.
Since there is a disagreement on test.support, maybe keep this PR for the unittest module, and write a new one for test.support changes?
Lib/unittest/_log.py Outdated
| self._raiseFailure( | ||
| "no logs of level{} or higher triggered on{}" | ||
| .format(logging.getLevelName(self.level), self.logger.name)) | ||
There was a problem hiding this comment.
Please remove empty lines at the end.
* 'master' of github.com:python/cpython: (2949 commits) Add files in tests/test_peg_generator to the install target lists (pythonGH-19723) bpo-40398: Fix typing.get_args() for special generic aliases. (pythonGH-19720) bpo-40348: Fix typos in the programming FAQ (pythonGH-19729) bpo-38387: Formally document PyDoc_STRVAR and PyDoc_STR macros (pythonGH-16607) bpo-40401: Remove duplicate pyhash.h include from pythoncore.vcxproj (pythonGH-19725) bpo-40387: Improve queue join() example. (pythonGH-19724) bpo-40396: Support GenericAlias in the typing functions. (pythonGH-19718) Fix typo in Lib/typing.py (pythonGH-19717) Fix typo in object.__format__ docs (pythonGH-19504) bpo-40275: Avoid importing logging in test.support (pythonGH-19601) bpo-40275: Avoid importing socket in test.support (pythonGH-19603) bpo-40275: Avoid importing asyncio in test.support (pythonGH-19600) bpo-40279: Add some error-handling to the module initialisation docs example (pythonGH-19705) closes bpo-40385: Remove Tools/scripts/checkpyc.py (pythonGH-19709) bpo-40334: Add What's New sections for PEP 617 and PEP 585 (pythonGH-19704) bpo-40340: Separate examples more clearly in the programming FAQ (pythonGH-19688) bpo-40360: Deprecate lib2to3 module in light of PEP 617 (pythonGH-19663) bpo-40334: Rewrite test_c_parser to avoid memory leaks (pythonGH-19694) bpo-38061: subprocess uses closefrom() on FreeBSD (pythonGH-19697) bpo-38061: os.closerange() uses closefrom() on FreeBSD (pythonGH-19696) ...
6 participants
logginglazily inassertLogs()inunittest.TestHandlerfromtest.supporttotest_logging.https://bugs.python.org/issue40275