Commit 63a70056f01145c404b6886885a2f837f6d049a3
Committed by
Christian Herdtweck
1 parent
d3624f1d
log_helper: fix tests after refactoring code
Showing
3 changed files
with
81 additions
and
175 deletions
tests/util/log_helper/log_helper_test_imported.py
| @@ -12,7 +12,7 @@ WARNING_MESSAGE = 'imported: warning log' | @@ -12,7 +12,7 @@ WARNING_MESSAGE = 'imported: warning log' | ||
| 12 | ERROR_MESSAGE = 'imported: error log' | 12 | ERROR_MESSAGE = 'imported: error log' |
| 13 | CRITICAL_MESSAGE = 'imported: critical log' | 13 | CRITICAL_MESSAGE = 'imported: critical log' |
| 14 | 14 | ||
| 15 | -logger = log_helper.get_or_create_logger('test_imported', logging.ERROR) | 15 | +logger = log_helper.get_or_create_silent_logger('test_imported', logging.ERROR) |
| 16 | 16 | ||
| 17 | 17 | ||
| 18 | def log(): | 18 | def log(): |
tests/util/log_helper/log_helper_test_main.py
| 1 | """ Test log_helpers """ | 1 | """ Test log_helpers """ |
| 2 | 2 | ||
| 3 | import sys | 3 | import sys |
| 4 | -import logging | ||
| 5 | from tests.util.log_helper import log_helper_test_imported | 4 | from tests.util.log_helper import log_helper_test_imported |
| 6 | from oletools.util.log_helper import log_helper | 5 | from oletools.util.log_helper import log_helper |
| 7 | 6 | ||
| @@ -11,90 +10,48 @@ WARNING_MESSAGE = 'main: warning log' | @@ -11,90 +10,48 @@ WARNING_MESSAGE = 'main: warning log' | ||
| 11 | ERROR_MESSAGE = 'main: error log' | 10 | ERROR_MESSAGE = 'main: error log' |
| 12 | CRITICAL_MESSAGE = 'main: critical log' | 11 | CRITICAL_MESSAGE = 'main: critical log' |
| 13 | 12 | ||
| 14 | -logger = log_helper.get_or_create_logger('test_main') | 13 | +logger = log_helper.get_or_create_silent_logger('test_main') |
| 15 | 14 | ||
| 16 | 15 | ||
| 17 | -def init_logging_and_log(cmd_line_args=None): | ||
| 18 | - args = cmd_line_args if cmd_line_args else sys.argv | 16 | +def init_logging_and_log(args): |
| 17 | + """ | ||
| 18 | + Try to cover possible logging scenarios. For each scenario covered, here's the expected args and outcome: | ||
| 19 | + - Log without enabling: ['<level>'] | ||
| 20 | + * logging when being imported - should never print | ||
| 21 | + - Log as JSON without enabling: ['as-json', '<level>'] | ||
| 22 | + * logging as JSON when being imported - should never print | ||
| 23 | + - Enable and log: ['enable', '<level>'] | ||
| 24 | + * logging when being run as script - should log messages | ||
| 25 | + - Enable and log as JSON: ['as-json', 'enable', '<level>'] | ||
| 26 | + * logging as JSON when being run as script - should log messages as JSON | ||
| 27 | + - Enable, log as JSON and throw: ['enable', 'as-json', 'throw', '<level>'] | ||
| 28 | + * should produce JSON-compatible output, even after an unhandled exception | ||
| 29 | + """ | ||
| 19 | 30 | ||
| 20 | - if 'silent' in args: | ||
| 21 | - return _log_silently() | ||
| 22 | - elif 'dictionary' in args: | ||
| 23 | - return _log_dictionary(args) | ||
| 24 | - elif 'current_level' in args: | ||
| 25 | - _enable_logging() | ||
| 26 | - return _log_at_current_level() | ||
| 27 | - elif 'default' in args: | ||
| 28 | - _enable_logging() | ||
| 29 | - return _log(logger) | 31 | + # the level should always be the last argument passed |
| 32 | + level = args[-1] | ||
| 33 | + use_json = 'as-json' in args | ||
| 34 | + throw = 'throw' in args | ||
| 30 | 35 | ||
| 31 | - use_json = '-j' in args | ||
| 32 | - throw_exception = 'throw' in args | 36 | + if 'enable' in args: |
| 37 | + log_helper.enable_logging(use_json, level, stream=sys.stdout) | ||
| 33 | 38 | ||
| 34 | - level = _parse_log_level(args) | 39 | + _log() |
| 35 | 40 | ||
| 36 | - _enable_logging(use_json, level) | ||
| 37 | - _log(logger) | ||
| 38 | - log_helper_test_imported.log() | ||
| 39 | - | ||
| 40 | - if throw_exception: | 41 | + if throw: |
| 41 | raise Exception('An exception occurred before ending the logging') | 42 | raise Exception('An exception occurred before ending the logging') |
| 42 | 43 | ||
| 43 | - _end_logging() | ||
| 44 | - | ||
| 45 | - | ||
| 46 | -def _parse_log_level(args): | ||
| 47 | - if 'debug' in args: | ||
| 48 | - return 'debug' | ||
| 49 | - elif 'info' in args: | ||
| 50 | - return 'info' | ||
| 51 | - elif 'warning' in args: | ||
| 52 | - return 'warning' | ||
| 53 | - elif 'error' in args: | ||
| 54 | - return 'error' | ||
| 55 | - else: | ||
| 56 | - return 'critical' | ||
| 57 | - | ||
| 58 | - | ||
| 59 | -def _log_dictionary(args): | ||
| 60 | - level = _parse_log_level(args) | ||
| 61 | - _enable_logging(True, level) | ||
| 62 | - | ||
| 63 | - logger.log_at_current_level({ | ||
| 64 | - 'msg': DEBUG_MESSAGE | ||
| 65 | - }) | ||
| 66 | log_helper.end_logging() | 44 | log_helper.end_logging() |
| 67 | 45 | ||
| 68 | 46 | ||
| 69 | -def _enable_logging(use_json=False, level='warning'): | ||
| 70 | - log_helper.enable_logging(use_json, level, stream=sys.stdout) | ||
| 71 | - | ||
| 72 | - | ||
| 73 | -def _log_at_current_level(): | ||
| 74 | - logger.log_at_current_level(DEBUG_MESSAGE) | ||
| 75 | - | ||
| 76 | - | ||
| 77 | -def _log_silently(): | ||
| 78 | - silent_logger = log_helper.get_or_create_silent_logger('test_main_silent', logging.DEBUG - 1) | ||
| 79 | - _log(silent_logger) | ||
| 80 | - | ||
| 81 | - | ||
| 82 | -def _log(current_logger): | ||
| 83 | - current_logger.debug(DEBUG_MESSAGE) | ||
| 84 | - current_logger.info(INFO_MESSAGE) | ||
| 85 | - current_logger.warning(WARNING_MESSAGE) | ||
| 86 | - current_logger.error(ERROR_MESSAGE) | ||
| 87 | - current_logger.critical(CRITICAL_MESSAGE) | ||
| 88 | - | ||
| 89 | - | ||
| 90 | -def _end_logging(): | ||
| 91 | - log_helper.end_logging() | 47 | +def _log(): |
| 48 | + logger.debug(DEBUG_MESSAGE) | ||
| 49 | + logger.info(INFO_MESSAGE) | ||
| 50 | + logger.warning(WARNING_MESSAGE) | ||
| 51 | + logger.error(ERROR_MESSAGE) | ||
| 52 | + logger.critical(CRITICAL_MESSAGE) | ||
| 53 | + log_helper_test_imported.log() | ||
| 92 | 54 | ||
| 93 | 55 | ||
| 94 | if __name__ == '__main__': | 56 | if __name__ == '__main__': |
| 95 | - # since we are using subprocess, add delimiters so we can easily extract | ||
| 96 | - # the output that matters (when debugging tests we get extra output, | ||
| 97 | - # so we need to ignore it) | ||
| 98 | - print('<#') | ||
| 99 | - init_logging_and_log(sys.argv) | ||
| 100 | - print('#>') | 57 | + init_logging_and_log(sys.argv[1:]) |
tests/util/log_helper/test_log_helper.py
| @@ -5,22 +5,18 @@ Check if it handles imported modules correctly | @@ -5,22 +5,18 @@ Check if it handles imported modules correctly | ||
| 5 | and that the default silent logger won't log when nothing is enabled | 5 | and that the default silent logger won't log when nothing is enabled |
| 6 | """ | 6 | """ |
| 7 | 7 | ||
| 8 | -from __future__ import print_function | ||
| 9 | - | ||
| 10 | import unittest | 8 | import unittest |
| 11 | import sys | 9 | import sys |
| 12 | import json | 10 | import json |
| 13 | -import re | 11 | +import subprocess |
| 14 | from tests.util.log_helper import log_helper_test_main | 12 | from tests.util.log_helper import log_helper_test_main |
| 15 | from tests.util.log_helper import log_helper_test_imported | 13 | from tests.util.log_helper import log_helper_test_imported |
| 16 | from os.path import dirname, join, relpath, abspath | 14 | from os.path import dirname, join, relpath, abspath |
| 17 | -from subprocess import check_output, STDOUT, CalledProcessError | ||
| 18 | 15 | ||
| 19 | # this is the common base of "tests" and "oletools" dirs | 16 | # this is the common base of "tests" and "oletools" dirs |
| 20 | -ROOT_DIRECTORY = dirname(dirname(dirname(dirname(abspath(__file__))))) | 17 | +ROOT_DIRECTORY = abspath(join(__file__, '..', '..', '..', '..')) |
| 21 | TEST_FILE = relpath(join(dirname(__file__), 'log_helper_test_main.py'), ROOT_DIRECTORY) | 18 | TEST_FILE = relpath(join(dirname(__file__), 'log_helper_test_main.py'), ROOT_DIRECTORY) |
| 22 | PYTHON_EXECUTABLE = sys.executable | 19 | PYTHON_EXECUTABLE = sys.executable |
| 23 | -REGEX = re.compile('<#(.*)(:?#>|Traceback)', re.MULTILINE | re.DOTALL) | ||
| 24 | 20 | ||
| 25 | MAIN_LOG_MESSAGES = [ | 21 | MAIN_LOG_MESSAGES = [ |
| 26 | log_helper_test_main.DEBUG_MESSAGE, | 22 | log_helper_test_main.DEBUG_MESSAGE, |
| @@ -32,31 +28,21 @@ MAIN_LOG_MESSAGES = [ | @@ -32,31 +28,21 @@ MAIN_LOG_MESSAGES = [ | ||
| 32 | 28 | ||
| 33 | 29 | ||
| 34 | class TestLogHelper(unittest.TestCase): | 30 | class TestLogHelper(unittest.TestCase): |
| 35 | - def test_default_logging_to_stderr(self): | ||
| 36 | - """ | ||
| 37 | - Basic test for simple logging | ||
| 38 | - """ | ||
| 39 | - output = self._run_test(['default']) | ||
| 40 | - self.assertIn(log_helper_test_main.WARNING_MESSAGE, output) | 31 | + def test_it_doesnt_log_when_not_enabled(self): |
| 32 | + output = self._run_test(['debug']) | ||
| 33 | + self.assertTrue(len(output) == 0) | ||
| 41 | 34 | ||
| 42 | - def test_logging_silently(self): | ||
| 43 | - """ | ||
| 44 | - Test that nothing will be logged when logging is not enabled | ||
| 45 | - and we are using a silent logger (uses the NullHandler) | ||
| 46 | - """ | ||
| 47 | - output = self._run_test(['silent']) | 35 | + def test_it_doesnt_log_json_when_not_enabled(self): |
| 36 | + output = self._run_test(['as-json', 'debug']) | ||
| 48 | self.assertTrue(len(output) == 0) | 37 | self.assertTrue(len(output) == 0) |
| 49 | 38 | ||
| 50 | - def test_setting_level_in_main_module(self): | ||
| 51 | - """ | ||
| 52 | - Make sure that the level set in the main module is kept when | ||
| 53 | - logging from imported modules. | ||
| 54 | - """ | ||
| 55 | - output = self._run_test(['debug']) | 39 | + def test_logs_when_enabled(self): |
| 40 | + output = self._run_test(['enable', 'warning']) | ||
| 56 | 41 | ||
| 57 | - expected_messages = MAIN_LOG_MESSAGES + [ | ||
| 58 | - log_helper_test_imported.DEBUG_MESSAGE, | ||
| 59 | - log_helper_test_imported.INFO_MESSAGE, | 42 | + expected_messages = [ |
| 43 | + log_helper_test_main.WARNING_MESSAGE, | ||
| 44 | + log_helper_test_main.ERROR_MESSAGE, | ||
| 45 | + log_helper_test_main.CRITICAL_MESSAGE, | ||
| 60 | log_helper_test_imported.WARNING_MESSAGE, | 46 | log_helper_test_imported.WARNING_MESSAGE, |
| 61 | log_helper_test_imported.ERROR_MESSAGE, | 47 | log_helper_test_imported.ERROR_MESSAGE, |
| 62 | log_helper_test_imported.CRITICAL_MESSAGE | 48 | log_helper_test_imported.CRITICAL_MESSAGE |
| @@ -65,97 +51,60 @@ class TestLogHelper(unittest.TestCase): | @@ -65,97 +51,60 @@ class TestLogHelper(unittest.TestCase): | ||
| 65 | for msg in expected_messages: | 51 | for msg in expected_messages: |
| 66 | self.assertIn(msg, output) | 52 | self.assertIn(msg, output) |
| 67 | 53 | ||
| 68 | - def test_logging_at_current_level(self): | ||
| 69 | - """ | ||
| 70 | - Test that logging at current level will always print a message | ||
| 71 | - """ | ||
| 72 | - output = self._run_test(['current_level']) | ||
| 73 | - self.assertIn(log_helper_test_main.DEBUG_MESSAGE, output) | ||
| 74 | - | ||
| 75 | - def test_logging_as_json(self): | ||
| 76 | - """ | ||
| 77 | - Basic test for json logging | ||
| 78 | - """ | ||
| 79 | - output = self._run_test(['critical', '-j']) | 54 | + def test_logs_json_when_enabled(self): |
| 55 | + output = self._run_test(['enable', 'as-json', 'critical']) | ||
| 80 | 56 | ||
| 81 | - try: | ||
| 82 | - json_data = json.loads(output) | ||
| 83 | - self._assert_json_messages(json_data, [ | ||
| 84 | - log_helper_test_main.CRITICAL_MESSAGE, | ||
| 85 | - log_helper_test_imported.CRITICAL_MESSAGE | ||
| 86 | - ]) | ||
| 87 | - except ValueError: | ||
| 88 | - self.fail('Invalid json:\n' + output) | ||
| 89 | - self.assertNotEqual(len(json_data), 0, msg='Output was empty') | ||
| 90 | - | ||
| 91 | - def test_logging_dictionary_as_json(self): | ||
| 92 | - """ | ||
| 93 | - Test support for passing a dictionary to the logger | ||
| 94 | - and have it logged as JSON | ||
| 95 | - """ | ||
| 96 | - output = self._run_test(['dictionary']) | ||
| 97 | - | ||
| 98 | - try: | ||
| 99 | - json_data = json.loads(output) | ||
| 100 | - self._assert_json_messages(json_data, [ | ||
| 101 | - log_helper_test_main.DEBUG_MESSAGE | ||
| 102 | - ]) | ||
| 103 | - except ValueError: | ||
| 104 | - self.fail('Invalid json:\n' + output) | ||
| 105 | - self.assertNotEqual(len(json_data), 0, msg='Output was empty') | 57 | + self._assert_json_messages(output, [ |
| 58 | + log_helper_test_main.CRITICAL_MESSAGE, | ||
| 59 | + log_helper_test_imported.CRITICAL_MESSAGE | ||
| 60 | + ]) | ||
| 106 | 61 | ||
| 107 | def test_json_correct_on_exceptions(self): | 62 | def test_json_correct_on_exceptions(self): |
| 108 | """ | 63 | """ |
| 109 | Test that even on unhandled exceptions our JSON is always correct | 64 | Test that even on unhandled exceptions our JSON is always correct |
| 110 | """ | 65 | """ |
| 111 | - output = self._run_test(['critical', 'throw', '-j'], True) | 66 | + output = self._run_test(['enable', 'as-json', 'throw', 'critical'], False) |
| 67 | + self._assert_json_messages(output, [ | ||
| 68 | + log_helper_test_main.CRITICAL_MESSAGE, | ||
| 69 | + log_helper_test_imported.CRITICAL_MESSAGE | ||
| 70 | + ]) | ||
| 112 | 71 | ||
| 72 | + def _assert_json_messages(self, output, messages): | ||
| 113 | try: | 73 | try: |
| 114 | json_data = json.loads(output) | 74 | json_data = json.loads(output) |
| 115 | - self._assert_json_messages(json_data, [ | ||
| 116 | - log_helper_test_main.CRITICAL_MESSAGE, | ||
| 117 | - log_helper_test_imported.CRITICAL_MESSAGE | ||
| 118 | - ]) | 75 | + self.assertEquals(len(json_data), len(messages)) |
| 76 | + | ||
| 77 | + for i in range(len(messages)): | ||
| 78 | + self.assertEquals(messages[i], json_data[i]['msg']) | ||
| 119 | except ValueError: | 79 | except ValueError: |
| 120 | self.fail('Invalid json:\n' + output) | 80 | self.fail('Invalid json:\n' + output) |
| 121 | - self.assertNotEqual(len(json_data), 0, msg='Output was empty') | ||
| 122 | 81 | ||
| 123 | - def _assert_json_messages(self, json_data, messages): | ||
| 124 | - self.assertEquals(len(json_data), len(messages)) | ||
| 125 | - | ||
| 126 | - for i in range(len(messages)): | ||
| 127 | - self.assertEquals(messages[i], json_data[i]['msg']) | 82 | + self.assertNotEqual(len(json_data), 0, msg='Output was empty') |
| 128 | 83 | ||
| 129 | - @staticmethod | ||
| 130 | - def _run_test(args, ignore_exceptions=False): | 84 | + def _run_test(self, args, should_succeed=True): |
| 131 | """ | 85 | """ |
| 132 | Use subprocess to better simulate the real scenario and avoid | 86 | Use subprocess to better simulate the real scenario and avoid |
| 133 | logging conflicts when running multiple tests (since logging depends on singletons, | 87 | logging conflicts when running multiple tests (since logging depends on singletons, |
| 134 | we might get errors or false positives between sequential tests runs) | 88 | we might get errors or false positives between sequential tests runs) |
| 135 | """ | 89 | """ |
| 136 | - try: | ||
| 137 | - output = check_output( | ||
| 138 | - [PYTHON_EXECUTABLE, TEST_FILE] + args, | ||
| 139 | - shell=False, | ||
| 140 | - cwd=ROOT_DIRECTORY, | ||
| 141 | - stderr=STDOUT, | ||
| 142 | - universal_newlines=True | ||
| 143 | - ) | ||
| 144 | - | ||
| 145 | - if not isinstance(output, str): | ||
| 146 | - output = output.decode('utf-8') | ||
| 147 | - except CalledProcessError as ex: | ||
| 148 | - if ignore_exceptions: | ||
| 149 | - output = ex.output | ||
| 150 | - else: | ||
| 151 | - # we want tests to fail if an exception occur | ||
| 152 | - print('Caught unexpected error. Print output and re-raise') | ||
| 153 | - for line in ex.output.splitlines(): | ||
| 154 | - print('error output: {}'.format(line.rstrip())) | ||
| 155 | - print('(If you have errors about imports, try unsetting PYTHONPATH)') | ||
| 156 | - raise ex | ||
| 157 | - | ||
| 158 | - return REGEX.search(output).group(1).strip() | 90 | + child = subprocess.Popen( |
| 91 | + [PYTHON_EXECUTABLE, TEST_FILE] + args, | ||
| 92 | + shell=False, | ||
| 93 | + env={'PYTHONPATH': ROOT_DIRECTORY}, | ||
| 94 | + universal_newlines=True, | ||
| 95 | + cwd=ROOT_DIRECTORY, | ||
| 96 | + stdin=None, | ||
| 97 | + stdout=subprocess.PIPE, | ||
| 98 | + stderr=subprocess.PIPE | ||
| 99 | + ) | ||
| 100 | + (output, output_err) = child.communicate() | ||
| 101 | + | ||
| 102 | + if not isinstance(output, str): | ||
| 103 | + output = output.decode('utf-8') | ||
| 104 | + | ||
| 105 | + self.assertEquals(child.returncode == 0, should_succeed) | ||
| 106 | + | ||
| 107 | + return output.strip() | ||
| 159 | 108 | ||
| 160 | 109 | ||
| 161 | # just in case somebody calls this file as a script | 110 | # just in case somebody calls this file as a script |