From 05b0c4c50f9504bcbc0eee2a65029d90ad04611e Mon Sep 17 00:00:00 2001 From: Jennifer Power Date: Tue, 26 Mar 2024 15:42:18 -0400 Subject: [PATCH] fix: prevent extra log messages in stdout (#199) * fix: prevent extra log messages in stdout GitLab CI still logs extra messages with the log filtering. Adds this line to prevent this. Signed-off-by: Jennifer Power * refactor: moves configure_test_logger to testutils The test_logger setup should not be packaged with the trestlebot code Signed-off-by: Jennifer Power * chore: do not fail CI for safety checks Safety checks should not fail PRs with no dependency changes Signed-off-by: Jennifer Power --------- Signed-off-by: Jennifer Power --- Makefile | 2 +- tests/testutils.py | 13 ++++++++++ tests/trestlebot/entrypoints/test_autosync.py | 24 +++++++++++++++++-- .../trestlebot/entrypoints/test_create_cd.py | 12 ++++++++-- .../trestlebot/entrypoints/test_create_ssp.py | 17 +++++++++++-- .../entrypoints/test_sync_upstreams.py | 13 ++++++++-- trestlebot/entrypoints/log.py | 15 ++++++++---- 7 files changed, 82 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 86033b9a..264007a8 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ test-code-cov: # https://github.com/python-poetry/poetry/issues/994#issuecomment-831598242 # Check for CVEs locally. For continuous dependency updates, we use dependabot. dep-cve-check: - @poetry export -f requirements.txt --without-hashes | poetry run safety check --stdin + @poetry export -f requirements.txt --without-hashes | poetry run safety check --continue-on-error --stdin .PHONY: dep-cve-check security-check: diff --git a/tests/testutils.py b/tests/testutils.py index 525f3331..5c5d2283 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -5,6 +5,7 @@ """Helper functions for unit test setup and teardown.""" import argparse +import logging import pathlib import shutil import tempfile @@ -21,6 +22,7 @@ from trestle.oscal import profile as prof from trestlebot.const import YAML_EXTENSION +from trestlebot.entrypoints.log import configure_logger JSON_TEST_DATA_PATH = pathlib.Path("tests/data/json/").resolve() @@ -32,6 +34,17 @@ TEST_REMOTE_REPO_URL = "http://localhost:8080/test.git" +def configure_test_logger(level: int = logging.INFO) -> None: + """ + Configure the logger for testing. + + Notes: This is used to patch the logger in tests + so the caplog can be used to capture log messages. + This does not happen when propagate is set to False. + """ + configure_logger(level=level, propagate=True) + + def clean(repo_path: str, repo: Optional[Repo]) -> None: """Clean up the temporary Git repository.""" if repo is not None: diff --git a/tests/trestlebot/entrypoints/test_autosync.py b/tests/trestlebot/entrypoints/test_autosync.py index 81f429bb..b6d7ccc5 100644 --- a/tests/trestlebot/entrypoints/test_autosync.py +++ b/tests/trestlebot/entrypoints/test_autosync.py @@ -7,11 +7,11 @@ import argparse import logging from typing import Any, Dict -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest -from tests.testutils import args_dict_to_list +from tests.testutils import args_dict_to_list, configure_test_logger from trestlebot.entrypoints.autosync import AutoSyncEntrypoint from trestlebot.entrypoints.autosync import main as cli_main from trestlebot.entrypoints.entrypoint_base import EntrypointInvalidArgException @@ -60,6 +60,10 @@ def test_validate_args_invalid_model(valid_args_dict: Dict[str, str]) -> None: auto_sync.validate_args(args) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_no_ssp_index(valid_args_dict: Dict[str, str], caplog: Any) -> None: """Test missing index file for ssp""" args_dict = valid_args_dict @@ -77,6 +81,10 @@ def test_no_ssp_index(valid_args_dict: Dict[str, str], caplog: Any) -> None: ) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_no_markdown_path(valid_args_dict: Dict[str, str], caplog: Any) -> None: """Test without a markdown file passed as a flag""" args_dict = valid_args_dict @@ -92,6 +100,10 @@ def test_no_markdown_path(valid_args_dict: Dict[str, str], caplog: Any) -> None: ) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_non_existent_working_dir(valid_args_dict: Dict[str, str], caplog: Any) -> None: """Test with a non-existent working directory""" args_dict = valid_args_dict @@ -107,6 +119,10 @@ def test_non_existent_working_dir(valid_args_dict: Dict[str, str], caplog: Any) ) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_invalid_working_dir(valid_args_dict: Dict[str, str], caplog: Any) -> None: """Test with directory that is not a trestle project root""" args_dict = valid_args_dict @@ -122,6 +138,10 @@ def test_invalid_working_dir(valid_args_dict: Dict[str, str], caplog: Any) -> No ) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_with_target_branch( tmp_trestle_dir: str, valid_args_dict: Dict[str, str], caplog: Any ) -> None: diff --git a/tests/trestlebot/entrypoints/test_create_cd.py b/tests/trestlebot/entrypoints/test_create_cd.py index bf539897..b96d12eb 100644 --- a/tests/trestlebot/entrypoints/test_create_cd.py +++ b/tests/trestlebot/entrypoints/test_create_cd.py @@ -7,11 +7,11 @@ import logging import pathlib from typing import Any, Dict -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest -from tests.testutils import args_dict_to_list, setup_for_compdef +from tests.testutils import args_dict_to_list, configure_test_logger, setup_for_compdef from trestlebot.entrypoints.create_cd import main as cli_main @@ -49,6 +49,10 @@ def test_create_cd_with_missing_args( cli_main() +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_create_cd_with_missing_profile( tmp_trestle_dir: str, valid_args_dict: Dict[str, str], caplog: Any ) -> None: @@ -72,6 +76,10 @@ def test_create_cd_with_missing_profile( ) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_create_cd_with_missing_filter_profile( tmp_trestle_dir: str, valid_args_dict: Dict[str, str], caplog: Any ) -> None: diff --git a/tests/trestlebot/entrypoints/test_create_ssp.py b/tests/trestlebot/entrypoints/test_create_ssp.py index 7d7d7076..ec303655 100644 --- a/tests/trestlebot/entrypoints/test_create_ssp.py +++ b/tests/trestlebot/entrypoints/test_create_ssp.py @@ -7,12 +7,17 @@ import logging import pathlib from typing import Any, Dict, Tuple -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest from git import Repo -from tests.testutils import TEST_YAML_HEADER, args_dict_to_list, setup_for_ssp +from tests.testutils import ( + TEST_YAML_HEADER, + args_dict_to_list, + configure_test_logger, + setup_for_ssp, +) from trestlebot.entrypoints.create_ssp import main as cli_main @@ -43,6 +48,10 @@ def base_args_dict() -> Dict[str, str]: ] +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_create_ssp( tmp_repo: Tuple[str, Repo], base_args_dict: Dict[str, str], caplog: Any ) -> None: @@ -78,6 +87,10 @@ def test_create_ssp( ) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_create_ssp_with_error( tmp_repo: Tuple[str, Repo], base_args_dict: Dict[str, str], caplog: Any ) -> None: diff --git a/tests/trestlebot/entrypoints/test_sync_upstreams.py b/tests/trestlebot/entrypoints/test_sync_upstreams.py index dad2271e..f08c14f2 100644 --- a/tests/trestlebot/entrypoints/test_sync_upstreams.py +++ b/tests/trestlebot/entrypoints/test_sync_upstreams.py @@ -6,12 +6,17 @@ import logging from typing import Any, Dict, Tuple -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest from git import Repo -from tests.testutils import args_dict_to_list, clean, prepare_upstream_repo +from tests.testutils import ( + args_dict_to_list, + clean, + configure_test_logger, + prepare_upstream_repo, +) from trestlebot.entrypoints.sync_upstreams import main as cli_main @@ -122,6 +127,10 @@ def test_with_exclude_model_names( clean(source, None) +@patch( + "trestlebot.entrypoints.log.configure_logger", + Mock(side_effect=configure_test_logger), +) def test_with_no_sources(valid_args_dict: Dict[str, str], caplog: Any) -> None: """Test with an invalid source argument.""" args_dict = valid_args_dict diff --git a/trestlebot/entrypoints/log.py b/trestlebot/entrypoints/log.py index e2995b24..a0d95d9b 100644 --- a/trestlebot/entrypoints/log.py +++ b/trestlebot/entrypoints/log.py @@ -7,6 +7,7 @@ import argparse import logging import sys +from typing import List import trestle.common.log as log @@ -26,10 +27,17 @@ def set_log_level_from_args(args: argparse.Namespace) -> None: configure_logger(logging.INFO) -def configure_logger(level: int = logging.INFO) -> None: +def configure_logger(level: int = logging.INFO, propagate: bool = False) -> None: """Configure the logger.""" + # Prevent extra message + _logger.propagate = propagate _logger.setLevel(level=level) + for handler in configure_handlers(): + _logger.addHandler(handler) + +def configure_handlers() -> List[logging.Handler]: + """Configure the handlers.""" # Create a StreamHandler to send non-error logs to stdout stdout_info_handler = logging.StreamHandler(sys.stdout) stdout_info_handler.setLevel(logging.INFO) @@ -49,7 +57,4 @@ def configure_logger(level: int = logging.INFO) -> None: ) stdout_debug_handler.setFormatter(detailed_formatter) stderr_handler.setFormatter(detailed_formatter) - - _logger.addHandler(stdout_debug_handler) - _logger.addHandler(stdout_info_handler) - _logger.addHandler(stderr_handler) + return [stdout_debug_handler, stdout_info_handler, stderr_handler]