From 3ce748a111d3a09b3e86c8eb695fcbec7d801bdc Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Wed, 18 Nov 2020 12:25:37 +0000 Subject: [PATCH] Removed custom logger Molecule no longer uses a custom logger class and we make use of the standard python logger class. This change replaces logger.success() and logger.out() with logger.info() calls. --- src/molecule/command/idempotence.py | 4 +-- src/molecule/command/init/role.py | 2 +- src/molecule/command/init/scenario.py | 2 +- .../dependency/ansible_galaxy/base.py | 2 -- src/molecule/dependency/base.py | 4 +-- src/molecule/logger.py | 28 +------------------ src/molecule/provisioner/ansible_playbook.py | 11 +------- src/molecule/test/functional/conftest.py | 2 +- .../test/unit/command/init/test_role.py | 6 ++-- .../test/unit/command/init/test_scenario.py | 6 ++-- .../test/unit/command/test_idempotence.py | 8 ++---- src/molecule/test/unit/conftest.py | 10 ------- .../ansible_galaxy/test_collections.py | 4 +-- .../dependency/ansible_galaxy/test_roles.py | 4 +-- .../test/unit/dependency/test_shell.py | 4 +-- src/molecule/test/unit/test_config.py | 4 +-- .../test/unit/verifier/test_ansible.py | 8 ++---- .../test/unit/verifier/test_testinfra.py | 9 +++--- src/molecule/verifier/ansible.py | 2 +- src/molecule/verifier/testinfra.py | 4 +-- 20 files changed, 34 insertions(+), 90 deletions(-) diff --git a/src/molecule/command/idempotence.py b/src/molecule/command/idempotence.py index 3fbff5ecf0..43cab4d476 100644 --- a/src/molecule/command/idempotence.py +++ b/src/molecule/command/idempotence.py @@ -78,12 +78,12 @@ def execute(self): msg = "Instances not converged. Please converge instances first." util.sysexit_with_message(msg) - output = self._config.provisioner.converge(out=None, err=None) + output = self._config.provisioner.converge() idempotent = self._is_idempotent(output) if idempotent: msg = "Idempotence completed successfully." - LOG.success(msg) + LOG.info(msg) else: msg = ( "Idempotence test failed because of the following tasks:\n" u"{}" diff --git a/src/molecule/command/init/role.py b/src/molecule/command/init/role.py index 3e88fd2ab3..8dfcb1b3ac 100644 --- a/src/molecule/command/init/role.py +++ b/src/molecule/command/init/role.py @@ -89,7 +89,7 @@ def execute(self): role_directory = os.path.join(role_directory, role_name) msg = "Initialized role in {} successfully.".format(role_directory) - LOG.success(msg) + LOG.info(msg) @command_base.click_command_ex() diff --git a/src/molecule/command/init/scenario.py b/src/molecule/command/init/scenario.py index 72f6a2c247..b6d400f710 100644 --- a/src/molecule/command/init/scenario.py +++ b/src/molecule/command/init/scenario.py @@ -112,7 +112,7 @@ def execute(self): role_directory = os.path.join(role_directory, role_name) msg = "Initialized scenario in {} successfully.".format(scenario_directory) - LOG.success(msg) + LOG.info(msg) def _role_exists(ctx, param, value): # pragma: no cover diff --git a/src/molecule/dependency/ansible_galaxy/base.py b/src/molecule/dependency/ansible_galaxy/base.py index 5862369fc1..37326a439c 100644 --- a/src/molecule/dependency/ansible_galaxy/base.py +++ b/src/molecule/dependency/ansible_galaxy/base.py @@ -107,8 +107,6 @@ def bake(self): self._sh_command = util.BakedCommand( cmd=[self.command, *self.COMMANDS, *util.dict2args(options), *verbose_flag], env=self.env, - stdout=LOG.out, - stderr=LOG.error, ) def execute(self): diff --git a/src/molecule/dependency/base.py b/src/molecule/dependency/base.py index c6c13fac68..420f312dc7 100644 --- a/src/molecule/dependency/base.py +++ b/src/molecule/dependency/base.py @@ -55,7 +55,7 @@ def execute_with_retries(self): # print(555, self._sh_command) util.run_command(self._sh_command, debug=self._config.debug) msg = "Dependency completed successfully." - LOG.success(msg) + LOG.info(msg) return except Exception: pass @@ -72,7 +72,7 @@ def execute_with_retries(self): try: util.run_command(self._sh_command, debug=self._config.debug) msg = "Dependency completed successfully." - LOG.success(msg) + LOG.info(msg) return except Exception as _exception: exception = _exception diff --git a/src/molecule/logger.py b/src/molecule/logger.py index e8667245fd..2470c13d97 100644 --- a/src/molecule/logger.py +++ b/src/molecule/logger.py @@ -26,8 +26,7 @@ from enrich.console import Console from enrich.logging import RichHandler -from molecule.console import console, should_do_markup, theme -from molecule.text import chomp +from molecule.console import should_do_markup, theme SUCCESS = 100 OUT = 101 @@ -45,29 +44,6 @@ def filter(self, logRecord): # pragma: no cover return logRecord.levelno <= self.__level -class CustomLogger(logging.getLoggerClass()): # type: ignore # see https://sam.hooke.me/note/2020/03/mypy-and-verbose-logging/ - """ - A custom logging class which adds additional methods to the logger. - - These methods serve as syntactic sugar for formatting log messages. - """ - - def __init__(self, name, level=logging.NOTSET): - """Construct CustomLogger.""" - super(CustomLogger, self).__init__(name, level=level) - logging.addLevelName(SUCCESS, "SUCCESS") - logging.addLevelName(OUT, "OUT") - - def success(self, msg, *args, **kwargs): - if self.isEnabledFor(SUCCESS): - self._log(SUCCESS, msg, args, **kwargs) - - def out(self, msg, *args, **kwargs): - msg = chomp(msg) - if self.isEnabledFor(OUT): - console.print(msg, args, **kwargs) - - class TrailingNewlineFormatter(logging.Formatter): """A custom logging formatter which removes additional newlines from messages.""" @@ -86,8 +62,6 @@ def get_logger(name=None) -> logging.Logger: name, ``__name__``. :return: logger object """ - logging.setLoggerClass(CustomLogger) - logger = logging.getLogger(name) # type: logging.Logger logger.setLevel(logging.DEBUG) diff --git a/src/molecule/provisioner/ansible_playbook.py b/src/molecule/provisioner/ansible_playbook.py index 6f21f29ffb..635638ef04 100644 --- a/src/molecule/provisioner/ansible_playbook.py +++ b/src/molecule/provisioner/ansible_playbook.py @@ -18,7 +18,6 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # DEALINGS IN THE SOFTWARE. """Ansible-Playbook Provisioner Module.""" - from molecule import logger, util LOG = logger.get_logger(__name__) @@ -27,24 +26,18 @@ class AnsiblePlaybook(object): """Privisioner Playbook.""" - def __init__(self, playbook, config, out=LOG.out, err=LOG.error): + def __init__(self, playbook, config): """ Set up the requirements to execute ``ansible-playbook`` and returns \ None. :param playbook: A string containing the path to the playbook. :param config: An instance of a Molecule config. - :param out: An optional function to process STDOUT for underlying - :func:``sh`` call. - :param err: An optional function to process STDERR for underlying - :func:``sh`` call. :returns: None """ self._ansible_command = None self._playbook = playbook self._config = config - self._out = out - self._err = err self._cli = {} self._env = self._config.provisioner.env @@ -92,8 +85,6 @@ def bake(self): ], cwd=self._config.scenario.directory, env=self._env, - stdout=self._out, - stderr=self._err, ) def execute(self): diff --git a/src/molecule/test/functional/conftest.py b/src/molecule/test/functional/conftest.py index 9b700e0fd8..ebcab1c4a1 100644 --- a/src/molecule/test/functional/conftest.py +++ b/src/molecule/test/functional/conftest.py @@ -73,7 +73,7 @@ def with_scenario(request, scenario_to_test, driver_name, scenario_name, skip_te yield if scenario_name: msg = "CLEANUP: Destroying instances for all scenario(s)" - LOG.out(msg) + LOG.info(msg) cmd = ["molecule", "destroy", "--driver-name", driver_name, "--all"] assert run_command(cmd).returncode == 0 diff --git a/src/molecule/test/unit/command/init/test_role.py b/src/molecule/test/unit/command/init/test_role.py index 95cd752e99..bfcd7242c3 100644 --- a/src/molecule/test/unit/command/init/test_role.py +++ b/src/molecule/test/unit/command/init/test_role.py @@ -43,18 +43,18 @@ def _instance(_command_args): return role.Role(_command_args) -def test_execute(temp_dir, _instance, patched_logger_info, patched_logger_success): +def test_execute(temp_dir, _instance, patched_logger_info): _instance.execute() msg = "Initializing new role test-role..." - patched_logger_info.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) assert os.path.isdir("./test-role") assert os.path.isdir("./test-role/molecule/default") role_directory = os.path.join(temp_dir.strpath, "test-role") msg = "Initialized role in {} successfully.".format(role_directory) - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) def test_execute_role_exists(temp_dir, _instance, patched_logger_critical): diff --git a/src/molecule/test/unit/command/init/test_scenario.py b/src/molecule/test/unit/command/init/test_scenario.py index 7c54023694..c06af9d2fb 100644 --- a/src/molecule/test/unit/command/init/test_scenario.py +++ b/src/molecule/test/unit/command/init/test_scenario.py @@ -49,17 +49,17 @@ def invalid_template_dir(resources_folder_path): return invalid_role_template_path -def test_execute(temp_dir, _instance, patched_logger_info, patched_logger_success): +def test_execute(temp_dir, _instance, patched_logger_info): _instance.execute() msg = "Initializing new scenario test-scenario..." - patched_logger_info.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) assert os.path.isdir("./molecule/test-scenario") scenario_directory = os.path.join(temp_dir.strpath, "molecule", "test-scenario") msg = "Initialized scenario in {} successfully.".format(scenario_directory) - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) def test_execute_scenario_exists(temp_dir, _instance, patched_logger_critical): diff --git a/src/molecule/test/unit/command/test_idempotence.py b/src/molecule/test/unit/command/test_idempotence.py index 16fd707175..11bef67ef4 100644 --- a/src/molecule/test/unit/command/test_idempotence.py +++ b/src/molecule/test/unit/command/test_idempotence.py @@ -17,7 +17,6 @@ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # DEALINGS IN THE SOFTWARE. - import pytest from molecule.command import idempotence @@ -43,22 +42,21 @@ def test_execute( patched_logger_info, patched_ansible_converge, _patched_is_idempotent, - patched_logger_success, _instance, ): _instance.execute() - assert len(patched_logger_info.mock_calls) == 1 + assert len(patched_logger_info.mock_calls) == 2 name, args, kwargs = patched_logger_info.mock_calls[0] assert "default" in args assert "idempotence" in args - patched_ansible_converge.assert_called_once_with(out=None, err=None) + patched_ansible_converge.assert_called_once_with() _patched_is_idempotent.assert_called_once_with("patched-ansible-converge-stdout") msg = "Idempotence completed successfully." - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) def test_execute_raises_when_not_converged( diff --git a/src/molecule/test/unit/conftest.py b/src/molecule/test/unit/conftest.py index 7039330405..0b70521f5e 100644 --- a/src/molecule/test/unit/conftest.py +++ b/src/molecule/test/unit/conftest.py @@ -167,11 +167,6 @@ def patched_logger_debug(mocker): return mocker.patch("logging.Logger.debug") -@pytest.fixture -def patched_logger_out(mocker): - return mocker.patch("molecule.logger.CustomLogger.out") - - @pytest.fixture def patched_logger_warning(mocker): return mocker.patch("logging.Logger.warning") @@ -187,11 +182,6 @@ def patched_logger_critical(mocker): return mocker.patch("logging.Logger.critical") -@pytest.fixture -def patched_logger_success(mocker): - return mocker.patch("molecule.logger.CustomLogger.success") - - @pytest.fixture def patched_run_command(mocker): m = mocker.patch("molecule.util.run_command") diff --git a/src/molecule/test/unit/dependency/ansible_galaxy/test_collections.py b/src/molecule/test/unit/dependency/ansible_galaxy/test_collections.py index 6794881f23..458b4c7acd 100644 --- a/src/molecule/test/unit/dependency/ansible_galaxy/test_collections.py +++ b/src/molecule/test/unit/dependency/ansible_galaxy/test_collections.py @@ -149,7 +149,7 @@ def test_collections_bake(_instance, role_file, roles_path): def test_execute( patched_run_command, _patched_ansible_galaxy_has_requirements_file, - patched_logger_success, + patched_logger_info, _instance, ): _instance._sh_command = "patched-command" @@ -163,7 +163,7 @@ def test_execute( patched_run_command.assert_called_once_with("patched-command", debug=False) msg = "Dependency completed successfully." - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_called_once_with(msg) def test_execute_does_not_execute_when_disabled( diff --git a/src/molecule/test/unit/dependency/ansible_galaxy/test_roles.py b/src/molecule/test/unit/dependency/ansible_galaxy/test_roles.py index 1b2065f3de..ddac1d8881 100644 --- a/src/molecule/test/unit/dependency/ansible_galaxy/test_roles.py +++ b/src/molecule/test/unit/dependency/ansible_galaxy/test_roles.py @@ -145,7 +145,7 @@ def test_galaxy_bake(_instance, role_file, roles_path): def test_execute( patched_run_command, _patched_ansible_galaxy_has_requirements_file, - patched_logger_success, + patched_logger_info, _instance, ): _instance._sh_command = "patched-command" @@ -159,7 +159,7 @@ def test_execute( patched_run_command.assert_called_once_with("patched-command", debug=False) msg = "Dependency completed successfully." - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_called_once_with(msg) def test_execute_does_not_execute_when_disabled( diff --git a/src/molecule/test/unit/dependency/test_shell.py b/src/molecule/test/unit/dependency/test_shell.py index 52c2df322e..34941d264c 100644 --- a/src/molecule/test/unit/dependency/test_shell.py +++ b/src/molecule/test/unit/dependency/test_shell.py @@ -90,14 +90,14 @@ def test_env_property(_instance): assert "bar" == _instance.env["FOO"] -def test_execute(patched_run_command, patched_logger_success, _instance): +def test_execute(patched_run_command, patched_logger_info, _instance): _instance._sh_command = "patched-command" _instance.execute() patched_run_command.assert_called_once_with("patched-command", debug=False) msg = "Dependency completed successfully." - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_called_once_with(msg) def test_execute_does_not_execute_when_disabled( diff --git a/src/molecule/test/unit/test_config.py b/src/molecule/test/unit/test_config.py index 836232492b..7509b0e052 100644 --- a/src/molecule/test/unit/test_config.py +++ b/src/molecule/test/unit/test_config.py @@ -292,9 +292,7 @@ def test_preflight_exists_when_validation_fails( patched_logger_critical.assert_called_once_with(msg) -def test_validate( - mocker, config_instance, patched_logger_debug, patched_logger_success -): +def test_validate(mocker, config_instance, patched_logger_debug): m = mocker.patch("molecule.model.schema_v3.validate") m.return_value = None diff --git a/src/molecule/test/unit/verifier/test_ansible.py b/src/molecule/test/unit/verifier/test_ansible.py index 04c7009087..d77ce40d44 100644 --- a/src/molecule/test/unit/verifier/test_ansible.py +++ b/src/molecule/test/unit/verifier/test_ansible.py @@ -76,18 +76,16 @@ def test_options_property_handles_cli_args(_instance): assert x == _instance.options -def test_execute( - patched_logger_info, _patched_ansible_verify, patched_logger_success, _instance -): +def test_execute(patched_logger_info, _patched_ansible_verify, _instance): _instance.execute() _patched_ansible_verify.assert_called_once_with() msg = "Running Ansible Verifier" - patched_logger_info.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) msg = "Verifier completed successfully." - patched_logger_success.assert_called_once_with(msg) + patched_logger_info.assert_any_call(msg) def test_execute_does_not_execute( diff --git a/src/molecule/test/unit/verifier/test_testinfra.py b/src/molecule/test/unit/verifier/test_testinfra.py index 99b3f2fb6d..0ab3182636 100644 --- a/src/molecule/test/unit/verifier/test_testinfra.py +++ b/src/molecule/test/unit/verifier/test_testinfra.py @@ -21,6 +21,7 @@ import os import pytest +from mock import call from molecule import config, util from molecule.verifier import testinfra @@ -228,7 +229,6 @@ def test_execute( patched_logger_info, patched_run_command, _patched_testinfra_get_tests, - patched_logger_success, _instance, ): _instance._testinfra_command = "patched-command" @@ -237,10 +237,9 @@ def test_execute( patched_run_command.assert_called_once_with("patched-command", debug=False) msg = "Executing Testinfra tests found in {}/...".format(_instance.directory) - patched_logger_info.assert_called_once_with(msg) - - msg = "Verifier completed successfully." - patched_logger_success.assert_called_once_with(msg) + msg2 = "Verifier completed successfully." + calls = [call(msg), call(msg2)] + patched_logger_info.assert_has_calls(calls) def test_execute_does_not_execute( diff --git a/src/molecule/verifier/ansible.py b/src/molecule/verifier/ansible.py index a9a04c5e57..ecf4ef437c 100644 --- a/src/molecule/verifier/ansible.py +++ b/src/molecule/verifier/ansible.py @@ -82,7 +82,7 @@ def execute(self): self._config.provisioner.verify() msg = "Verifier completed successfully." - log.success(msg) + log.info(msg) def schema(self): return { diff --git a/src/molecule/verifier/testinfra.py b/src/molecule/verifier/testinfra.py index 63f8969433..a159ab6e7e 100644 --- a/src/molecule/verifier/testinfra.py +++ b/src/molecule/verifier/testinfra.py @@ -161,8 +161,6 @@ def bake(self): cmd=["pytest", *util.dict2args(options), *self._tests, *args], cwd=self._config.scenario.directory, env=self.env, - stdout=LOG.out, - stderr=LOG.error, ) # print(self._testinfra_command.cmd) @@ -186,7 +184,7 @@ def execute(self): result = util.run_command(self._testinfra_command, debug=self._config.debug) if result.returncode == 0: msg = "Verifier completed successfully." - LOG.success(msg) + LOG.info(msg) else: util.sysexit(result.returncode)