From 204ece594c24c8ad532f57de54fd77b723f1ffe0 Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Sat, 22 Jan 2022 22:35:59 -0800 Subject: [PATCH 1/5] fix(logger): test generates logfile Currently test_copy_config_to_ext_loggers_clean_old_handlers creates a file called "logfile" --- tests/functional/test_logger_utils.py | 40 ++++++++++----------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/tests/functional/test_logger_utils.py b/tests/functional/test_logger_utils.py index 1317fefc6ab..8b802586389 100644 --- a/tests/functional/test_logger_utils.py +++ b/tests/functional/test_logger_utils.py @@ -30,9 +30,8 @@ class LogLevel(Enum): @pytest.fixture def logger(stdout, log_level): def _logger(): - logging.basicConfig(stream=stdout, level=log_level.NOTSET.value) - logger = logging.getLogger(name=service_name()) - return logger + logging.basicConfig(stream=stdout, level=log_level.INFO.value) + return logging.getLogger(name=service_name()) return _logger @@ -51,10 +50,7 @@ def service_name(): def test_copy_config_to_ext_loggers(stdout, logger, log_level): - - msg = "test message" - - # GIVEN a external logger and powertools logger initialized + # GIVEN an external logger and powertools logger initialized logger_1 = logger() logger_2 = logger() @@ -62,6 +58,7 @@ def test_copy_config_to_ext_loggers(stdout, logger, log_level): # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used utils.copy_config_to_registered_loggers(source_logger=powertools_logger) + msg = "test message1" logger_1.info(msg) logger_2.info(msg) logs = capture_multiple_logging_statements_output(stdout) @@ -77,15 +74,13 @@ def test_copy_config_to_ext_loggers(stdout, logger, log_level): def test_copy_config_to_ext_loggers_include(stdout, logger, log_level): - - msg = "test message" - - # GIVEN a external logger and powertools logger initialized + # GIVEN an external logger and powertools logger initialized logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={logger.name}) + msg = "test message2" logger.info(msg) log = capture_logging_output(stdout) @@ -99,8 +94,7 @@ def test_copy_config_to_ext_loggers_include(stdout, logger, log_level): def test_copy_config_to_ext_loggers_wrong_include(stdout, logger, log_level): - - # GIVEN a external logger and powertools logger initialized + # GIVEN an external logger and powertools logger initialized logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) @@ -112,8 +106,7 @@ def test_copy_config_to_ext_loggers_wrong_include(stdout, logger, log_level): def test_copy_config_to_ext_loggers_exclude(stdout, logger, log_level): - - # GIVEN a external logger and powertools logger initialized + # GIVEN an external logger and powertools logger initialized logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) @@ -125,10 +118,7 @@ def test_copy_config_to_ext_loggers_exclude(stdout, logger, log_level): def test_copy_config_to_ext_loggers_include_exclude(stdout, logger, log_level): - - msg = "test message" - - # GIVEN a external logger and powertools logger initialized + # GIVEN an external logger and powertools logger initialized logger_1 = logger() logger_2 = logger() @@ -138,6 +128,7 @@ def test_copy_config_to_ext_loggers_include_exclude(stdout, logger, log_level): utils.copy_config_to_registered_loggers( source_logger=powertools_logger, include={logger_1.name, logger_2.name}, exclude={logger_1.name} ) + msg = "test message3" logger_2.info(msg) log = capture_logging_output(stdout) @@ -152,10 +143,9 @@ def test_copy_config_to_ext_loggers_include_exclude(stdout, logger, log_level): def test_copy_config_to_ext_loggers_clean_old_handlers(stdout, logger, log_level): - - # GIVEN a external logger with handler and powertools logger initialized + # GIVEN an external logger with handler and powertools logger initialized logger = logger() - handler = logging.FileHandler("logfile") + handler = logging.NullHandler() logger.addHandler(handler) powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) @@ -169,10 +159,7 @@ def test_copy_config_to_ext_loggers_clean_old_handlers(stdout, logger, log_level def test_copy_config_to_ext_loggers_custom_log_level(stdout, logger, log_level): - - msg = "test message" - - # GIVEN a external logger and powertools logger initialized + # GIVEN an external logger and powertools logger initialized logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.CRITICAL.value, stream=stdout) level = log_level.WARNING.name @@ -180,6 +167,7 @@ def test_copy_config_to_ext_loggers_custom_log_level(stdout, logger, log_level): # WHEN configuration copied from powertools logger to ALL external loggers # AND our external logger used with custom log_level utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={logger.name}, log_level=level) + msg = "test message4" logger.warning(msg) log = capture_logging_output(stdout) From 5bddf705fc929d9964452aef12f2b64466df500b Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Tue, 1 Feb 2022 20:40:12 +0000 Subject: [PATCH 2/5] docs: add better BDD coments --- tests/functional/test_logger_utils.py | 37 +++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/tests/functional/test_logger_utils.py b/tests/functional/test_logger_utils.py index 8b802586389..bf6d7d4ed5b 100644 --- a/tests/functional/test_logger_utils.py +++ b/tests/functional/test_logger_utils.py @@ -50,20 +50,21 @@ def service_name(): def test_copy_config_to_ext_loggers(stdout, logger, log_level): - # GIVEN an external logger and powertools logger initialized + # GIVEN two external loggers and powertools logger initialized logger_1 = logger() logger_2 = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used + # WHEN configuration copied from powertools logger to ALL external loggers + # AND external loggers used utils.copy_config_to_registered_loggers(source_logger=powertools_logger) msg = "test message1" logger_1.info(msg) logger_2.info(msg) logs = capture_multiple_logging_statements_output(stdout) - # THEN + # THEN all external loggers used Powertools handler, formatter and log level for index, logger in enumerate([logger_1, logger_2]): assert len(logger.handlers) == 1 assert type(logger.handlers[0]) is logging.StreamHandler @@ -78,13 +79,14 @@ def test_copy_config_to_ext_loggers_include(stdout, logger, log_level): logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used + # WHEN configuration copied from powertools logger to INCLUDED external loggers + # AND our external logger used utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={logger.name}) msg = "test message2" logger.info(msg) log = capture_logging_output(stdout) - # THEN + # THEN included external loggers used Powertools handler, formatter and log level. assert len(logger.handlers) == 1 assert type(logger.handlers[0]) is logging.StreamHandler assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter @@ -98,10 +100,10 @@ def test_copy_config_to_ext_loggers_wrong_include(stdout, logger, log_level): logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used + # THEN included external loggers used Powertools handler, formatter and log level. utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={"non-existing-logger"}) - # THEN + # THEN existing external logger is not modified assert not logger.handlers @@ -110,21 +112,22 @@ def test_copy_config_to_ext_loggers_exclude(stdout, logger, log_level): logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used + # WHEN configuration copied from powertools logger to ALL BUT external logger utils.copy_config_to_registered_loggers(source_logger=powertools_logger, exclude={logger.name}) - # THEN + # THEN external logger is not modified assert not logger.handlers def test_copy_config_to_ext_loggers_include_exclude(stdout, logger, log_level): - # GIVEN an external logger and powertools logger initialized + # GIVEN two external loggers and powertools logger initialized logger_1 = logger() logger_2 = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used + # WHEN configuration copied from powertools logger to INCLUDED external loggers + # AND external logger_1 is also in EXCLUDE list utils.copy_config_to_registered_loggers( source_logger=powertools_logger, include={logger_1.name, logger_2.name}, exclude={logger_1.name} ) @@ -132,7 +135,7 @@ def test_copy_config_to_ext_loggers_include_exclude(stdout, logger, log_level): logger_2.info(msg) log = capture_logging_output(stdout) - # THEN + # THEN logger_1 is not modified and Logger_2 used Powertools handler, formatter and log level assert not logger_1.handlers assert len(logger_2.handlers) == 1 assert type(logger_2.handlers[0]) is logging.StreamHandler @@ -149,10 +152,10 @@ def test_copy_config_to_ext_loggers_clean_old_handlers(stdout, logger, log_level logger.addHandler(handler) powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to ALL external loggers AND our external logger used + # WHEN configuration copied from powertools logger to ALL external loggers utils.copy_config_to_registered_loggers(source_logger=powertools_logger) - # THEN + # THEN old logger's handler removed and Powertools configuration used instead assert len(logger.handlers) == 1 assert type(logger.handlers[0]) is logging.StreamHandler assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter @@ -164,14 +167,14 @@ def test_copy_config_to_ext_loggers_custom_log_level(stdout, logger, log_level): powertools_logger = Logger(service=service_name(), level=log_level.CRITICAL.value, stream=stdout) level = log_level.WARNING.name - # WHEN configuration copied from powertools logger to ALL external loggers - # AND our external logger used with custom log_level + # WHEN configuration copied from powertools logger to INCLUDED external logger + # AND external logger used with custom log_level utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={logger.name}, log_level=level) msg = "test message4" logger.warning(msg) log = capture_logging_output(stdout) - # THEN + # THEN external logger used Powertools handler, formatter and CUSTOM log level. assert len(logger.handlers) == 1 assert type(logger.handlers[0]) is logging.StreamHandler assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter From 26c50df9ece3c1c1b90b226d0e9e7d29ccb0761b Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Tue, 1 Feb 2022 20:48:45 +0000 Subject: [PATCH 3/5] chore: use isinstance over type --- tests/functional/test_logger_utils.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/functional/test_logger_utils.py b/tests/functional/test_logger_utils.py index bf6d7d4ed5b..abf816b5f18 100644 --- a/tests/functional/test_logger_utils.py +++ b/tests/functional/test_logger_utils.py @@ -67,8 +67,8 @@ def test_copy_config_to_ext_loggers(stdout, logger, log_level): # THEN all external loggers used Powertools handler, formatter and log level for index, logger in enumerate([logger_1, logger_2]): assert len(logger.handlers) == 1 - assert type(logger.handlers[0]) is logging.StreamHandler - assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter + assert isinstance(logger.handlers[0], logging.StreamHandler) + assert isinstance(logger.handlers[0].formatter, formatter.LambdaPowertoolsFormatter) assert logger.level == log_level.INFO.value assert logs[index]["message"] == msg assert logs[index]["level"] == log_level.INFO.name @@ -88,8 +88,8 @@ def test_copy_config_to_ext_loggers_include(stdout, logger, log_level): # THEN included external loggers used Powertools handler, formatter and log level. assert len(logger.handlers) == 1 - assert type(logger.handlers[0]) is logging.StreamHandler - assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter + assert isinstance(logger.handlers[0], logging.StreamHandler) + assert isinstance(logger.handlers[0].formatter, formatter.LambdaPowertoolsFormatter) assert logger.level == log_level.INFO.value assert log["message"] == msg assert log["level"] == log_level.INFO.name @@ -138,8 +138,8 @@ def test_copy_config_to_ext_loggers_include_exclude(stdout, logger, log_level): # THEN logger_1 is not modified and Logger_2 used Powertools handler, formatter and log level assert not logger_1.handlers assert len(logger_2.handlers) == 1 - assert type(logger_2.handlers[0]) is logging.StreamHandler - assert type(logger_2.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter + assert isinstance(logger_2.handlers[0], logging.StreamHandler) + assert isinstance(logger_2.handlers[0].formatter, formatter.LambdaPowertoolsFormatter) assert logger_2.level == log_level.INFO.value assert log["message"] == msg assert log["level"] == log_level.INFO.name @@ -157,8 +157,8 @@ def test_copy_config_to_ext_loggers_clean_old_handlers(stdout, logger, log_level # THEN old logger's handler removed and Powertools configuration used instead assert len(logger.handlers) == 1 - assert type(logger.handlers[0]) is logging.StreamHandler - assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter + assert isinstance(logger.handlers[0], logging.StreamHandler) + assert isinstance(logger.handlers[0].formatter, formatter.LambdaPowertoolsFormatter) def test_copy_config_to_ext_loggers_custom_log_level(stdout, logger, log_level): @@ -176,8 +176,8 @@ def test_copy_config_to_ext_loggers_custom_log_level(stdout, logger, log_level): # THEN external logger used Powertools handler, formatter and CUSTOM log level. assert len(logger.handlers) == 1 - assert type(logger.handlers[0]) is logging.StreamHandler - assert type(logger.handlers[0].formatter) is formatter.LambdaPowertoolsFormatter + assert isinstance(logger.handlers[0], logging.StreamHandler) + assert isinstance(logger.handlers[0].formatter, formatter.LambdaPowertoolsFormatter) assert powertools_logger.level == log_level.CRITICAL.value assert logger.level == log_level.WARNING.value assert log["message"] == msg From f4473d3761b07d8836b29a067bc345cfdfa0e57b Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Wed, 2 Feb 2022 02:35:21 +0000 Subject: [PATCH 4/5] chore: correct docs --- tests/functional/test_logger_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_logger_utils.py b/tests/functional/test_logger_utils.py index abf816b5f18..cd6abd11d15 100644 --- a/tests/functional/test_logger_utils.py +++ b/tests/functional/test_logger_utils.py @@ -100,7 +100,7 @@ def test_copy_config_to_ext_loggers_wrong_include(stdout, logger, log_level): logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # THEN included external loggers used Powertools handler, formatter and log level. + # WHEN configuration copied from powertools logger to INCLUDED NON EXISTING utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={"non-existing-logger"}) # THEN existing external logger is not modified From 5ea99b33d5728def37b4424184b359efec419cf5 Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Wed, 2 Feb 2022 02:43:04 +0000 Subject: [PATCH 5/5] chore: correct docs --- tests/functional/test_logger_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_logger_utils.py b/tests/functional/test_logger_utils.py index cd6abd11d15..85f60db97f8 100644 --- a/tests/functional/test_logger_utils.py +++ b/tests/functional/test_logger_utils.py @@ -100,7 +100,7 @@ def test_copy_config_to_ext_loggers_wrong_include(stdout, logger, log_level): logger = logger() powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout) - # WHEN configuration copied from powertools logger to INCLUDED NON EXISTING + # WHEN configuration copied from powertools logger to INCLUDED NON EXISTING external loggers utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={"non-existing-logger"}) # THEN existing external logger is not modified