Avoid changing global logging state in tests#1522
Open
matt-graham wants to merge 5 commits intomasterfrom
Open
Avoid changing global logging state in tests#1522matt-graham wants to merge 5 commits intomasterfrom
matt-graham wants to merge 5 commits intomasterfrom
Conversation
Collaborator
|
@tamuri can we get this PR in? I'm sure its all complete |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1521
Removes
tlo.logging.core.resetfunction (which was only used in tests intests/test_logging.py) as this resets logging state to initial state on first import oftlo.logging.corewhich we are unlikely want to do in practice, as this overwrites any changes to 'global' (tlo.logging.coremodule level) logging state resulting as side effects of subsequent module imports - for example the very common pattern oflogger = getLogger(__name__).A new context manager
restore_global_stateis instead provided to serve the purpose of allowing testing changes to the global logging state without this persisting across tests. This records the initial global logging state variables on entry in to the context manager and restores these to their original values on exit from the context manager. This also involves performing some resetting of attributes of new loggers created in context managed code, but we do not explicitly remove the corresponding logger objects in the global state of the base logging library as this is difficult to do robustly as the base loggers store references to each other and use placeholders for loggers higher in hierarchy which haven't been initialised at the point a child logger is initialised.To confine accesses to the logging global state to the
tlo.logging.coremodule, this PR also moves theget_custom_levelsfunction from thetlo.logging.helpersmodule totlo.logging.core, swaps the previous usage of_logging.root.manager.loggerDictto find all initialisedtlo.methodsloggers to instead use the global_loggersdictionary (which should contain instances oftlo*loggers). There was also an instance of iteration over a set in this function previously which has been changed to iteration over a list to avoid the ordering of iterations changing in each interpreter instance (unclear if this had any effect but thought I'd fix it just to be safe...).