From 12694022d9f85e227df5f2730e8f01a0c32f1d09 Mon Sep 17 00:00:00 2001 From: Quinn Date: Thu, 18 Dec 2025 18:03:46 +0000 Subject: [PATCH 1/8] Preliminary logging and error reporting overhaul. --- .gitignore | 1 + integration-tests/.pytest.ini | 15 ++++-- integration-tests/tests/conftest.py | 29 +++++++++++ .../tests/fixtures/package_config.py | 6 +++ .../tests/fixtures/package_instance.py | 18 +++---- .../tests/utils/asserting_utils.py | 50 +++++++++++++------ .../tests/utils/clp_mode_utils.py | 11 +++- .../tests/utils/package_utils.py | 10 ++-- integration-tests/tests/utils/port_utils.py | 2 +- 9 files changed, 105 insertions(+), 37 deletions(-) diff --git a/.gitignore b/.gitignore index 41da100695..09d150ce95 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ __pycache__/ .task/ build/ dist/ +__* \ No newline at end of file diff --git a/integration-tests/.pytest.ini b/integration-tests/.pytest.ini index 07fb7b9ee4..ccf9ad3530 100644 --- a/integration-tests/.pytest.ini +++ b/integration-tests/.pytest.ini @@ -1,20 +1,25 @@ [pytest] addopts = - --capture=no --code-highlight=yes --color=yes - -rA + --show-capture=no + --tb=short --strict-config --strict-markers - --verbose env = D:CLP_BUILD_DIR=../build D:CLP_CORE_BINS_DIR=../build/core D:CLP_PACKAGE_DIR=../build/clp-package + log_cli = True -log_cli_date_format = %Y-%m-%d %H:%M:%S,%f -log_cli_format = %(name)s %(asctime)s [%(levelname)s] %(message)s log_cli_level = INFO +log_cli_format = %(levelname)-10s %(asctime)-25s %(message)s +log_cli_date_format = %Y-%m-%d %H:%M:%S +log_auto_indent = 37 + +log_file_mode = a +log_file_level = DEBUG + markers = clp: mark tests that use the CLP storage engine clp_s: mark tests that use the CLP-S storage engine diff --git a/integration-tests/tests/conftest.py b/integration-tests/tests/conftest.py index c1af599c98..2e4d7574bf 100644 --- a/integration-tests/tests/conftest.py +++ b/integration-tests/tests/conftest.py @@ -1,5 +1,9 @@ """Global pytest setup.""" +import uuid +from collections.abc import Iterator +from pathlib import Path + import pytest # Make the fixtures defined in `tests/fixtures/` globally available without imports. @@ -23,3 +27,28 @@ def pytest_addoption(parser: pytest.Parser) -> None: default="55000", help="Base port for CLP package integration tests.", ) + + test_run_id = str(uuid.uuid4())[-4:] + log_file_name = Path("__pytest_testrun_logs") / f"testrun_{test_run_id}.log" + parser.addini( + "log_file_path", + help="Path to the log file for this test.", + type="paths", + default=log_file_name, + ) + +@pytest.hookimpl(tryfirst=True) +def pytest_report_header(config) -> str: + """Docstring""" + log_file_path = Path(config.getini("log_file_path")).expanduser().resolve() + return f"Log file path for this test run: {log_file_path}" + + +@pytest.hookimpl(hookwrapper=True,tryfirst=True) +def pytest_runtest_setup(item)-> Iterator[None]: + """Docstring""" + config = item.config + logging_plugin = config.pluginmanager.get_plugin("logging-plugin") + log_file_path = Path(config.getini("log_file_path")) + logging_plugin.set_log_path(str(log_file_path)) + yield diff --git a/integration-tests/tests/fixtures/package_config.py b/integration-tests/tests/fixtures/package_config.py index 8db533dcf2..be0d44b21f 100644 --- a/integration-tests/tests/fixtures/package_config.py +++ b/integration-tests/tests/fixtures/package_config.py @@ -1,5 +1,6 @@ """Fixtures that create and remove temporary config files for CLP packages.""" +import logging from collections.abc import Iterator import pytest @@ -11,6 +12,8 @@ from tests.utils.config import PackageConfig, PackagePathConfig from tests.utils.port_utils import assign_ports_from_base +logger = logging.getLogger(__name__) + @pytest.fixture def fixt_package_config( @@ -25,6 +28,7 @@ def fixt_package_config( :raise ValueError: if the CLP base port's value is invalid. """ mode_name: str = request.param + logger.info("Setting up the '%s' package...", mode_name) clp_config_obj = get_clp_config_from_mode(mode_name) @@ -34,6 +38,7 @@ def fixt_package_config( base_port = int(base_port_string) except ValueError as err: err_msg = f"Invalid value '{base_port_string}' for '--base-port'; expected an integer." + logger.error(err_msg) raise ValueError(err_msg) from err assign_ports_from_base(base_port, clp_config_obj) @@ -51,4 +56,5 @@ def fixt_package_config( try: yield package_config finally: + logger.info("Cleaning up the '%s' package...", mode_name) package_config.temp_config_file_path.unlink(missing_ok=True) diff --git a/integration-tests/tests/fixtures/package_instance.py b/integration-tests/tests/fixtures/package_instance.py index b2ea7ef4b3..5af4e36d53 100644 --- a/integration-tests/tests/fixtures/package_instance.py +++ b/integration-tests/tests/fixtures/package_instance.py @@ -1,5 +1,6 @@ """Fixtures that start and stop CLP package instances for integration tests.""" +import logging from collections.abc import Iterator import pytest @@ -13,9 +14,11 @@ stop_clp_package, ) +logger = logging.getLogger(__name__) + @pytest.fixture -def fixt_package_instance(fixt_package_config: PackageConfig) -> Iterator[PackageInstance]: +def fixt_package_instance(request: pytest.FixtureRequest, fixt_package_config: PackageConfig) -> Iterator[PackageInstance]: """ Starts a CLP package instance for the given configuration and stops it during teardown. @@ -25,15 +28,10 @@ def fixt_package_instance(fixt_package_config: PackageConfig) -> Iterator[Packag mode_name = fixt_package_config.mode_name try: - start_clp_package(fixt_package_config) + logger.info("Starting the '%s' package...", mode_name) + start_clp_package(request, fixt_package_config) instance = PackageInstance(package_config=fixt_package_config) yield instance - except RuntimeError: - base_port = fixt_package_config.base_port - pytest.fail( - f"Failed to start the {mode_name} package. This could mean that one of the ports" - f" derived from base_port={base_port} was unavailable. You can specify a new value for" - " base_port with the '--base-port' flag." - ) finally: - stop_clp_package(fixt_package_config) + logger.info("Stopping the '%s' package...", mode_name) + stop_clp_package(request, fixt_package_config) diff --git a/integration-tests/tests/utils/asserting_utils.py b/integration-tests/tests/utils/asserting_utils.py index 651f67ea2e..3f66e54627 100644 --- a/integration-tests/tests/utils/asserting_utils.py +++ b/integration-tests/tests/utils/asserting_utils.py @@ -1,8 +1,8 @@ """Utilities that raise pytest assertions on failure.""" import logging -import shlex import subprocess +from pathlib import Path from typing import Any import pytest @@ -17,7 +17,7 @@ logger = logging.getLogger(__name__) -def run_and_assert(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[Any]: +def run_and_assert(request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[Any]: """ Runs a command with subprocess and asserts that it succeeds with pytest. @@ -26,15 +26,29 @@ def run_and_assert(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess :return: The completed process object, for inspection or further handling. :raise: pytest.fail if the command exits with a non-zero return code. """ - logger.info("Running command: %s", shlex.join(cmd)) - - try: - proc = subprocess.run(cmd, check=True, **kwargs) - except subprocess.CalledProcessError as e: - pytest.fail(f"Command failed: {' '.join(cmd)}: {e}") - except subprocess.TimeoutExpired as e: - pytest.fail(f"Command timed out: {' '.join(cmd)}: {e}") - return proc + log_file_path = Path(request.config.getini("log_file_path")) + with open(log_file_path, "ab") as log_file: + try: + proc = subprocess.run( + cmd, + stdout=log_file, + stderr=log_file, + check=True, + **kwargs + ) + except subprocess.CalledProcessError: + fail_msg = ( + "Called process failed. Check error summary at the end of the test for more information." + ) + logger.error(fail_msg) + pytest.fail(fail_msg) + except subprocess.TimeoutExpired: + fail_msg = ( + "Called process timed out. Check error summary at the end of the test for more information." + ) + logger.error(fail_msg) + pytest.fail(fail_msg) + return proc def validate_package_running(package_instance: PackageInstance) -> None: @@ -45,6 +59,9 @@ def validate_package_running(package_instance: PackageInstance) -> None: :param package_instance: :raise pytest.fail: if the sets of running services and required components do not match. """ + mode_name = package_instance.package_config.mode_name + logger.info("Validating that all components of the '%s' package are running...", mode_name) + # Get list of services currently running in the Compose project. instance_id = package_instance.clp_instance_id project_name = f"clp-package-{instance_id}" @@ -55,17 +72,17 @@ def validate_package_running(package_instance: PackageInstance) -> None: if required_components == running_services: return - fail_msg = "Component mismatch." + cmd_fail_msg = "Component mismatch." missing_components = required_components - running_services if missing_components: - fail_msg += f"\nMissing components: {missing_components}." + cmd_fail_msg += f"\nMissing components: {missing_components}." unexpected_components = running_services - required_components if unexpected_components: - fail_msg += f"\nUnexpected services: {unexpected_components}." + cmd_fail_msg += f"\nUnexpected services: {unexpected_components}." - pytest.fail(fail_msg) + pytest.fail(cmd_fail_msg) def validate_running_mode_correct(package_instance: PackageInstance) -> None: @@ -79,6 +96,9 @@ def validate_running_mode_correct(package_instance: PackageInstance) -> None: :raise pytest.fail: if the ClpConfig object cannot be validated. :raise pytest.fail: if the running ClpConfig does not match the intended ClpConfig. """ + mode_name = package_instance.package_config.mode_name + logger.info("Validating that the '%s' package is running in the correct configuration...", mode_name) + shared_config_dict = load_yaml_to_dict(package_instance.shared_config_file_path) try: running_config = ClpConfig.model_validate(shared_config_dict) diff --git a/integration-tests/tests/utils/clp_mode_utils.py b/integration-tests/tests/utils/clp_mode_utils.py index 07370bc233..dbdad6e01d 100644 --- a/integration-tests/tests/utils/clp_mode_utils.py +++ b/integration-tests/tests/utils/clp_mode_utils.py @@ -1,5 +1,6 @@ """Provides utilities related to the user-level configurations of CLP's operating modes.""" +import logging from collections.abc import Callable from typing import Any @@ -23,6 +24,9 @@ ) from pydantic import BaseModel +logger = logging.getLogger(__name__) + + CLP_MODE_CONFIGS: dict[str, Callable[[], ClpConfig]] = { "clp-text": lambda: ClpConfig( package=Package( @@ -93,7 +97,12 @@ def get_clp_config_from_mode(mode_name: str) -> ClpConfig: :raise ValueError: If the mode is not supported. """ if mode_name not in CLP_MODE_CONFIGS: - err_msg = f"Unsupported mode: {mode_name}" + err_msg = f"Unsupported mode: '{mode_name}'" + err_log_msg = ( + err_msg + + "\nCheck the full output log for more information (see test header for file path)." + ) + logger.error(err_log_msg) raise ValueError(err_msg) return CLP_MODE_CONFIGS[mode_name]() diff --git a/integration-tests/tests/utils/package_utils.py b/integration-tests/tests/utils/package_utils.py index 7d714f707a..ff76d57f73 100644 --- a/integration-tests/tests/utils/package_utils.py +++ b/integration-tests/tests/utils/package_utils.py @@ -1,12 +1,12 @@ """Provides utility functions related to the CLP package used across `integration-tests`.""" +import pytest from tests.utils.asserting_utils import run_and_assert from tests.utils.config import PackageConfig DEFAULT_CMD_TIMEOUT_SECONDS = 120.0 - -def start_clp_package(package_config: PackageConfig) -> None: +def start_clp_package(request: pytest.FixtureRequest, package_config: PackageConfig) -> None: """ Starts an instance of the CLP package. @@ -23,10 +23,10 @@ def start_clp_package(package_config: PackageConfig) -> None: "--config", str(temp_config_file_path), ] # fmt: on - run_and_assert(start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + run_and_assert(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) -def stop_clp_package(package_config: PackageConfig) -> None: +def stop_clp_package(request: pytest.FixtureRequest, package_config: PackageConfig) -> None: """ Stops the running instance of the CLP package. @@ -43,4 +43,4 @@ def stop_clp_package(package_config: PackageConfig) -> None: "--config", str(temp_config_file_path), ] # fmt: on - run_and_assert(stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + run_and_assert(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) diff --git a/integration-tests/tests/utils/port_utils.py b/integration-tests/tests/utils/port_utils.py index cfb1b47c45..fd4f2047f0 100644 --- a/integration-tests/tests/utils/port_utils.py +++ b/integration-tests/tests/utils/port_utils.py @@ -160,7 +160,7 @@ def _validate_port_range_bounds(port_range: range) -> None: required_range_str = _format_port_range(port_range) valid_range_str = _format_port_range(VALID_PORT_RANGE) err_msg = ( - f"The port range derived from --base-port ({required_range_str}) must fall within" + f"The port range derived from '--base-port' ({required_range_str}) must fall within" f" the range of valid ports ({valid_range_str})." ) raise ValueError(err_msg) From 0562f48bebfa0ab6623a6d31a39bc7c989fce6f8 Mon Sep 17 00:00:00 2001 From: Quinn Date: Fri, 19 Dec 2025 19:47:46 +0000 Subject: [PATCH 2/8] Clean up implementation. --- integration-tests/.pytest.ini | 5 +- integration-tests/pyproject.toml | 3 + integration-tests/tests/conftest.py | 32 ++++++++-- .../tests/fixtures/package_config.py | 6 +- .../tests/fixtures/package_instance.py | 4 +- .../tests/test_identity_transformation.py | 14 +++-- .../tests/utils/asserting_utils.py | 60 ++++++++++--------- .../tests/utils/clp_mode_utils.py | 8 +-- integration-tests/tests/utils/config.py | 4 ++ .../tests/utils/logging_utils.py | 14 +++++ .../tests/utils/package_utils.py | 27 ++++++++- integration-tests/tests/utils/port_utils.py | 7 +++ 12 files changed, 133 insertions(+), 51 deletions(-) create mode 100644 integration-tests/tests/utils/logging_utils.py diff --git a/integration-tests/.pytest.ini b/integration-tests/.pytest.ini index ccf9ad3530..62ffeb8e51 100644 --- a/integration-tests/.pytest.ini +++ b/integration-tests/.pytest.ini @@ -13,12 +13,13 @@ env = log_cli = True log_cli_level = INFO -log_cli_format = %(levelname)-10s %(asctime)-25s %(message)s +log_cli_format = %(levelname)-8s %(asctime)-25s %(message)s log_cli_date_format = %Y-%m-%d %H:%M:%S -log_auto_indent = 37 log_file_mode = a log_file_level = DEBUG +log_file_format = %(levelname)-8s %(asctime)-30s [%(name)s:%(filename)s:%(lineno)d]: %(message)s +log_file_date_format = %Y-%m-%d %H:%M:%S,%f markers = clp: mark tests that use the CLP storage engine diff --git a/integration-tests/pyproject.toml b/integration-tests/pyproject.toml index d6177ef0ec..c710bda62d 100644 --- a/integration-tests/pyproject.toml +++ b/integration-tests/pyproject.toml @@ -43,6 +43,9 @@ show_error_end = true [tool.ruff] extend = "../tools/yscope-dev-utils/exports/lint-configs/python/ruff.toml" +[tool.ruff.lint] +ignore = ["TRY400"] + [tool.uv] default-groups = ["clp", "dev"] diff --git a/integration-tests/tests/conftest.py b/integration-tests/tests/conftest.py index 2e4d7574bf..056da37366 100644 --- a/integration-tests/tests/conftest.py +++ b/integration-tests/tests/conftest.py @@ -6,6 +6,8 @@ import pytest +from tests.utils.logging_utils import BLUE, BOLD, RESET + # Make the fixtures defined in `tests/fixtures/` globally available without imports. pytest_plugins = [ "tests.fixtures.integration_test_logs", @@ -29,7 +31,7 @@ def pytest_addoption(parser: pytest.Parser) -> None: ) test_run_id = str(uuid.uuid4())[-4:] - log_file_name = Path("__pytest_testrun_logs") / f"testrun_{test_run_id}.log" + log_file_name = Path("__pytest_logs") / f"testrun_{test_run_id}.log" parser.addini( "log_file_path", help="Path to the log file for this test.", @@ -37,18 +39,36 @@ def pytest_addoption(parser: pytest.Parser) -> None: default=log_file_name, ) + +def pytest_itemcollected(item: pytest.Item) -> None: + """Prettify the name of the test for output purposes.""" + item._nodeid = f"{BOLD}{BLUE}Running test: {item.name}{RESET}" # noqa: SLF001 + + @pytest.hookimpl(tryfirst=True) -def pytest_report_header(config) -> str: - """Docstring""" +def pytest_report_header(config: pytest.Config) -> str: + """ + Adds a field to the header at the start of the test run. + + :param config: + """ log_file_path = Path(config.getini("log_file_path")).expanduser().resolve() return f"Log file path for this test run: {log_file_path}" -@pytest.hookimpl(hookwrapper=True,tryfirst=True) -def pytest_runtest_setup(item)-> Iterator[None]: - """Docstring""" +@pytest.hookimpl(hookwrapper=True, tryfirst=True) +def pytest_runtest_setup(item: pytest.Item) -> Iterator[None]: + """ + Sets the output file for the logger to the log file path for this test run. + + :param item: + """ config = item.config logging_plugin = config.pluginmanager.get_plugin("logging-plugin") + if logging_plugin is None: + err_msg = "Expected pytest plugin 'logging-plugin' to be registered." + raise RuntimeError(err_msg) + log_file_path = Path(config.getini("log_file_path")) logging_plugin.set_log_path(str(log_file_path)) yield diff --git a/integration-tests/tests/fixtures/package_config.py b/integration-tests/tests/fixtures/package_config.py index be0d44b21f..88e4f76384 100644 --- a/integration-tests/tests/fixtures/package_config.py +++ b/integration-tests/tests/fixtures/package_config.py @@ -10,6 +10,7 @@ get_required_component_list, ) from tests.utils.config import PackageConfig, PackagePathConfig +from tests.utils.logging_utils import construct_log_err_msg from tests.utils.port_utils import assign_ports_from_base logger = logging.getLogger(__name__) @@ -30,18 +31,21 @@ def fixt_package_config( mode_name: str = request.param logger.info("Setting up the '%s' package...", mode_name) + logger.debug("Getting the ClpConfig object for the '%s' package...", mode_name) clp_config_obj = get_clp_config_from_mode(mode_name) # Assign ports based on the clp base port CLI option. + logger.debug("Assigning ports to the components in the '%s' package...", mode_name) base_port_string = request.config.getoption("--base-port") try: base_port = int(base_port_string) except ValueError as err: err_msg = f"Invalid value '{base_port_string}' for '--base-port'; expected an integer." - logger.error(err_msg) + logger.error(construct_log_err_msg(err_msg)) raise ValueError(err_msg) from err assign_ports_from_base(base_port, clp_config_obj) + logger.debug("Constructing the list of required components for the '%s' package...", mode_name) required_components = get_required_component_list(clp_config_obj) # Construct PackageConfig. diff --git a/integration-tests/tests/fixtures/package_instance.py b/integration-tests/tests/fixtures/package_instance.py index 5af4e36d53..b2c9f85278 100644 --- a/integration-tests/tests/fixtures/package_instance.py +++ b/integration-tests/tests/fixtures/package_instance.py @@ -18,7 +18,9 @@ @pytest.fixture -def fixt_package_instance(request: pytest.FixtureRequest, fixt_package_config: PackageConfig) -> Iterator[PackageInstance]: +def fixt_package_instance( + request: pytest.FixtureRequest, fixt_package_config: PackageConfig +) -> Iterator[PackageInstance]: """ Starts a CLP package instance for the given configuration and stops it during teardown. diff --git a/integration-tests/tests/test_identity_transformation.py b/integration-tests/tests/test_identity_transformation.py index 9d95f357a8..ae96092e1e 100644 --- a/integration-tests/tests/test_identity_transformation.py +++ b/integration-tests/tests/test_identity_transformation.py @@ -73,10 +73,10 @@ def test_clp_identity_transform( src_path, ] # fmt: on - run_and_assert(compression_cmd) + run_and_assert(request, compression_cmd) decompression_cmd = [bin_path, "x", compression_path, decompression_path] - run_and_assert(decompression_cmd) + run_and_assert(request, decompression_cmd) input_path = test_paths.logs_source_dir output_path = test_paths.decompression_dir @@ -113,7 +113,7 @@ def test_clp_s_identity_transform( logs_source_dir=integration_test_logs.extraction_dir, integration_test_path_config=integration_test_path_config, ) - _clp_s_compress_and_decompress(clp_core_path_config, test_paths) + _clp_s_compress_and_decompress(request, clp_core_path_config, test_paths) # Recompress the decompressed output that's consolidated into a single json file, and decompress # it again to verify consistency. The compression input of the second iteration points to the @@ -126,7 +126,7 @@ def test_clp_s_identity_transform( logs_source_dir=test_paths.decompression_dir, integration_test_path_config=integration_test_path_config, ) - _clp_s_compress_and_decompress(clp_core_path_config, consolidated_json_test_paths) + _clp_s_compress_and_decompress(request, clp_core_path_config, consolidated_json_test_paths) _consolidated_json_file_name = "original" input_path = consolidated_json_test_paths.logs_source_dir / _consolidated_json_file_name @@ -140,6 +140,7 @@ def test_clp_s_identity_transform( def _clp_s_compress_and_decompress( + request: pytest.FixtureRequest, clp_core_path_config: ClpCorePathConfig, test_paths: CompressionTestPathConfig, ) -> None: @@ -148,5 +149,6 @@ def _clp_s_compress_and_decompress( src_path = str(test_paths.logs_source_dir) compression_path = str(test_paths.compression_dir) decompression_path = str(test_paths.decompression_dir) - run_and_assert([bin_path, "c", compression_path, src_path]) - run_and_assert([bin_path, "x", compression_path, decompression_path]) + + run_and_assert(request, [bin_path, "c", compression_path, src_path]) + run_and_assert(request, [bin_path, "x", compression_path, decompression_path]) diff --git a/integration-tests/tests/utils/asserting_utils.py b/integration-tests/tests/utils/asserting_utils.py index 3f66e54627..a0bf457bec 100644 --- a/integration-tests/tests/utils/asserting_utils.py +++ b/integration-tests/tests/utils/asserting_utils.py @@ -12,12 +12,15 @@ from tests.utils.clp_mode_utils import compare_mode_signatures from tests.utils.config import PackageInstance from tests.utils.docker_utils import list_running_services_in_compose_project +from tests.utils.logging_utils import construct_log_err_msg from tests.utils.utils import load_yaml_to_dict logger = logging.getLogger(__name__) -def run_and_assert(request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[Any]: +def run_and_assert( + request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any +) -> subprocess.CompletedProcess[Any]: """ Runs a command with subprocess and asserts that it succeeds with pytest. @@ -27,27 +30,20 @@ def run_and_assert(request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any :raise: pytest.fail if the command exits with a non-zero return code. """ log_file_path = Path(request.config.getini("log_file_path")) - with open(log_file_path, "ab") as log_file: + with log_file_path.open("ab") as log_file: try: - proc = subprocess.run( - cmd, - stdout=log_file, - stderr=log_file, - check=True, - **kwargs - ) - except subprocess.CalledProcessError: - fail_msg = ( - "Called process failed. Check error summary at the end of the test for more information." - ) - logger.error(fail_msg) - pytest.fail(fail_msg) - except subprocess.TimeoutExpired: - fail_msg = ( - "Called process timed out. Check error summary at the end of the test for more information." - ) - logger.error(fail_msg) - pytest.fail(fail_msg) + log_debug_msg = f"Now running command: {cmd}" + logger.debug(log_debug_msg) + proc = subprocess.run(cmd, stdout=log_file, stderr=log_file, check=True, **kwargs) + except subprocess.CalledProcessError as error: + err_msg = f"Called process failed: {error}" + logger.debug(err_msg) + pytest.fail(err_msg) + except subprocess.TimeoutExpired as error: + err_msg = f"Called process timed out: {error}" + logger.debug(err_msg) + pytest.fail(err_msg) + return proc @@ -69,20 +65,22 @@ def validate_package_running(package_instance: PackageInstance) -> None: # Compare with list of required components. required_components = set(package_instance.package_config.component_list) + if required_components == running_services: return - cmd_fail_msg = "Component mismatch." + err_msg = f"Component validation failed for the {mode_name} package test." missing_components = required_components - running_services if missing_components: - cmd_fail_msg += f"\nMissing components: {missing_components}." + err_msg += f" Missing components: {missing_components}." unexpected_components = running_services - required_components if unexpected_components: - cmd_fail_msg += f"\nUnexpected services: {unexpected_components}." + err_msg += f" Unexpected services: {unexpected_components}." - pytest.fail(cmd_fail_msg) + logger.error(construct_log_err_msg(err_msg)) + pytest.fail(err_msg) def validate_running_mode_correct(package_instance: PackageInstance) -> None: @@ -97,15 +95,21 @@ def validate_running_mode_correct(package_instance: PackageInstance) -> None: :raise pytest.fail: if the running ClpConfig does not match the intended ClpConfig. """ mode_name = package_instance.package_config.mode_name - logger.info("Validating that the '%s' package is running in the correct configuration...", mode_name) + logger.info( + "Validating that the '%s' package is running in the correct configuration...", mode_name + ) shared_config_dict = load_yaml_to_dict(package_instance.shared_config_file_path) try: running_config = ClpConfig.model_validate(shared_config_dict) except ValidationError as err: - pytest.fail(f"Shared config failed validation: {err}") + err_msg = f"The shared config file could not be validated: {err}" + logger.error(construct_log_err_msg(err_msg)) + pytest.fail(err_msg) intended_config = package_instance.package_config.clp_config if not compare_mode_signatures(intended_config, running_config): - pytest.fail("Mode mismatch: running configuration does not match intended configuration.") + err_msg = f"Mode validation failed for the {mode_name} package test." + logger.error(construct_log_err_msg(err_msg)) + pytest.fail(err_msg) diff --git a/integration-tests/tests/utils/clp_mode_utils.py b/integration-tests/tests/utils/clp_mode_utils.py index dbdad6e01d..1d4a17090c 100644 --- a/integration-tests/tests/utils/clp_mode_utils.py +++ b/integration-tests/tests/utils/clp_mode_utils.py @@ -24,6 +24,8 @@ ) from pydantic import BaseModel +from tests.utils.logging_utils import construct_log_err_msg + logger = logging.getLogger(__name__) @@ -98,11 +100,7 @@ def get_clp_config_from_mode(mode_name: str) -> ClpConfig: """ if mode_name not in CLP_MODE_CONFIGS: err_msg = f"Unsupported mode: '{mode_name}'" - err_log_msg = ( - err_msg + - "\nCheck the full output log for more information (see test header for file path)." - ) - logger.error(err_log_msg) + logger.error(construct_log_err_msg(err_msg)) raise ValueError(err_msg) return CLP_MODE_CONFIGS[mode_name]() diff --git a/integration-tests/tests/utils/config.py b/integration-tests/tests/utils/config.py index ee9fc94a5a..2599ac9683 100644 --- a/integration-tests/tests/utils/config.py +++ b/integration-tests/tests/utils/config.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import re from dataclasses import dataclass, field, InitVar from pathlib import Path @@ -19,6 +20,8 @@ validate_file_exists, ) +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class ClpCorePathConfig: @@ -150,6 +153,7 @@ def temp_config_file_path(self) -> Path: def _write_temp_config_file(self) -> None: """Writes the temporary config file for this package.""" + logger.debug("Writing the config file for the '%s' package...", self.mode_name) temp_config_file_path = self.temp_config_file_path payload = self.clp_config.dump_to_primitive_dict() # type: ignore[no-untyped-call] diff --git a/integration-tests/tests/utils/logging_utils.py b/integration-tests/tests/utils/logging_utils.py new file mode 100644 index 0000000000..3f7d3180f7 --- /dev/null +++ b/integration-tests/tests/utils/logging_utils.py @@ -0,0 +1,14 @@ +"""Utilities for logging during the test run.""" + +BLUE = "\033[34m" +BOLD = "\033[1m" +GREEN = "\033[32m" +RESET = "\033[0m" + + +def construct_log_err_msg(err_msg: str) -> str: + """Append a signal that directs readers to the test output log file.""" + return ( + err_msg + + " Check the full test output log for more information (see test header for file path)." + ) diff --git a/integration-tests/tests/utils/package_utils.py b/integration-tests/tests/utils/package_utils.py index ff76d57f73..5aa80cb4c4 100644 --- a/integration-tests/tests/utils/package_utils.py +++ b/integration-tests/tests/utils/package_utils.py @@ -1,11 +1,20 @@ """Provides utility functions related to the CLP package used across `integration-tests`.""" + +import logging +from subprocess import SubprocessError + import pytest from tests.utils.asserting_utils import run_and_assert from tests.utils.config import PackageConfig +from tests.utils.logging_utils import construct_log_err_msg + +logger = logging.getLogger(__name__) + DEFAULT_CMD_TIMEOUT_SECONDS = 120.0 + def start_clp_package(request: pytest.FixtureRequest, package_config: PackageConfig) -> None: """ Starts an instance of the CLP package. @@ -23,7 +32,14 @@ def start_clp_package(request: pytest.FixtureRequest, package_config: PackageCon "--config", str(temp_config_file_path), ] # fmt: on - run_and_assert(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + + try: + run_and_assert(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + except SubprocessError: + mode_name = package_config.mode_name + err_msg = f"The {mode_name} package failed to start." + logger.error(construct_log_err_msg(err_msg)) + pytest.fail(err_msg) def stop_clp_package(request: pytest.FixtureRequest, package_config: PackageConfig) -> None: @@ -43,4 +59,11 @@ def stop_clp_package(request: pytest.FixtureRequest, package_config: PackageConf "--config", str(temp_config_file_path), ] # fmt: on - run_and_assert(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + + try: + run_and_assert(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + except SubprocessError: + mode_name = package_config.mode_name + err_msg = f"The {mode_name} package failed to stop." + logger.error(construct_log_err_msg(err_msg)) + pytest.fail(err_msg) diff --git a/integration-tests/tests/utils/port_utils.py b/integration-tests/tests/utils/port_utils.py index fd4f2047f0..c9086fc89c 100644 --- a/integration-tests/tests/utils/port_utils.py +++ b/integration-tests/tests/utils/port_utils.py @@ -1,5 +1,6 @@ """Functions for facilitating the port connections for the CLP package.""" +import logging import socket from dataclasses import dataclass from typing import Any @@ -9,6 +10,10 @@ REDUCER_COMPONENT_NAME, ) +from tests.utils.logging_utils import construct_log_err_msg + +logger = logging.getLogger(__name__) + # Port constants. MIN_NON_PRIVILEGED_PORT = 1024 MAX_PORT = 65535 @@ -76,6 +81,7 @@ def _check_ports_available(host: str, port_range: range) -> None: f"Port '{port}' in the desired range ({range_str}) is already in use. " "Choose a different port range for the test environment." ) + logger.error(construct_log_err_msg(err_msg)) raise ValueError(err_msg) @@ -163,4 +169,5 @@ def _validate_port_range_bounds(port_range: range) -> None: f"The port range derived from '--base-port' ({required_range_str}) must fall within" f" the range of valid ports ({valid_range_str})." ) + logger.error(construct_log_err_msg(err_msg)) raise ValueError(err_msg) From 6da4264a117dce3fed0f88863e91b45a5537be8c Mon Sep 17 00:00:00 2001 From: Quinn Date: Tue, 6 Jan 2026 16:04:56 +0000 Subject: [PATCH 3/8] Address Junhao's comments. --- integration-tests/.pytest.ini | 2 +- .../tests/utils/asserting_utils.py | 27 +++++-------------- .../tests/utils/package_utils.py | 2 ++ integration-tests/tests/utils/port_utils.py | 2 ++ 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/integration-tests/.pytest.ini b/integration-tests/.pytest.ini index 62ffeb8e51..04149ddfb1 100644 --- a/integration-tests/.pytest.ini +++ b/integration-tests/.pytest.ini @@ -3,9 +3,9 @@ addopts = --code-highlight=yes --color=yes --show-capture=no - --tb=short --strict-config --strict-markers + --tb=short env = D:CLP_BUILD_DIR=../build D:CLP_CORE_BINS_DIR=../build/core diff --git a/integration-tests/tests/utils/asserting_utils.py b/integration-tests/tests/utils/asserting_utils.py index a0bf457bec..4f130bd7df 100644 --- a/integration-tests/tests/utils/asserting_utils.py +++ b/integration-tests/tests/utils/asserting_utils.py @@ -18,33 +18,20 @@ logger = logging.getLogger(__name__) -def run_and_assert( - request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any -) -> subprocess.CompletedProcess[Any]: +def run_and_assert(request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any) -> None: """ - Runs a command with subprocess and asserts that it succeeds with pytest. + Runs a command with subprocess. + :param request: Pytest fixture request. :param cmd: Command and arguments to execute. :param kwargs: Additional keyword arguments passed through to the subprocess. - :return: The completed process object, for inspection or further handling. - :raise: pytest.fail if the command exits with a non-zero return code. + :raise: Propagates `subprocess.run`'s errors. """ log_file_path = Path(request.config.getini("log_file_path")) with log_file_path.open("ab") as log_file: - try: - log_debug_msg = f"Now running command: {cmd}" - logger.debug(log_debug_msg) - proc = subprocess.run(cmd, stdout=log_file, stderr=log_file, check=True, **kwargs) - except subprocess.CalledProcessError as error: - err_msg = f"Called process failed: {error}" - logger.debug(err_msg) - pytest.fail(err_msg) - except subprocess.TimeoutExpired as error: - err_msg = f"Called process timed out: {error}" - logger.debug(err_msg) - pytest.fail(err_msg) - - return proc + log_debug_msg = f"Now running command: {cmd}" + logger.debug(log_debug_msg) + subprocess.run(cmd, stdout=log_file, stderr=log_file, check=True, **kwargs) def validate_package_running(package_instance: PackageInstance) -> None: diff --git a/integration-tests/tests/utils/package_utils.py b/integration-tests/tests/utils/package_utils.py index 5aa80cb4c4..355b33ae54 100644 --- a/integration-tests/tests/utils/package_utils.py +++ b/integration-tests/tests/utils/package_utils.py @@ -19,6 +19,7 @@ def start_clp_package(request: pytest.FixtureRequest, package_config: PackageCon """ Starts an instance of the CLP package. + :param request: :param package_config: :raise: Propagates `run_and_assert`'s errors. """ @@ -46,6 +47,7 @@ def stop_clp_package(request: pytest.FixtureRequest, package_config: PackageConf """ Stops the running instance of the CLP package. + :param request: :param package_config: :raise: Propagates `run_and_assert`'s errors. """ diff --git a/integration-tests/tests/utils/port_utils.py b/integration-tests/tests/utils/port_utils.py index c9086fc89c..6c8da3fab2 100644 --- a/integration-tests/tests/utils/port_utils.py +++ b/integration-tests/tests/utils/port_utils.py @@ -65,6 +65,8 @@ def assign_ports_from_base(base_port: int, clp_config: ClpConfig) -> None: setattr(assignment.component, assignment.attr_name, current_port) current_port += assignment.port_count + clp_config.database.port = 3306 + def _check_ports_available(host: str, port_range: range) -> None: """ From f5ec1b53008e68082693b4bd3ebb80fddb63a13f Mon Sep 17 00:00:00 2001 From: Quinn Date: Tue, 6 Jan 2026 16:11:09 +0000 Subject: [PATCH 4/8] Remove erroneous line. --- integration-tests/tests/utils/port_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/integration-tests/tests/utils/port_utils.py b/integration-tests/tests/utils/port_utils.py index 6c8da3fab2..c9086fc89c 100644 --- a/integration-tests/tests/utils/port_utils.py +++ b/integration-tests/tests/utils/port_utils.py @@ -65,8 +65,6 @@ def assign_ports_from_base(base_port: int, clp_config: ClpConfig) -> None: setattr(assignment.component, assignment.attr_name, current_port) current_port += assignment.port_count - clp_config.database.port = 3306 - def _check_ports_available(host: str, port_range: range) -> None: """ From d9dc252f9ee1456cc08c17bbe122833debbc4ee2 Mon Sep 17 00:00:00 2001 From: Quinn Date: Sat, 31 Jan 2026 16:11:41 +0000 Subject: [PATCH 5/8] Lint. --- .../tests/fixtures/package_instance.py | 10 ++++---- .../tests/utils/asserting_utils.py | 2 +- .../tests/utils/package_utils.py | 23 +++++++++++++++---- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/integration-tests/tests/fixtures/package_instance.py b/integration-tests/tests/fixtures/package_instance.py index 1689235da9..7837e2a207 100644 --- a/integration-tests/tests/fixtures/package_instance.py +++ b/integration-tests/tests/fixtures/package_instance.py @@ -18,7 +18,9 @@ @pytest.fixture(scope="module") -def fixt_package_instance(fixt_package_test_config: PackageTestConfig) -> Iterator[PackageInstance]: +def fixt_package_instance( + request: pytest.FixtureRequest, fixt_package_test_config: PackageTestConfig +) -> Iterator[PackageInstance]: """ Starts a CLP package instance for the given configuration and stops it during teardown. @@ -30,10 +32,10 @@ def fixt_package_instance(fixt_package_test_config: PackageTestConfig) -> Iterat """ mode_config = fixt_package_test_config.mode_config mode_name = mode_config.mode_name - + try: logger.info("Starting the '%s' package...", mode_name) - start_clp_package(fixt_package_test_config) + start_clp_package(request, fixt_package_test_config) instance = PackageInstance(package_test_config=fixt_package_test_config) yield instance except RuntimeError: @@ -45,4 +47,4 @@ def fixt_package_instance(fixt_package_test_config: PackageTestConfig) -> Iterat ) finally: logger.info("Stopping the '%s' package...", mode_name) - stop_clp_package(fixt_package_test_config) + stop_clp_package(request, fixt_package_test_config) diff --git a/integration-tests/tests/utils/asserting_utils.py b/integration-tests/tests/utils/asserting_utils.py index 69201ca3f9..d93e7bb068 100644 --- a/integration-tests/tests/utils/asserting_utils.py +++ b/integration-tests/tests/utils/asserting_utils.py @@ -57,7 +57,7 @@ def _validate_package_running(package_instance: PackageInstance) -> None: :param package_instance: :raise pytest.fail: if the sets of running services and required components do not match. """ - mode_name = package_instance.package_config.mode_name + mode_name = package_instance.package_test_config.mode_config.mode_name logger.info("Validating that all components of the '%s' package are running...", mode_name) # Get list of services currently running in the Compose project. diff --git a/integration-tests/tests/utils/package_utils.py b/integration-tests/tests/utils/package_utils.py index 87daddbaa9..9fd26cc346 100644 --- a/integration-tests/tests/utils/package_utils.py +++ b/integration-tests/tests/utils/package_utils.py @@ -7,11 +7,17 @@ from tests.utils.asserting_utils import run_and_assert from tests.utils.config import PackageTestConfig +from tests.utils.logging_utils import construct_log_err_msg + +logger = logging.getLogger(__name__) + DEFAULT_CMD_TIMEOUT_SECONDS = 120.0 -def start_clp_package(package_test_config: PackageTestConfig) -> None: +def start_clp_package( + request: pytest.FixtureRequest, package_test_config: PackageTestConfig +) -> None: """ Starts an instance of the CLP package. @@ -28,10 +34,19 @@ def start_clp_package(package_test_config: PackageTestConfig) -> None: "--config", str(temp_config_file_path), ] # fmt: on - run_and_assert(start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + + try: + run_and_assert(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + except SubprocessError: + mode_name = package_test_config.mode_config.mode_name + err_msg = f"The {mode_name} package failed to start." + logger.error(construct_log_err_msg(err_msg)) + pytest.fail(err_msg) -def stop_clp_package(package_test_config: PackageTestConfig) -> None: +def stop_clp_package( + request: pytest.FixtureRequest, package_test_config: PackageTestConfig +) -> None: """ Stops the running instance of the CLP package. @@ -52,7 +67,7 @@ def stop_clp_package(package_test_config: PackageTestConfig) -> None: try: run_and_assert(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) except SubprocessError: - mode_name = package_config.mode_name + mode_name = package_test_config.mode_config.mode_name err_msg = f"The {mode_name} package failed to stop." logger.error(construct_log_err_msg(err_msg)) pytest.fail(err_msg) From 43f65aacb3cce687136f41cedb1fb756380ca83c Mon Sep 17 00:00:00 2001 From: Quinn Date: Sat, 31 Jan 2026 18:14:01 +0000 Subject: [PATCH 6/8] Adjust after merging 1843. --- integration-tests/.pytest.ini | 7 +++--- integration-tests/tests/conftest.py | 22 ++++++++++------- .../tests/fixtures/package_instance.py | 7 ------ .../tests/fixtures/package_test_config.py | 1 + .../package_tests/clp_json/test_clp_json.py | 24 +++++-------------- .../package_tests/clp_text/test_clp_text.py | 18 +++++++------- .../tests/utils/asserting_utils.py | 8 +++++-- .../tests/utils/package_utils.py | 4 ++-- 8 files changed, 41 insertions(+), 50 deletions(-) diff --git a/integration-tests/.pytest.ini b/integration-tests/.pytest.ini index 54da8a787a..9b9f60cc85 100644 --- a/integration-tests/.pytest.ini +++ b/integration-tests/.pytest.ini @@ -6,6 +6,7 @@ addopts = --strict-config --strict-markers --tb=short + env = D:CLP_BUILD_DIR=../build D:CLP_CORE_BINS_DIR=../build/core @@ -13,13 +14,13 @@ env = log_cli = True log_cli_level = INFO -log_cli_format = %(levelname)-8s %(asctime)-25s %(message)s +log_cli_format = %(asctime)s.%(msecs)03d %(levelname)s %(message)s log_cli_date_format = %Y-%m-%d %H:%M:%S log_file_mode = a log_file_level = DEBUG -log_file_format = %(levelname)-8s %(asctime)-30s [%(name)s:%(filename)s:%(lineno)d]: %(message)s -log_file_date_format = %Y-%m-%d %H:%M:%S,%f +log_file_format = %(asctime)s.%(msecs)03d %(levelname)s [%(name)s:%(filename)s:%(lineno)d]: %(message)s +log_file_date_format = %Y-%m-%d %H:%M:%S markers = clp: mark tests that use the CLP storage engine diff --git a/integration-tests/tests/conftest.py b/integration-tests/tests/conftest.py index f59804639d..9b4537f290 100644 --- a/integration-tests/tests/conftest.py +++ b/integration-tests/tests/conftest.py @@ -19,7 +19,7 @@ def pytest_addoption(parser: pytest.Parser) -> None: """ - Adds options for pytest. + Adds options for `pytest`. :param parser: """ @@ -30,25 +30,31 @@ def pytest_addoption(parser: pytest.Parser) -> None: help="Base port for CLP package integration tests.", ) + # Sets up a unique log file for this test run, and stores the path to the file. test_run_id = str(uuid.uuid4())[-4:] - log_file_name = Path("__pytest_logs") / f"testrun_{test_run_id}.log" + log_file_path = Path("__pytest_logs") / f"testrun_{test_run_id}.log" parser.addini( "log_file_path", help="Path to the log file for this test.", type="paths", - default=log_file_name, + default=log_file_path, ) def pytest_itemcollected(item: pytest.Item) -> None: - """Prettify the name of the test for output purposes.""" - item._nodeid = f"{BOLD}{BLUE}Running test: {item.name}{RESET}" # noqa: SLF001 + """ + Prettifies the name of the test for output purposes. + + :param item: + """ + item._nodeid = f"{BOLD}{BLUE}{item.name}{RESET}" # noqa: SLF001 @pytest.hookimpl(tryfirst=True) def pytest_report_header(config: pytest.Config) -> str: """ - Adds a field to the header at the start of the test run. + Adds a field to the header at the start of the test run that reports the path to the log file + for this test run. :param config: """ @@ -56,10 +62,10 @@ def pytest_report_header(config: pytest.Config) -> str: return f"Log file path for this test run: {log_file_path}" -@pytest.hookimpl(hookwrapper=True, tryfirst=True) +@pytest.hookimpl(wrapper=True) def pytest_runtest_setup(item: pytest.Item) -> Iterator[None]: """ - Sets the output file for the logger to the log file path for this test run. + Sets `log_file_path` as the output file for the logger. :param item: """ diff --git a/integration-tests/tests/fixtures/package_instance.py b/integration-tests/tests/fixtures/package_instance.py index 7837e2a207..f346aaccc8 100644 --- a/integration-tests/tests/fixtures/package_instance.py +++ b/integration-tests/tests/fixtures/package_instance.py @@ -38,13 +38,6 @@ def fixt_package_instance( start_clp_package(request, fixt_package_test_config) instance = PackageInstance(package_test_config=fixt_package_test_config) yield instance - except RuntimeError: - base_port = fixt_package_test_config.base_port - pytest.fail( - f"Failed to start the {mode_name} package. This could mean that one of the ports" - f" derived from base_port={base_port} was unavailable. You can specify a new value for" - " base_port with the '--base-port' flag." - ) finally: logger.info("Stopping the '%s' package...", mode_name) stop_clp_package(request, fixt_package_test_config) diff --git a/integration-tests/tests/fixtures/package_test_config.py b/integration-tests/tests/fixtures/package_test_config.py index aeca1cfd13..fc0f1ccbb0 100644 --- a/integration-tests/tests/fixtures/package_test_config.py +++ b/integration-tests/tests/fixtures/package_test_config.py @@ -28,6 +28,7 @@ def fixt_package_test_config( mode_config: PackageModeConfig = request.param mode_name = mode_config.mode_name clp_config_obj = mode_config.clp_config + logger.info("Setting up the '%s' package...", mode_name) # Assign ports based on the clp base port CLI option. diff --git a/integration-tests/tests/package_tests/clp_json/test_clp_json.py b/integration-tests/tests/package_tests/clp_json/test_clp_json.py index d3c48af72f..a3c779e7ac 100644 --- a/integration-tests/tests/package_tests/clp_json/test_clp_json.py +++ b/integration-tests/tests/package_tests/clp_json/test_clp_json.py @@ -1,7 +1,5 @@ """Tests for the clp-json package.""" -import logging - import pytest from clp_py_utils.clp_config import ( ClpConfig, @@ -16,9 +14,6 @@ from tests.utils.clp_mode_utils import CLP_API_SERVER_COMPONENT, CLP_BASE_COMPONENTS from tests.utils.config import PackageInstance, PackageModeConfig -logger = logging.getLogger(__name__) - - # Mode description for this module. CLP_JSON_MODE = PackageModeConfig( mode_name="clp-json", @@ -36,27 +31,26 @@ pytestmark = [ pytest.mark.package, pytest.mark.clp_json, - pytest.mark.parametrize("fixt_package_test_config", [CLP_JSON_MODE], indirect=True), + pytest.mark.parametrize( + "fixt_package_test_config", [CLP_JSON_MODE], indirect=True, ids=[CLP_JSON_MODE.mode_name] + ), ] @pytest.mark.startup def test_clp_json_startup(fixt_package_instance: PackageInstance) -> None: """ - Validate that the `clp-json` package starts up successfully. + Validates that the `clp-json` package starts up successfully. :param fixt_package_instance: """ validate_package_instance(fixt_package_instance) - log_msg = "test_clp_json_startup was successful." - logger.info(log_msg) - @pytest.mark.compression def test_clp_json_compression(fixt_package_instance: PackageInstance) -> None: """ - Validate that the `clp-json` package successfully compresses some dataset. + Validates that the `clp-json` package successfully compresses some dataset. :param fixt_package_instance: """ @@ -65,14 +59,11 @@ def test_clp_json_compression(fixt_package_instance: PackageInstance) -> None: # TODO: compress some dataset and check the correctness of compression. assert True - log_msg = "test_clp_json_compression was successful." - logger.info(log_msg) - @pytest.mark.search def test_clp_json_search(fixt_package_instance: PackageInstance) -> None: """ - Validate that the `clp-json` package successfully searches some dataset. + Validates that the `clp-json` package successfully searches some dataset. :param fixt_package_instance: """ @@ -83,6 +74,3 @@ def test_clp_json_search(fixt_package_instance: PackageInstance) -> None: # TODO: search through that dataset and check the correctness of the search results. assert True - - log_msg = "test_clp_json_search was successful." - logger.info(log_msg) diff --git a/integration-tests/tests/package_tests/clp_text/test_clp_text.py b/integration-tests/tests/package_tests/clp_text/test_clp_text.py index dbbc862f4c..076b8d9f5a 100644 --- a/integration-tests/tests/package_tests/clp_text/test_clp_text.py +++ b/integration-tests/tests/package_tests/clp_text/test_clp_text.py @@ -1,7 +1,5 @@ """Tests for the clp-text package.""" -import logging - import pytest from clp_py_utils.clp_config import ( ClpConfig, @@ -16,9 +14,6 @@ from tests.utils.clp_mode_utils import CLP_BASE_COMPONENTS from tests.utils.config import PackageInstance, PackageModeConfig -logger = logging.getLogger(__name__) - - # Mode description for this module. CLP_TEXT_MODE = PackageModeConfig( mode_name="clp-text", @@ -38,14 +33,17 @@ pytestmark = [ pytest.mark.package, pytest.mark.clp_text, - pytest.mark.parametrize("fixt_package_test_config", [CLP_TEXT_MODE], indirect=True), + pytest.mark.parametrize( + "fixt_package_test_config", [CLP_TEXT_MODE], indirect=True, ids=[CLP_TEXT_MODE.mode_name] + ), ] @pytest.mark.startup def test_clp_text_startup(fixt_package_instance: PackageInstance) -> None: - """Tests package startup.""" - validate_package_instance(fixt_package_instance) + """ + Validates that the `clp-text` package starts up successfully. - log_msg = "test_clp_text_startup was successful." - logger.info(log_msg) + :param fixt_package_instance: + """ + validate_package_instance(fixt_package_instance) diff --git a/integration-tests/tests/utils/asserting_utils.py b/integration-tests/tests/utils/asserting_utils.py index d93e7bb068..d6b0f06fce 100644 --- a/integration-tests/tests/utils/asserting_utils.py +++ b/integration-tests/tests/utils/asserting_utils.py @@ -42,6 +42,9 @@ def validate_package_instance(package_instance: PackageInstance) -> None: :param package_instance: """ + mode_name = package_instance.package_test_config.mode_config.mode_name + logger.info("Validating that the '%s' package is running correctly...", mode_name) + # Ensure that all package components are running. _validate_package_running(package_instance) @@ -58,7 +61,7 @@ def _validate_package_running(package_instance: PackageInstance) -> None: :raise pytest.fail: if the sets of running services and required components do not match. """ mode_name = package_instance.package_test_config.mode_config.mode_name - logger.info("Validating that all components of the '%s' package are running...", mode_name) + logger.debug("Validating that all components of the '%s' package are running...", mode_name) # Get list of services currently running in the Compose project. instance_id = package_instance.clp_instance_id @@ -70,6 +73,7 @@ def _validate_package_running(package_instance: PackageInstance) -> None: if required_components == running_services: return + # Construct error message. err_msg = f"Component validation failed for the {mode_name} package test." missing_components = required_components - running_services @@ -96,7 +100,7 @@ def _validate_running_mode_correct(package_instance: PackageInstance) -> None: :raise pytest.fail: if the running ClpConfig does not match the intended ClpConfig. """ mode_name = package_instance.package_test_config.mode_config.mode_name - logger.info( + logger.debug( "Validating that the '%s' package is running in the correct configuration...", mode_name ) diff --git a/integration-tests/tests/utils/package_utils.py b/integration-tests/tests/utils/package_utils.py index 9fd26cc346..20b8185161 100644 --- a/integration-tests/tests/utils/package_utils.py +++ b/integration-tests/tests/utils/package_utils.py @@ -39,7 +39,7 @@ def start_clp_package( run_and_assert(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) except SubprocessError: mode_name = package_test_config.mode_config.mode_name - err_msg = f"The {mode_name} package failed to start." + err_msg = f"The '{mode_name}' package failed to start." logger.error(construct_log_err_msg(err_msg)) pytest.fail(err_msg) @@ -68,6 +68,6 @@ def stop_clp_package( run_and_assert(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) except SubprocessError: mode_name = package_test_config.mode_config.mode_name - err_msg = f"The {mode_name} package failed to stop." + err_msg = f"The '{mode_name}' package failed to stop." logger.error(construct_log_err_msg(err_msg)) pytest.fail(err_msg) From 1c8992e9a31f694406252cb25d02d4aaa9469e12 Mon Sep 17 00:00:00 2001 From: Quinn Date: Sat, 31 Jan 2026 18:15:55 +0000 Subject: [PATCH 7/8] Add newline. --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 09d150ce95..d988d97268 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,4 @@ __pycache__/ .task/ build/ dist/ -__* \ No newline at end of file +__* From 72a95d6cb16e3a9b99b8d46c6cef481d589b13e1 Mon Sep 17 00:00:00 2001 From: Quinn Date: Thu, 5 Feb 2026 20:44:00 +0000 Subject: [PATCH 8/8] Address Junhao's comments. --- .gitignore | 1 - integration-tests/tests/conftest.py | 20 +++++++++++++------ .../tests/test_identity_transformation.py | 10 +++++----- .../tests/utils/asserting_utils.py | 2 +- .../tests/utils/logging_utils.py | 9 +++++++-- .../tests/utils/package_utils.py | 16 ++++++++------- 6 files changed, 36 insertions(+), 22 deletions(-) diff --git a/.gitignore b/.gitignore index d988d97268..41da100695 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,3 @@ __pycache__/ .task/ build/ dist/ -__* diff --git a/integration-tests/tests/conftest.py b/integration-tests/tests/conftest.py index 9b4537f290..9e5caf48d7 100644 --- a/integration-tests/tests/conftest.py +++ b/integration-tests/tests/conftest.py @@ -1,12 +1,13 @@ """Global pytest setup.""" -import uuid +import datetime from collections.abc import Iterator from pathlib import Path import pytest from tests.utils.logging_utils import BLUE, BOLD, RESET +from tests.utils.utils import resolve_path_env_var # Make the fixtures defined in `tests/fixtures/` globally available without imports. pytest_plugins = [ @@ -17,6 +18,7 @@ ] +@pytest.hookimpl() def pytest_addoption(parser: pytest.Parser) -> None: """ Adds options for `pytest`. @@ -31,13 +33,19 @@ def pytest_addoption(parser: pytest.Parser) -> None: ) # Sets up a unique log file for this test run, and stores the path to the file. - test_run_id = str(uuid.uuid4())[-4:] - log_file_path = Path("__pytest_logs") / f"testrun_{test_run_id}.log" + now = datetime.datetime.now() # noqa: DTZ005 + test_run_id = now.strftime("%Y-%m-%d-%H-%M-%S") + log_file_path = ( + resolve_path_env_var("CLP_BUILD_DIR") + / "integration-tests" + / "test_logs" + / f"testrun_{test_run_id}.log" + ) parser.addini( "log_file_path", help="Path to the log file for this test.", - type="paths", - default=log_file_path, + type="string", + default=str(log_file_path), ) @@ -47,7 +55,7 @@ def pytest_itemcollected(item: pytest.Item) -> None: :param item: """ - item._nodeid = f"{BOLD}{BLUE}{item.name}{RESET}" # noqa: SLF001 + item._nodeid = f"{BOLD}{BLUE}{item.nodeid}{RESET}" # noqa: SLF001 @pytest.hookimpl(tryfirst=True) diff --git a/integration-tests/tests/test_identity_transformation.py b/integration-tests/tests/test_identity_transformation.py index ae96092e1e..b2e1c4501a 100644 --- a/integration-tests/tests/test_identity_transformation.py +++ b/integration-tests/tests/test_identity_transformation.py @@ -5,7 +5,7 @@ import pytest -from tests.utils.asserting_utils import run_and_assert +from tests.utils.asserting_utils import run_and_log_to_file from tests.utils.config import ( ClpCorePathConfig, CompressionTestPathConfig, @@ -73,10 +73,10 @@ def test_clp_identity_transform( src_path, ] # fmt: on - run_and_assert(request, compression_cmd) + run_and_log_to_file(request, compression_cmd) decompression_cmd = [bin_path, "x", compression_path, decompression_path] - run_and_assert(request, decompression_cmd) + run_and_log_to_file(request, decompression_cmd) input_path = test_paths.logs_source_dir output_path = test_paths.decompression_dir @@ -150,5 +150,5 @@ def _clp_s_compress_and_decompress( compression_path = str(test_paths.compression_dir) decompression_path = str(test_paths.decompression_dir) - run_and_assert(request, [bin_path, "c", compression_path, src_path]) - run_and_assert(request, [bin_path, "x", compression_path, decompression_path]) + run_and_log_to_file(request, [bin_path, "c", compression_path, src_path]) + run_and_log_to_file(request, [bin_path, "x", compression_path, decompression_path]) diff --git a/integration-tests/tests/utils/asserting_utils.py b/integration-tests/tests/utils/asserting_utils.py index d6b0f06fce..cbdd399fc1 100644 --- a/integration-tests/tests/utils/asserting_utils.py +++ b/integration-tests/tests/utils/asserting_utils.py @@ -18,7 +18,7 @@ logger = logging.getLogger(__name__) -def run_and_assert(request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any) -> None: +def run_and_log_to_file(request: pytest.FixtureRequest, cmd: list[str], **kwargs: Any) -> None: """ Runs a command with subprocess. diff --git a/integration-tests/tests/utils/logging_utils.py b/integration-tests/tests/utils/logging_utils.py index 3f7d3180f7..46570e14ce 100644 --- a/integration-tests/tests/utils/logging_utils.py +++ b/integration-tests/tests/utils/logging_utils.py @@ -2,12 +2,17 @@ BLUE = "\033[34m" BOLD = "\033[1m" -GREEN = "\033[32m" RESET = "\033[0m" def construct_log_err_msg(err_msg: str) -> str: - """Append a signal that directs readers to the test output log file.""" + """ + Append a signal that directs readers to the test output log file. + + :param err_msg: The base error message onto which the signal will be appended. + :return: An error message that directs readers to look in the test output log file. + + """ return ( err_msg + " Check the full test output log for more information (see test header for file path)." diff --git a/integration-tests/tests/utils/package_utils.py b/integration-tests/tests/utils/package_utils.py index 20b8185161..18629463ce 100644 --- a/integration-tests/tests/utils/package_utils.py +++ b/integration-tests/tests/utils/package_utils.py @@ -5,7 +5,7 @@ import pytest -from tests.utils.asserting_utils import run_and_assert +from tests.utils.asserting_utils import run_and_log_to_file from tests.utils.config import PackageTestConfig from tests.utils.logging_utils import construct_log_err_msg @@ -21,8 +21,9 @@ def start_clp_package( """ Starts an instance of the CLP package. + :param request: Pytest fixture request. :param package_test_config: - :raise: Propagates `run_and_assert`'s errors. + :raise: Propagates `run_and_log_to_file`'s errors. """ path_config = package_test_config.path_config start_script_path = path_config.start_script_path @@ -36,8 +37,8 @@ def start_clp_package( # fmt: on try: - run_and_assert(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) - except SubprocessError: + run_and_log_to_file(request, start_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + except (SubprocessError, OSError): mode_name = package_test_config.mode_config.mode_name err_msg = f"The '{mode_name}' package failed to start." logger.error(construct_log_err_msg(err_msg)) @@ -50,8 +51,9 @@ def stop_clp_package( """ Stops the running instance of the CLP package. + :param request: Pytest fixture request. :param package_test_config: - :raise: Propagates `run_and_assert`'s errors. + :raise: Propagates `run_and_log_to_file`'s errors. """ path_config = package_test_config.path_config stop_script_path = path_config.stop_script_path @@ -65,8 +67,8 @@ def stop_clp_package( # fmt: on try: - run_and_assert(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) - except SubprocessError: + run_and_log_to_file(request, stop_cmd, timeout=DEFAULT_CMD_TIMEOUT_SECONDS) + except (SubprocessError, OSError): mode_name = package_test_config.mode_config.mode_name err_msg = f"The '{mode_name}' package failed to stop." logger.error(construct_log_err_msg(err_msg))