From d46315ba4f92d5868585730ef3ca44808aca0f8b Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 07:45:59 +0300 Subject: [PATCH 01/13] WIP --- conftest.py | 89 +++++ poetry.lock | 18 +- pyproject.toml | 13 + tests/llm/conftest.py | 549 ++++++++++++++++++++++++----- tests/llm/test_ask_holmes.py | 83 +++-- tests/llm/utils/commands.py | 122 ++++++- tests/llm/utils/mock_toolset.py | 23 +- tests/llm/utils/system.py | 30 +- tests/llm/utils/test_case_utils.py | 8 +- tests/llm/utils/test_helpers.py | 77 ++++ 10 files changed, 869 insertions(+), 143 deletions(-) create mode 100644 conftest.py create mode 100644 tests/llm/utils/test_helpers.py diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000000..eec7c85ae9 --- /dev/null +++ b/conftest.py @@ -0,0 +1,89 @@ +import logging +from tests.llm.conftest import xxxxxpytest_terminal_summary2 + + +# Register the same options that the LLM conftest expects +def pytest_addoption(parser): + """Add custom pytest command line options""" + parser.addoption( + "--generate-mocks", + action="store_true", + default=False, + help="Generate mock data files during test execution instead of using existing mocks", + ) + parser.addoption( + "--regenerate-all-mocks", + action="store_true", + default=False, + help="Regenerate all mock data files, replacing existing ones (implies --generate-mocks)", + ) + parser.addoption( + "--skip-setup", + action="store_true", + default=False, + help="Skip running before_test commands for test cases (useful for iterative test development)", + ) + parser.addoption( + "--skip-cleanup", + action="store_true", + default=False, + help="Skip running after_test commands for test cases (useful for debugging test failures)", + ) + + +# reexport the pytest_terminal_summary2 function to be used in the main conftest.py +pytest_terminal_summary = xxxxxpytest_terminal_summary2 + + +def pytest_configure(config): + """Configure pytest settings""" + # Configure worker-specific log files for xdist compatibility + # worker_id = getattr(config, "workerinput", {}).get("workerid", "master") + # if worker_id != "master": + # # Set worker-specific log file to avoid conflicts + # config.option.log_file = f"tests-{worker_id}.log" + + # Determine worker id + # Also see: https://pytest-xdist.readthedocs.io/en/latest/how-to.html#creating-one-log-file-for-each-worker + # worker_id = os.environ.get("PYTEST_XDIST_WORKER", default="gw0") + + # # Create logs folder + # logs_folder = os.environ.get("LOGS_FOLDER", default="logs_folder") + # os.makedirs(logs_folder, exist_ok=True) + + # # Create file handler to output logs into corresponding worker file + # file_handler = logging.FileHandler(f"{logs_folder}/logs_worker_{worker_id}.log", mode="w") + # file_handler.setFormatter( + # logging.Formatter( + # fmt="{asctime} {levelname}:{name}:{lineno}:{message}", + # style="{", + # ) + # ) + # # Create stream handler to output logs on console + # # This is a workaround for a known limitation: + # # https://pytest-xdist.readthedocs.io/en/latest/known-limitations.html + # console_handler = logging.StreamHandler(sys.stderr) # pytest only prints error logs + # console_handler.setFormatter( + # logging.Formatter( + # # Include worker id in log messages, \r is needed to separate lines in console + # fmt="\r{asctime} " + worker_id + ":{levelname}:{name}:{lineno}:{message}", + # style="{", + # ) + # ) + # # Configure logging + # logging.basicConfig(level=logging.INFO, force=True, handlers=[console_handler, file_handler]) + + # Suppress noisy LiteLLM logs during testing + logging.getLogger("LiteLLM").setLevel(logging.ERROR) + # Also suppress the verbose logger used by LiteLLM + logging.getLogger("LiteLLM.verbose_logger").setLevel(logging.ERROR) + # Suppress litellm sub-loggers + logging.getLogger("litellm").setLevel(logging.ERROR) + logging.getLogger("litellm.cost_calculator").setLevel(logging.ERROR) + logging.getLogger("litellm.litellm_core_utils").setLevel(logging.ERROR) + logging.getLogger("litellm.litellm_core_utils.litellm_logging").setLevel( + logging.ERROR + ) + # Suppress httpx HTTP request logs + logging.getLogger("httpx").setLevel(logging.WARNING) + logging.getLogger("httpcore").setLevel(logging.WARNING) diff --git a/poetry.lock b/poetry.lock index cbf6128a68..31e2baca17 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3167,6 +3167,22 @@ pytest = ">=6.2.5" [package.extras] dev = ["pre-commit", "pytest-asyncio", "tox"] +[[package]] +name = "pytest-shared-session-scope" +version = "0.4.0" +description = "Pytest session-scoped fixture that works with xdist" +optional = false +python-versions = ">=3.10" +files = [ + {file = "pytest_shared_session_scope-0.4.0-py3-none-any.whl", hash = "sha256:583508332f0ffdf306fb50487893e4bd4f893caf21778b7b0ea9fad6767fecce"}, + {file = "pytest_shared_session_scope-0.4.0.tar.gz", hash = "sha256:30da6ced4c734bb7cdbc10310da754ca9c8ae75c1384ceb3eda82fa76db647d2"}, +] + +[package.dependencies] +filelock = ">=3.16.0" +pytest = ">=7.0.0" +typing-extensions = ">=3.6.2" + [[package]] name = "pytest-xdist" version = "3.6.1" @@ -4597,4 +4613,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "e46c01cfb23840ad9e4279210c97c08c6f9c1e21918ceca416828a72eb7c69d9" +content-hash = "dd05b6f4bc7d9c875fae14230df8b3170f03d8c0d387293dd4c9731b2e036382" diff --git a/pyproject.toml b/pyproject.toml index 03b8e5a1ca..32f0f7e437 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,6 +47,7 @@ python_multipart = "^0.0.18" kubernetes = "^32.0.1" mcp = "v1.9.0" prompt-toolkit = "^3.0.51" +pytest-shared-session-scope = "^0.4.0" azure-identity = "^1.23.0" azure-core = "^1.34.0" requests = "^2.32.4" @@ -106,6 +107,18 @@ addopts = [ "--durations=5", # Show 5 slowest tests after each run ] +# Logging configuration for pytest +log_cli = true +log_cli_level = "INFO" +log_cli_format = "%(asctime)s [%(levelname)8s] [%(name)s] %(message)s" +log_cli_date_format = "%Y-%m-%d %H:%M:%S" + +# File logging +log_file = "tests.log" +log_file_level = "INFO" # Changed from DEBUG to reduce noise in HTML report +log_file_format = "%(asctime)s [%(levelname)8s] [%(name)s] %(message)s" +log_file_date_format = "%Y-%m-%d %H:%M:%S" + [tool.coverage.run] branch = true omit = [ diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index 9c2552c145..6cb14356a7 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -1,31 +1,42 @@ -# Standard library imports import logging import os import textwrap +import warnings from contextlib import contextmanager from dataclasses import dataclass from typing import List, Optional -# Third-party imports import pytest from litellm import completion from rich.console import Console from rich.table import Table +from pytest_shared_session_scope import ( + shared_session_scope_json, + SetupToken, + CleanupToken, +) -# Local imports from tests.llm.utils.braintrust import get_experiment_name from tests.llm.utils.constants import PROJECT from tests.llm.utils.classifiers import create_llm_client -from tests.llm.utils.mock_toolset import MockMode # type: ignore[attr-defined] +from tests.llm.utils.mock_toolset import MockMode, MockGenerationConfig # type: ignore[attr-defined] # Configuration constants DEBUG_SEPARATOR = "=" * 80 LLM_TEST_TYPES = ["test_ask_holmes", "test_investigate", "test_workload_health"] +MAX_ERROR_LINES = 10 +MAX_WORKERS = 30 def is_llm_test(nodeid: str) -> bool: """Check if a test nodeid is for an LLM test.""" - return any(test_type in nodeid for test_type in LLM_TEST_TYPES) + return any( + [ + "test_ask_holmes" in nodeid, + "test_investigate" in nodeid, + "test_workload_health" in nodeid, + ] + ) # Status determination types @@ -85,6 +96,7 @@ def short_status(self) -> str: @pytest.fixture(scope="session") def mock_generation_config(request): """Session-scoped fixture that provides mock generation configuration and mode.""" + # Safely get options with defaults in case they're not registered generate_mocks = request.config.getoption("--generate-mocks") regenerate_all_mocks = request.config.getoption("--regenerate-all-mocks") @@ -93,22 +105,111 @@ def mock_generation_config(request): generate_mocks = True # Determine mode based on environment and options - if os.environ.get("RUN_LIVE"): + if os.getenv("RUN_LIVE", "False").lower() in ("true", "1", "t"): mode = MockMode.LIVE elif generate_mocks: mode = MockMode.GENERATE else: mode = MockMode.MOCK - class MockGenerationConfig: - def __init__(self, generate_mocks_enabled, regenerate_all_enabled, mock_mode): - self.generate_mocks = generate_mocks_enabled - self.regenerate_all_mocks = regenerate_all_enabled - self.mode = mock_mode - return MockGenerationConfig(generate_mocks, regenerate_all_mocks, mode) +# Handles before_test and after_test +# see https://github.com/StefanBRas/pytest-shared-session-scope +@shared_session_scope_json() +def shared_test_infrastructure(request, mock_generation_config: MockGenerationConfig): + """Shared session-scoped fixture for test infrastructure setup/cleanup coordination""" + collect_only = request.config.getoption("--collect-only") + + # If we're in collect-only mode or RUN_LIVE is not set, skip setup/cleanup entirely + if collect_only or mock_generation_config.mode == MockMode.MOCK: + print( + f"Skipping shared test infrastructure setup/cleanup (mode: {mock_generation_config.mode}, collect_only: {collect_only})" + ) + # Must yield twice even when skipping due to ohw pytest-shared-session-scope works + initial = yield + cleanup_token = yield {"test_cases_for_cleanup": []} + return + print( + f"Running shared test infrastructure setup/cleanup (mode: {mock_generation_config.mode}, collect_only: {collect_only})" + ) + + # First yield: get initial value (SetupToken.FIRST if first worker, data if subsequent) + initial = yield + + if initial is SetupToken.FIRST: + # This is the first worker to run the fixture + test_cases = _extract_test_cases_needing_setup(request.session) + + # Clear mock directories if --regenerate-all-mocks is set + cleared_directories = [] + regenerate_all = request.config.getoption("--regenerate-all-mocks") + + if regenerate_all: + cleared_directories = _clear_mock_directories(request.session) + + # Run setup unless --skip-setup is set + # Check skip-setup option + skip_setup = request.config.getoption("--skip-setup") + + if test_cases and not skip_setup: + _run_test_setup(test_cases) + elif skip_setup: + print("\nโญ๏ธ Skipping test setup due to --skip-setup flag") + + data = { + "test_cases_for_cleanup": [tc.id for tc in test_cases], + "cleared_mock_directories": cleared_directories, + } + else: + # This is a worker using the fixture after the first worker + data = initial + + # Actual test runs here when we yield - then we get back a cleanup token from pytest-shared-session-scope + cleanup_token = yield data + + if cleanup_token is CleanupToken.LAST: + # This is the last worker to exit - responsible for cleanup + test_case_ids = data.get("test_cases_for_cleanup", []) + + # Check skip-cleanup option + skip_cleanup = request.config.getoption("--skip-cleanup") + + if test_case_ids and not skip_cleanup: + # Reconstruct test cases from IDs + from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] # type: ignore[attr-defined] + + cleanup_test_cases = [] + + for item in request.session.items: + if ( + item.get_closest_marker("llm") + and hasattr(item, "callspec") + and "test_case" in item.callspec.params + ): + test_case = item.callspec.params["test_case"] + if ( + isinstance(test_case, HolmesTestCase) + and test_case.id in test_case_ids + and test_case not in cleanup_test_cases + ): + cleanup_test_cases.append(test_case) + + if cleanup_test_cases: + _run_test_cleanup(cleanup_test_cases) + elif skip_cleanup: + print("\nโญ๏ธ Skipping test cleanup due to --skip-cleanup flag") + + +@pytest.fixture(scope="session", autouse=True) +def test_infrastructure_coordination(shared_test_infrastructure): + """Ensure the shared test infrastructure fixture is used (triggers setup/cleanup)""" + # This fixture just ensures shared_test_infrastructure runs for all sessions + # All the actual logic is in shared_test_infrastructure + yield + + @dataclass class TestResult: nodeid: str @@ -153,28 +254,6 @@ def test_name(self) -> str: return self.nodeid.split("::")[-1] if "::" in self.nodeid else self.nodeid -def pytest_addoption(parser): - """Add custom pytest command line options""" - parser.addoption( - "--generate-mocks", - action="store_true", - default=False, - help="Generate mock data files during test execution instead of using existing mocks", - ) - parser.addoption( - "--regenerate-all-mocks", - action="store_true", - default=False, - help="Regenerate all mock data files, replacing existing ones (implies --generate-mocks)", - ) - - -def pytest_configure(config): - """Configure pytest settings""" - # Suppress noisy LiteLLM logs during testing - logging.getLogger("LiteLLM").setLevel(logging.WARNING) - - @contextmanager def force_pytest_output(request): """Context manager to force output display even when pytest captures stdout""" @@ -213,10 +292,13 @@ def check_llm_api_with_test_call(): @pytest.fixture(scope="session", autouse=True) -def llm_session_setup(request): +def llm_availablity_check(request): """Handle LLM test session setup: show warning, check API, and skip if needed""" # Don't show messages during collection-only mode - if request.config.getoption("--collect-only"): + # Check if we're in collect-only mode + collect_only = request.config.getoption("--collect-only") + + if collect_only: return # Check if LLM marker is being excluded @@ -332,6 +414,7 @@ def braintrust_eval_link(request): root_span_id = value # Construct Braintrust URL for this specific test + # NATAN - this link is correct braintrust_url = get_braintrust_url( test_suite, temp_result.test_id, temp_result.test_name, span_id, root_span_id ) @@ -341,11 +424,35 @@ def braintrust_eval_link(request): print() -def pytest_terminal_summary(terminalreporter, exitstatus, config): +def _safe_print(terminalreporter, message=""): + """Safely print to terminal reporter to avoid I/O errors""" + try: + terminalreporter.write_line(message) + except Exception: + # If write_line fails, try direct write + try: + terminalreporter._tw.write(message + "\n") + except Exception: + # Last resort - ignore if all writing fails + pass + + +def xxxxxpytest_terminal_summary2(terminalreporter, exitstatus, config): """Generate GitHub Actions report and Rich summary table from terminalreporter.stats (xdist compatible)""" if not hasattr(terminalreporter, "stats"): return + # When using xdist, only the master process should display the summary + # Check if we're in a worker process + worker_id = ( + getattr(config, "workerinput", {}).get("workerid", None) + if hasattr(config, "workerinput") + else None + ) + if worker_id is not None: + # We're in a worker process, don't display summary + return + # Collect and sort test results from terminalreporter.stats sorted_results, mock_tracking_data = _collect_test_results_from_stats( terminalreporter @@ -358,10 +465,10 @@ def pytest_terminal_summary(terminalreporter, exitstatus, config): _handle_github_output(sorted_results) # Handle console/developer output (Rich table + Braintrust links) - _handle_console_output(sorted_results) + _handle_console_output(sorted_results, terminalreporter) # Report mock operation statistics - _report_mock_operations(config, mock_tracking_data) + _report_mock_operations(config, mock_tracking_data, terminalreporter) def markdown_table(headers, rows): @@ -654,7 +761,7 @@ def _handle_github_output(sorted_results): file.write(f"{total_regressions}") -def _handle_console_output(sorted_results): +def _handle_console_output(sorted_results, terminalreporter=None): """Display Rich table and Braintrust links for developers.""" if not sorted_results: return @@ -735,28 +842,9 @@ def _handle_console_output(sorted_results): analysis, ) + # Use force_terminal to ensure output is displayed even when captured console.print(table) - # Print Braintrust links if enabled - if os.environ.get("BRAINTRUST_API_KEY"): - print("๐Ÿ” BRAINTRUST EVAL LINKS:") - for result in sorted_results: - test_suite_full = ( - "ask_holmes" if result["test_type"] == "ask" else "investigate" - ) - braintrust_url = get_braintrust_url( - test_suite_full, - result["test_id"], - result["test_name"], - result.get("braintrust_span_id"), - result.get("braintrust_root_span_id"), - ) - if braintrust_url: - print( - f"* {result['test_id']}_{result['test_name']} " - f"({result['test_type']}) - {braintrust_url}" - ) - def _get_analysis_for_result(result): """Get analysis text for a test result, with proper text wrapping.""" @@ -819,35 +907,46 @@ def _get_llm_analysis(result: TestResult) -> str: return f"Analysis failed: {e}" -def _report_mock_operations(config, mock_tracking_data): +def _report_mock_operations(config, mock_tracking_data, terminalreporter=None): """Report mock file operations and statistics.""" - if not config.getoption("--generate-mocks") and not config.getoption( - "--regenerate-all-mocks" - ): + # Use default parameter to safely handle missing options + generate_mocks = False + regenerate_all_mocks = False + + try: + generate_mocks = config.getoption("--generate-mocks", default=False) + regenerate_all_mocks = config.getoption("--regenerate-all-mocks", default=False) + except (AttributeError, ValueError): + # Options not available, use defaults + pass + + if not generate_mocks and not regenerate_all_mocks: return - regenerate_mode = config.getoption("--regenerate-all-mocks") + regenerate_mode = regenerate_all_mocks generated_mocks = mock_tracking_data["generated_mocks"] - cleared_dirs = mock_tracking_data["cleared_directories"] mock_failures = mock_tracking_data["mock_failures"] + # If no terminalreporter, skip output + if not terminalreporter: + return + # Header - print(f"\n{'=' * 80}") - print( - f"{'๐Ÿ”„ MOCK REGENERATION SUMMARY' if regenerate_mode else '๐Ÿ”ง MOCK GENERATION SUMMARY'}" + _safe_print(terminalreporter, f"\n{'=' * 80}") + _safe_print( + terminalreporter, + f"{'๐Ÿ”„ MOCK REGENERATION SUMMARY' if regenerate_mode else '๐Ÿ”ง MOCK GENERATION SUMMARY'}", ) - print(f"{'=' * 80}") + _safe_print(terminalreporter, f"{'=' * 80}") - # Cleared directories - if cleared_dirs: - print(f"๐Ÿงน Cleared existing mocks from {len(cleared_dirs)} test directories:") - for dir_name in sorted(cleared_dirs): - print(f" - {dir_name}") - print() + # Note: Cleared directories are now handled by shared_test_infrastructure fixture + # and reported during setup phase to ensure single execution across workers # Generated mocks if generated_mocks: - print(f"โœ… Generated {len(generated_mocks)} mock files:\n") + _safe_print( + terminalreporter, f"โœ… Generated {len(generated_mocks)} mock files:\n" + ) # Group by test case by_test_case = {} @@ -860,20 +959,25 @@ def _report_mock_operations(config, mock_tracking_data): ) for test_case, mock_files in sorted(by_test_case.items()): - print(f"๐Ÿ“ {test_case}:") + _safe_print(terminalreporter, f"๐Ÿ“ {test_case}:") for mock_file in mock_files: - print(f" - {mock_file}") - print() + _safe_print(terminalreporter, f" - {mock_file}") + _safe_print(terminalreporter) else: mode_text = "regeneration" if regenerate_mode else "generation" - print(f"โœ… Mock {mode_text} was enabled but no new mock files were created") + _safe_print( + terminalreporter, + f"โœ… Mock {mode_text} was enabled but no new mock files were created", + ) # Failures if mock_failures: - print(f"โš ๏ธ {len(mock_failures)} mock-related failures occurred:") + _safe_print( + terminalreporter, f"โš ๏ธ {len(mock_failures)} mock-related failures occurred:" + ) for failure in mock_failures: - print(f" - {failure}") - print() + _safe_print(terminalreporter, f" - {failure}") + _safe_print(terminalreporter) # Checklist checklist = [ @@ -885,7 +989,278 @@ def _report_mock_operations(config, mock_tracking_data): "If pod/resource names change across tool calls, regenerate ALL mocks with --regenerate-all-mocks", ] - print("๐Ÿ“‹ REVIEW CHECKLIST:") + _safe_print(terminalreporter, "๐Ÿ“‹ REVIEW CHECKLIST:") for item in checklist: - print(f" โ–ก {item}") - print("=" * 80) + _safe_print(terminalreporter, f" โ–ก {item}") + _safe_print(terminalreporter, "=" * 80) + + +def _format_error_output(error_details: str) -> str: + """Format error details with truncation if needed""" + from tests.llm.utils.test_helpers import truncate_output + + return truncate_output(error_details, max_lines=MAX_ERROR_LINES) + + +def _run_test_setup(test_cases): + """Run before_test for each test case in parallel""" + from tests.llm.utils.commands import before_test + from concurrent.futures import ThreadPoolExecutor, as_completed + import time + + print(f"Setting up infrastructure for {len(test_cases)} test cases") + + start_time = time.time() + successful_test_cases = 0 + failed_test_cases = 0 + timed_out_test_cases = 0 + + with ThreadPoolExecutor(max_workers=min(len(test_cases), MAX_WORKERS)) as executor: + # Submit all setup tasks + future_to_test_case = { + executor.submit(before_test, test_case): test_case + for test_case in test_cases + } + + # Wait for all tasks to complete and handle results + for future in as_completed(future_to_test_case): + test_case = future_to_test_case[future] + try: + result = future.result() # Single CommandResult for the test case + remaining_cases = ( + len(test_cases) + - successful_test_cases + - failed_test_cases + - timed_out_test_cases + ) + if result.success: + successful_test_cases += 1 + print( + f"โœ… Setup {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); setups remaining: {remaining_cases}" + ) + elif result.error_type == "timeout": + timed_out_test_cases += 1 + print( + f"โฐ Setup {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; setups remaining: {remaining_cases}" + ) + + # Show the exact command that timed out + truncated_error = _format_error_output(result.error_details) + print(textwrap.indent(truncated_error, " ")) + logging.error( + f"[{test_case.id}] Setup timeout: {result.error_details}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"Setup timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", + UserWarning, + stacklevel=2, + ) + else: + failed_test_cases += 1 + print( + f"โŒ Setup {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); setups remaining: {remaining_cases}" + ) + + # Limit error details to 10 lines and add proper formatting + truncated_error = _format_error_output(result.error_details) + print(textwrap.indent(truncated_error, " ")) + logging.error( + f"[{test_case.id}] Setup failed: {result.error_details}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"Setup failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", + UserWarning, + stacklevel=2, + ) + + except Exception as e: + failed_test_cases += 1 + print(f"โŒ Setup {test_case.id}: EXCEPTION - {e}") + logging.error(f"Setup exception for {test_case.id}: {str(e)}") + + # Emit warning to make it visible in pytest output + warnings.warn( + f"Setup exception for test {test_case.id}: {str(e)}", + UserWarning, + stacklevel=2, + ) + + elapsed_time = time.time() - start_time + print( + f"\n๐Ÿ• Setup completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" + ) + + +def _run_test_cleanup(test_cases): + """Run after_test for each test case in parallel""" + from tests.llm.utils.commands import after_test + from concurrent.futures import ThreadPoolExecutor, as_completed + import time + + print(f"Cleaning up infrastructure after tests for {len(test_cases)} test cases") + + start_time = time.time() + successful_test_cases = 0 + failed_test_cases = 0 + timed_out_test_cases = 0 + + with ThreadPoolExecutor(max_workers=min(len(test_cases), MAX_WORKERS)) as executor: + # Submit all cleanup tasks + future_to_test_case = { + executor.submit(after_test, test_case): test_case + for test_case in test_cases + } + + # Wait for all tasks to complete and handle results + for future in as_completed(future_to_test_case): + test_case = future_to_test_case[future] + try: + result = future.result() # Single CommandResult for the test case + remaining_cases = ( + len(test_cases) + - successful_test_cases + - failed_test_cases + - timed_out_test_cases + ) + + if result.success: + successful_test_cases += 1 + print( + f"โœ… Cleanup {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); cleanups remaining: {remaining_cases}" + ) + elif result.error_type == "timeout": + timed_out_test_cases += 1 + print( + f"โฐ Cleanup {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; cleanups remaining: {remaining_cases}" + ) + + # Show the exact command that timed out + truncated_error = _format_error_output(result.error_details) + print(textwrap.indent(truncated_error, " ")) + logging.error( + f"[{test_case.id}] Cleanup timeout: {result.error_details}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"Cleanup timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", + UserWarning, + stacklevel=2, + ) + else: + failed_test_cases += 1 + print( + f"โŒ Cleanup {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); cleanups remaining: {remaining_cases}" + ) + + # Limit error details to 10 lines and add proper formatting + truncated_error = _format_error_output(result.error_details) + print(textwrap.indent(truncated_error, " ")) + logging.error( + f"[{test_case.id}] Cleanup failed: {result.error_details}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"Cleanup failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", + UserWarning, + stacklevel=2, + ) + + except Exception as e: + failed_test_cases += 1 + print(f"โŒ Cleanup {test_case.id}: EXCEPTION - {e}") + logging.error(f"Cleanup exception for {test_case.id}: {str(e)}") + + # Emit warning to make it visible in pytest output + warnings.warn( + f"Cleanup exception for test {test_case.id}: {str(e)}", + UserWarning, + stacklevel=2, + ) + + elapsed_time = time.time() - start_time + print( + f"\n๐Ÿ• Cleanup completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" + ) + + +def _extract_test_cases_needing_setup(session): + """Extract unique test cases that need setup from session items""" + from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] + + seen_ids = set() + test_cases = [] + + for item in session.items: + if ( + item.get_closest_marker("llm") + and hasattr(item, "callspec") + and "test_case" in item.callspec.params + ): + test_case = item.callspec.params["test_case"] + if ( + isinstance(test_case, HolmesTestCase) + and test_case.before_test + and test_case.id not in seen_ids + ): + test_cases.append(test_case) + seen_ids.add(test_case.id) + + return test_cases + + +def _clear_mock_directories(session): + """Clear mock directories for all test cases when --regenerate-all-mocks is set""" + from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] + import glob + + print("\n๐Ÿงน Clearing mock files for --regenerate-all-mocks") + + cleared_directories = set() + total_files_removed = 0 + + # Extract all unique test case folders + test_folders = set() + for item in session.items: + if ( + item.get_closest_marker("llm") + and hasattr(item, "callspec") + and "test_case" in item.callspec.params + ): + test_case = item.callspec.params["test_case"] + if isinstance(test_case, HolmesTestCase): + test_folders.add(test_case.folder) + + # Clear mock files from each folder + for folder in test_folders: + patterns = [ + os.path.join(folder, "*.txt"), + os.path.join(folder, "*.json"), + ] + + folder_files_removed = 0 + for pattern in patterns: + for file_path in glob.glob(pattern): + try: + os.remove(file_path) + folder_files_removed += 1 + total_files_removed += 1 + except Exception as e: + logging.warning(f"Could not remove {file_path}: {e}") + + if folder_files_removed > 0: + cleared_directories.add(folder) + print( + f" โœ… Cleared {folder_files_removed} mock files from {os.path.basename(folder)}" + ) + + print( + f" ๐Ÿ“Š Total: Cleared {total_files_removed} files from {len(cleared_directories)} directories\n" + ) + + return list(cleared_directories) diff --git a/tests/llm/test_ask_holmes.py b/tests/llm/test_ask_holmes.py index 9cf7223131..e6f5565391 100644 --- a/tests/llm/test_ask_holmes.py +++ b/tests/llm/test_ask_holmes.py @@ -14,14 +14,24 @@ from holmes.core.tools_utils.tool_executor import ToolExecutor import tests.llm.utils.braintrust as braintrust_util from tests.llm.utils.classifiers import evaluate_correctness -from tests.llm.utils.commands import after_test, before_test, set_test_env_vars +from tests.llm.utils.commands import set_test_env_vars from tests.llm.utils.constants import PROJECT -from tests.llm.utils.mock_toolset import MockToolsetManager -from braintrust import SpanTypeAttribute +from tests.llm.utils.mock_toolset import ( + MockToolsetManager, + MockMode, + MockGenerationConfig, +) from tests.llm.utils.test_case_utils import AskHolmesTestCase, Evaluation, MockHelper from os import path from tests.llm.utils.tags import add_tags_to_eval from holmes.core.tracing import SpanType +from tests.llm.utils.test_helpers import ( + log_tool_calls_to_spans, + print_expected_output, + print_correctness_evaluation, + print_tool_calls_summary, + print_tool_calls_detailed, +) TEST_CASES_FOLDER = Path( path.abspath(path.join(path.dirname(__file__), "fixtures", "test_ask_holmes")) @@ -66,18 +76,40 @@ def test_ask_holmes( test_case: AskHolmesTestCase, caplog, request, - mock_generation_config, + mock_generation_config: MockGenerationConfig, + shared_test_infrastructure, # type: ignore ): - tracer = TracingFactory.create_tracer("braintrust", project=PROJECT) + print(f"\n๐Ÿงช TEST: {test_case.id}") + print(" CONFIGURATION:") + print( + f" โ€ข Mode: {'โšช๏ธ MOCKED' if mock_generation_config.mode == MockMode.MOCK else '๐Ÿ”ฅ LIVE'}, Generate Mocks: {mock_generation_config.generate_mocks}" + ) + print(f" โ€ข User Prompt: {test_case.user_prompt}") + print(f" โ€ข Expected Output: {test_case.expected_output}") + if test_case.before_test: + if "\n" in test_case.before_test: + print(" โ€ข Before Test:") + for line in test_case.before_test.strip().split("\n"): + print(f" {line}") + else: + print(f" โ€ข Before Test: {test_case.before_test}") + + if test_case.after_test: + if "\n" in test_case.after_test: + print(" โ€ข After Test:") + for line in test_case.after_test.strip().split("\n"): + print(f" {line}") + else: + print(f" โ€ข After Test: {test_case.after_test}") - # Create experiment using unified API + tracer = TracingFactory.create_tracer("braintrust", project=PROJECT) tracer.start_experiment( experiment_name=experiment_name, metadata=braintrust_util.get_machine_state_tags(), ) - # Create evaluation span and use as context manager result: Optional[LLMResult] = None + try: with tracer.start_trace( name=test_case.id, span_type=SpanType.TASK @@ -92,8 +124,7 @@ def test_ask_holmes( ("braintrust_root_span_id", str(eval_span.root_span_id)) ) - with eval_span.start_span("Before Test Setup", type=SpanTypeAttribute.TASK): - before_test(test_case) + # Setup is handled by session-scoped fixture now # Mock datetime if mocked_date is provided if test_case.mocked_date: @@ -123,6 +154,10 @@ def test_ask_holmes( request=request, ) + if result.tool_calls: + # Log tool calls to Braintrust spans + log_tool_calls_to_spans(result.tool_calls, eval_span) + except Exception as e: # Log error to span if available try: @@ -167,12 +202,12 @@ def test_ask_holmes( request.node.user_properties.append(("actual_correctness_score", 0)) request.node.user_properties.append(("mock_data_failure", True)) - after_test(test_case) + # Cleanup is handled by session-scoped fixture now raise finally: - with eval_span.start_span("After Test Teardown", type=SpanTypeAttribute.TASK): - after_test(test_case) + # Cleanup is handled by session-scoped fixture now + pass input = test_case.user_prompt output = result.result @@ -183,8 +218,7 @@ def test_ask_holmes( if not isinstance(expected, list): expected = [expected] - debug_expected = "\n- ".join(expected) - print(f"** EXPECTED **\n- {debug_expected}") + print_expected_output(expected) prompt = ( result.messages[0]["content"] @@ -203,9 +237,10 @@ def test_ask_holmes( evaluation_type=evaluation_type, caplog=caplog, ) - print( - f"\nCORRECTNESS:\nscore = {correctness_eval.score}\nRATIONALE:\n{correctness_eval.metadata.get('rationale', '')}" - ) + print("\n๐Ÿ’ฌ ACTUAL OUTPUT:") + print(f" {output}") + + print_correctness_evaluation(correctness_eval) scores["correctness"] = correctness_eval.score @@ -220,13 +255,16 @@ def test_ask_holmes( metadata={"system_prompt": prompt}, ) + # Print tool calls summary + print_tool_calls_summary(result.tool_calls) + if result.tool_calls: tools_called = [tc.description for tc in result.tool_calls] else: tools_called = "None" - print(f"\n** TOOLS CALLED **\n{tools_called}") - print(f"\n** OUTPUT **\n{output}") - print(f"\n** SCORES **\n{scores}") + + # Print detailed tool output + print_tool_calls_detailed(result.tool_calls) # Store data for summary plugin expected_correctness_score = ( @@ -234,6 +272,7 @@ def test_ask_holmes( if isinstance(test_case.evaluation.correctness, Evaluation) else test_case.evaluation.correctness ) + debug_expected = "\n- ".join(expected) request.node.user_properties.append(("expected", debug_expected)) request.node.user_properties.append(("actual", output or "")) request.node.user_properties.append( @@ -285,7 +324,9 @@ def ask_holmes( tool_executor = ToolExecutor(mock.enabled_toolsets) enabled_toolsets = [t.name for t in tool_executor.enabled_toolsets] - print(f"** ENABLED TOOLSETS **\n{', '.join(enabled_toolsets)}") + print( + f"\n๐Ÿ› ๏ธ ENABLED TOOLSETS ({len(enabled_toolsets)}):", ", ".join(enabled_toolsets) + ) ai = ToolCallingLLM( tool_executor=tool_executor, diff --git a/tests/llm/utils/commands.py b/tests/llm/utils/commands.py index b306a94f1f..13c00a8e71 100644 --- a/tests/llm/utils/commands.py +++ b/tests/llm/utils/commands.py @@ -2,11 +2,39 @@ import logging import os import subprocess +import time from contextlib import contextmanager from typing import Dict, Optional from tests.llm.utils.test_case_utils import HolmesTestCase +class CommandResult: + def __init__( + self, + command: str, + test_case_id: str, + success: bool, + exit_code: int = None, + elapsed_time: float = 0, + error_type: str = None, + error_details: str = None, + ): + self.command = command + self.test_case_id = test_case_id + self.success = success + self.exit_code = exit_code + self.elapsed_time = elapsed_time + self.error_type = error_type # 'timeout', 'failure', or None + self.error_details = error_details + + @property + def exit_info(self) -> str: + """Get formatted exit information.""" + return ( + f"exit {self.exit_code}" if self.exit_code is not None else "no exit code" + ) + + def invoke_command(command: str, cwd: str) -> str: try: logging.debug(f"Running `{command}` in {cwd}") @@ -30,24 +58,94 @@ def invoke_command(command: str, cwd: str) -> str: raise e -def before_test(test_case: HolmesTestCase): - if test_case.before_test and os.environ.get("RUN_LIVE", "").strip().lower() in ( +def _run_commands( + test_case: HolmesTestCase, commands_str: str, operation: str +) -> CommandResult: + """Generic command runner for setup/cleanup operations.""" + if not commands_str or os.environ.get("RUN_LIVE", "").strip().lower() not in ( "1", "true", ): - commands = test_case.before_test.split("\n") - for command in commands: - invoke_command(command=command, cwd=test_case.folder) + return CommandResult( + command=f"(no {operation} needed)", + test_case_id=test_case.id, + success=True, + elapsed_time=0, + ) + start_time = time.time() + commands = commands_str.strip().split("\n") + combined_output = [] -def after_test(test_case: HolmesTestCase): - if test_case.after_test and os.environ.get("RUN_LIVE", "").strip().lower() in ( - "1", - "true", - ): - commands = test_case.after_test.split("\n") + try: for command in commands: - invoke_command(command=command, cwd=test_case.folder) + if command.strip(): # Skip empty lines + output = invoke_command(command=command, cwd=test_case.folder) + combined_output.append(f"$ {command}\n{output}") + + elapsed_time = time.time() - start_time + return CommandResult( + command=f"{operation.capitalize()}: {len(commands)} command(s)", + test_case_id=test_case.id, + success=True, + elapsed_time=elapsed_time, + ) + except subprocess.CalledProcessError as e: + elapsed_time = time.time() - start_time + error_details = "\n".join(combined_output) + error_details += f"\n$ {e.cmd}\nExit code: {e.returncode}\nstdout:\n{e.stdout}\nstderr:\n{e.stderr}" + + return CommandResult( + command=f"{operation.capitalize()} failed at: {e.cmd}", + test_case_id=test_case.id, + success=False, + exit_code=e.returncode, + elapsed_time=elapsed_time, + error_type="failure", + error_details=error_details, + ) + except subprocess.TimeoutExpired as e: + elapsed_time = time.time() - start_time + error_details = "\n".join(combined_output) + error_details += f"\n$ {e.cmd}\nTIMEOUT after {e.timeout}s" + + return CommandResult( + command=f"{operation.capitalize()} timeout: {e.cmd}", + test_case_id=test_case.id, + success=False, + elapsed_time=elapsed_time, + error_type="timeout", + error_details=error_details, + ) + except Exception as e: + elapsed_time = time.time() - start_time + error_details = "\n".join(combined_output) + error_details += f"\nUnexpected error: {str(e)}" + + return CommandResult( + command=f"{operation.capitalize()} failed", + test_case_id=test_case.id, + success=False, + elapsed_time=elapsed_time, + error_type="failure", + error_details=error_details, + ) + + +def before_test(test_case: HolmesTestCase) -> CommandResult: + """Run before_test commands for a test case. + + Returns a CommandResult with success=True if no setup is needed or all commands succeed. + """ + return _run_commands(test_case, test_case.before_test, "setup") + + +def after_test(test_case: HolmesTestCase) -> CommandResult: + """Run after_test commands for a test case. + + Returns a CommandResult with success=True if no cleanup is needed or all commands succeed. + """ + return _run_commands(test_case, test_case.after_test, "cleanup") @contextmanager diff --git a/tests/llm/utils/mock_toolset.py b/tests/llm/utils/mock_toolset.py index f096f95c55..f92823d7f7 100644 --- a/tests/llm/utils/mock_toolset.py +++ b/tests/llm/utils/mock_toolset.py @@ -66,6 +66,13 @@ class MockMode(Enum): LIVE = "live" # Use real tools without mocking +class MockGenerationConfig: + def __init__(self, generate_mocks_enabled, regenerate_all_enabled, mock_mode): + self.generate_mocks = generate_mocks_enabled + self.regenerate_all_mocks = regenerate_all_enabled + self.mode = mock_mode + + class MockMetadata(BaseModel): """Metadata stored in mock files.""" @@ -281,7 +288,7 @@ def _load_all_mocks(self) -> Dict[str, List[ToolMock]]: raise MockDataCorruptedError( f"Mock file {file_path} is in old format and needs to be updated (see PR #372)", tool_name=metadata.get("tool_name", "unknown"), - ) + ) from None content = "".join(lines[2:]) if len(lines) > 2 else None if content is not None: @@ -434,9 +441,10 @@ def configure_toolsets( toolset.enabled = definition.enabled configured.append(toolset) - # Check prerequisites for enabled toolsets + # Check prerequisites for enabled toolsets with timeout if toolset.enabled: try: + # TODO: add timeout toolset.check_prerequisites() except Exception: logging.error( @@ -475,15 +483,8 @@ def __init__( self.file_manager = MockFileManager(test_case_folder, add_params_to_mock_file) self.configurator = ToolsetConfigurator() - # Clear mocks if regenerating - if mock_generation_config.regenerate_all_mocks: - if request and test_case_folder not in getattr( - mock_generation_config, "_cleared_folders", set() - ): - self.file_manager.clear_mocks(request) - if not hasattr(mock_generation_config, "_cleared_folders"): - mock_generation_config._cleared_folders = set() - mock_generation_config._cleared_folders.add(test_case_folder) + # Note: Mock clearing is now handled by the shared_test_infrastructure fixture + # to ensure it only happens once across all workers when using --regenerate-all-mocks # Load and configure toolsets self._initialize_toolsets() diff --git a/tests/llm/utils/system.py b/tests/llm/utils/system.py index fd8f321fb1..f378ce185a 100644 --- a/tests/llm/utils/system.py +++ b/tests/llm/utils/system.py @@ -8,13 +8,29 @@ def get_active_branch_name(): - head_dir = Path(".", ".git", "HEAD") - with head_dir.open("r") as f: - content = f.read().splitlines() - - for line in content: - if line[0:4] == "ref:": - return line.partition("refs/heads/")[2] + try: + # First check if .git is a file (worktree case) + git_path = Path(".git") + if git_path.is_file(): + # Read the worktree git directory path + with git_path.open("r") as f: + content = f.read().strip() + if content.startswith("gitdir:"): + worktree_git_dir = Path(content.split("gitdir:", 1)[1].strip()) + head_file = worktree_git_dir / "HEAD" + else: + return "Unknown" + else: + # Regular .git directory + head_file = git_path / "HEAD" + + with head_file.open("r") as f: + content = f.read().splitlines() + for line in content: + if line[0:4] == "ref:": + return line.partition("refs/heads/")[2] + except Exception: + pass return "Unknown" diff --git a/tests/llm/utils/test_case_utils.py b/tests/llm/utils/test_case_utils.py index c1c7878d3f..0b1666f029 100644 --- a/tests/llm/utils/test_case_utils.py +++ b/tests/llm/utils/test_case_utils.py @@ -96,7 +96,7 @@ def load_test_cases(self) -> List[HolmesTestCase]: ] # ignoring hidden files like Mac's .DS_Store for test_case_id in test_cases_ids: test_case_folder = self._test_cases_folder.joinpath(test_case_id) - logging.info("Evaluating potential test case folder: {test_case_folder}") + logging.debug(f"Evaluating potential test case folder: {test_case_folder}") try: config_dict = yaml.safe_load( read_file(test_case_folder.joinpath(CONFIG_FILE_NAME)) @@ -142,15 +142,15 @@ def load_test_cases(self) -> List[HolmesTestCase]: config_dict ) - logging.info(f"Successfully loaded test case {test_case_id}") + logging.debug(f"Successfully loaded test case {test_case_id}") except FileNotFoundError: - logging.info( + logging.debug( f"Folder {self._test_cases_folder}/{test_case_id} ignored because it is missing a {CONFIG_FILE_NAME} file." ) continue test_cases.append(test_case) - logging.info(f"Found {len(test_cases)} in {self._test_cases_folder}") + logging.debug(f"Found {len(test_cases)} in {self._test_cases_folder}") return test_cases diff --git a/tests/llm/utils/test_helpers.py b/tests/llm/utils/test_helpers.py new file mode 100644 index 0000000000..31359a8ce2 --- /dev/null +++ b/tests/llm/utils/test_helpers.py @@ -0,0 +1,77 @@ +"""Test helper functions for enhanced output formatting.""" + +import textwrap +from typing import List, Any + + +def truncate_output(data: str, max_lines: int = 10, label: str = "lines") -> str: + """Truncate output to max_lines for readability.""" + lines = data.split("\n") + if len(lines) > max_lines: + preview_lines = lines[:max_lines] + remaining = len(lines) - max_lines + preview_lines.append(f"... [TRUNCATED: {remaining} more {label} not shown]") + return "\n".join(preview_lines) + return data + + +# Backward compatibility alias +_truncate_tool_output = truncate_output + + +def print_tool_calls_detailed(tool_calls: List[Any]) -> None: + """Print detailed tool output for debugging (limited to 10 lines per tool)""" + if tool_calls: + print("\n๐Ÿ”ง TOOLS CALLED (DETAILED):") + for tc in tool_calls: + truncated_data = _truncate_tool_output(tc.result.data) + print(f"\n") + print(textwrap.indent(truncated_data, " ")) + print("") + else: + print("\n๐Ÿ”ง TOOLS CALLED: None") + + +def print_tool_calls_summary(tool_calls: List[Any]) -> None: + """Print summary of tool calls.""" + if tool_calls: + print(f"\n๐Ÿ”ง TOOLS CALLED ({len(tool_calls)}):") + for i, tc in enumerate(tool_calls, 1): + print(f" {i}. {tc.description}") + else: + print("\n๐Ÿ”ง TOOLS CALLED: None") + + +def print_expected_output(expected: List[str]) -> None: + """Print expected output in formatted way.""" + print("\n๐Ÿ“ EXPECTED OUTPUT:") + for exp in expected: + print(f" - {exp}") + + +def print_correctness_evaluation(correctness_eval: Any) -> None: + """Print correctness evaluation results.""" + print("\nโš–๏ธ CORRECTNESS EVALUATION:") + print(f" Score: {correctness_eval.score}") + print(" Rationale: ") + rationale = correctness_eval.metadata.get("rationale", "") + for line in rationale.split("\n"): + if line.strip(): + print(f" {line}") + + +def log_tool_calls_to_spans(tool_calls: List[Any], parent_span: Any) -> None: + """Log tool calls to Braintrust spans for traceability.""" + if not tool_calls or not parent_span: + return + + for tc in tool_calls: + with parent_span.start_span(name=tc.tool_name, type="tool") as tool_span: + tool_span.log( + input={"description": tc.description}, + output={ + "data": tc.result.data + if hasattr(tc.result, "data") + else str(tc.result) + }, + ) From 28b1f24b8d8939b960451538b75f336052a044c9 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 11:08:49 +0300 Subject: [PATCH 02/13] fixes --- conftest.py | 12 +- pyproject.toml | 2 +- tests/llm/conftest.py | 831 +---------------------- tests/llm/reporting/__init__.py | 1 + tests/llm/reporting/github_reporter.py | 126 ++++ tests/llm/reporting/property_manager.py | 75 ++ tests/llm/reporting/terminal_reporter.py | 157 +++++ tests/llm/test_ask_holmes.py | 69 +- tests/llm/test_investigate.py | 27 +- tests/llm/test_workload_health.py | 15 +- tests/llm/utils/braintrust.py | 54 +- tests/llm/utils/langfuse.py | 14 - tests/llm/utils/mock_toolset.py | 171 ++++- tests/llm/utils/property_manager.py | 62 ++ tests/llm/utils/setup_cleanup.py | 156 +++++ tests/llm/utils/test_mock_toolset.py | 2 +- tests/llm/utils/test_results.py | 101 +++ 17 files changed, 959 insertions(+), 916 deletions(-) create mode 100644 tests/llm/reporting/__init__.py create mode 100644 tests/llm/reporting/github_reporter.py create mode 100644 tests/llm/reporting/property_manager.py create mode 100644 tests/llm/reporting/terminal_reporter.py create mode 100644 tests/llm/utils/property_manager.py create mode 100644 tests/llm/utils/setup_cleanup.py create mode 100644 tests/llm/utils/test_results.py diff --git a/conftest.py b/conftest.py index eec7c85ae9..040ba40c9c 100644 --- a/conftest.py +++ b/conftest.py @@ -1,8 +1,7 @@ import logging -from tests.llm.conftest import xxxxxpytest_terminal_summary2 +from tests.llm.conftest import show_llm_summary_report -# Register the same options that the LLM conftest expects def pytest_addoption(parser): """Add custom pytest command line options""" parser.addoption( @@ -31,10 +30,6 @@ def pytest_addoption(parser): ) -# reexport the pytest_terminal_summary2 function to be used in the main conftest.py -pytest_terminal_summary = xxxxxpytest_terminal_summary2 - - def pytest_configure(config): """Configure pytest settings""" # Configure worker-specific log files for xdist compatibility @@ -87,3 +82,8 @@ def pytest_configure(config): # Suppress httpx HTTP request logs logging.getLogger("httpx").setLevel(logging.WARNING) logging.getLogger("httpcore").setLevel(logging.WARNING) + + +# due to pytest quirks, we need to define this in the main conftest.py - when defined in the llm conftest.py it +# is SOMETIMES picked up and sometimes not, depending on how the test was invokedr +pytest_terminal_summary = show_llm_summary_report diff --git a/pyproject.toml b/pyproject.toml index 32f0f7e437..755bad1914 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,7 +104,7 @@ addopts = [ "--cov=holmes", "-rs", # Show skip reasons by default "--tb=short", # Show shorter tracebacks by default - "--durations=5", # Show 5 slowest tests after each run + "--durations=10", # Show 5 slowest tests after each run ] # Logging configuration for pytest diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index 6cb14356a7..49650c5de4 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -1,31 +1,32 @@ -import logging import os -import textwrap -import warnings from contextlib import contextmanager -from dataclasses import dataclass -from typing import List, Optional import pytest -from litellm import completion -from rich.console import Console -from rich.table import Table from pytest_shared_session_scope import ( shared_session_scope_json, SetupToken, CleanupToken, ) -from tests.llm.utils.braintrust import get_experiment_name -from tests.llm.utils.constants import PROJECT +from tests.llm.utils.test_results import TestResult from tests.llm.utils.classifiers import create_llm_client -from tests.llm.utils.mock_toolset import MockMode, MockGenerationConfig # type: ignore[attr-defined] +from tests.llm.utils.mock_toolset import ( # type: ignore[attr-defined] + MockMode, + MockGenerationConfig, + report_mock_operations, +) +from tests.llm.reporting.terminal_reporter import handle_console_output +from tests.llm.reporting.github_reporter import handle_github_output +from tests.llm.utils.braintrust import get_braintrust_url +from tests.llm.utils.setup_cleanup import ( + run_test_setup, + run_test_cleanup, + extract_test_cases_needing_setup, +) # Configuration constants DEBUG_SEPARATOR = "=" * 80 LLM_TEST_TYPES = ["test_ask_holmes", "test_investigate", "test_workload_health"] -MAX_ERROR_LINES = 10 -MAX_WORKERS = 30 def is_llm_test(nodeid: str) -> bool: @@ -39,60 +40,6 @@ def is_llm_test(nodeid: str) -> bool: ) -# Status determination types -class TestStatus: - """Encapsulates test status determination logic.""" - - def __init__(self, result: dict): - self.actual_score = int(result.get("actual_correctness_score", 0)) - self.expected_score = int(result.get("expected_correctness_score", 1)) - self.is_mock_failure = result.get("mock_data_failure", False) - - @property - def passed(self) -> bool: - return ( - self.actual_score == 1 - ) # TODO: possibly add `and not self.is_mock_failure` - - @property - def is_regression(self) -> bool: - if self.passed or self.is_mock_failure: - return False - # Known failure (expected to fail) - if self.actual_score == 0 and self.expected_score == 0: - return False - return True - - @property - def markdown_symbol(self) -> str: - if self.is_mock_failure: - return ":wrench:" - elif self.passed: - return ":white_check_mark:" - elif self.actual_score == 0 and self.expected_score == 0: - return ":warning:" - else: - return ":x:" - - @property - def console_status(self) -> str: - if self.is_mock_failure: - return "[yellow]MOCK FAILURE[/yellow]" - elif self.passed: - return "[green]PASS[/green]" - else: - return "[red]FAIL[/red]" - - @property - def short_status(self) -> str: - if self.is_mock_failure: - return "MOCK FAILURE" - elif self.passed: - return "PASS" - else: - return "FAIL" - - @pytest.fixture(scope="session") def mock_generation_config(request): """Session-scoped fixture that provides mock generation configuration and mode.""" @@ -127,7 +74,7 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo print( f"Skipping shared test infrastructure setup/cleanup (mode: {mock_generation_config.mode}, collect_only: {collect_only})" ) - # Must yield twice even when skipping due to ohw pytest-shared-session-scope works + # Must yield twice even when skipping due to how pytest-shared-session-scope works initial = yield cleanup_token = yield {"test_cases_for_cleanup": []} return @@ -140,21 +87,23 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo if initial is SetupToken.FIRST: # This is the first worker to run the fixture - test_cases = _extract_test_cases_needing_setup(request.session) + test_cases = extract_test_cases_needing_setup(request.session) # Clear mock directories if --regenerate-all-mocks is set cleared_directories = [] regenerate_all = request.config.getoption("--regenerate-all-mocks") if regenerate_all: - cleared_directories = _clear_mock_directories(request.session) + from tests.llm.utils.mock_toolset import clear_all_mocks # type: ignore[attr-defined] + + cleared_directories = clear_all_mocks(request.session) # Run setup unless --skip-setup is set # Check skip-setup option skip_setup = request.config.getoption("--skip-setup") if test_cases and not skip_setup: - _run_test_setup(test_cases) + run_test_setup(test_cases) elif skip_setup: print("\nโญ๏ธ Skipping test setup due to --skip-setup flag") @@ -197,11 +146,12 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo cleanup_test_cases.append(test_case) if cleanup_test_cases: - _run_test_cleanup(cleanup_test_cases) + run_test_cleanup(cleanup_test_cases) elif skip_cleanup: print("\nโญ๏ธ Skipping test cleanup due to --skip-cleanup flag") +# TODO: do we actually need this? @pytest.fixture(scope="session", autouse=True) def test_infrastructure_coordination(shared_test_infrastructure): """Ensure the shared test infrastructure fixture is used (triggers setup/cleanup)""" @@ -210,50 +160,6 @@ def test_infrastructure_coordination(shared_test_infrastructure): yield -@dataclass -class TestResult: - nodeid: str - expected: str - actual: str - pass_fail: str - tools_called: List[str] - logs: str - test_type: str = "" - error_message: Optional[str] = None - execution_time: Optional[float] = None - expected_correctness_score: float = 1.0 - actual_correctness_score: float = 0.0 - mock_data_failure: bool = False - - @property - def test_id(self) -> str: - """Extract test ID from pytest nodeid. - - Example: 'test_ask_holmes[01_how_many_pods]' -> '01' - """ - if "[" in self.nodeid and "]" in self.nodeid: - test_case = self.nodeid.split("[")[1].split("]")[0] - # Extract number from start of test case name - return test_case.split("_")[0] if "_" in test_case else test_case - return "unknown" - - @property - def test_name(self) -> str: - """Extract readable test name from pytest nodeid. - - Example: 'test_ask_holmes[01_how_many_pods]' -> 'how_many_pods' - """ - try: - if "[" in self.nodeid and "]" in self.nodeid: - test_case = self.nodeid.split("[")[1].split("]")[0] - # Remove number prefix and convert underscores to spaces - parts = test_case.split("_")[1:] if "_" in test_case else [test_case] - return "_".join(parts) - except (IndexError, AttributeError): - pass - return self.nodeid.split("::")[-1] if "::" in self.nodeid else self.nodeid - - @contextmanager def force_pytest_output(request): """Context manager to force output display even when pytest captures stdout""" @@ -424,22 +330,11 @@ def braintrust_eval_link(request): print() -def _safe_print(terminalreporter, message=""): - """Safely print to terminal reporter to avoid I/O errors""" - try: - terminalreporter.write_line(message) - except Exception: - # If write_line fails, try direct write - try: - terminalreporter._tw.write(message + "\n") - except Exception: - # Last resort - ignore if all writing fails - pass - - -def xxxxxpytest_terminal_summary2(terminalreporter, exitstatus, config): +def show_llm_summary_report(terminalreporter, exitstatus, config): """Generate GitHub Actions report and Rich summary table from terminalreporter.stats (xdist compatible)""" + print("\n\n[DEBUG] pytest_terminal_summary called!") if not hasattr(terminalreporter, "stats"): + print("[DEBUG] terminalreporter has no stats attribute") return # When using xdist, only the master process should display the summary @@ -458,30 +353,26 @@ def xxxxxpytest_terminal_summary2(terminalreporter, exitstatus, config): terminalreporter ) + print(f"[DEBUG] Found {len(sorted_results)} test results") if not sorted_results: + print("[DEBUG] No sorted results found, returning") return # Handle GitHub/CI output (markdown + file writing) - _handle_github_output(sorted_results) + handle_github_output(sorted_results) # Handle console/developer output (Rich table + Braintrust links) - _handle_console_output(sorted_results, terminalreporter) + handle_console_output(sorted_results, terminalreporter) # Report mock operation statistics - _report_mock_operations(config, mock_tracking_data, terminalreporter) - - -def markdown_table(headers, rows): - """Generate a markdown table from headers and rows.""" - markdown = "| " + " | ".join(headers) + " |\n" - markdown += "| " + " | ".join(["---" for _ in headers]) + " |\n" - for row in rows: - markdown += "| " + " | ".join(str(cell) for cell in row) + " |\n" - return markdown + report_mock_operations(config, mock_tracking_data, terminalreporter) def _collect_test_results_from_stats(terminalreporter): """Collect and parse test results from terminalreporter.stats.""" + print( + f"[DEBUG] _collect_test_results_from_stats called, stats keys: {list(terminalreporter.stats.keys())}" + ) test_results = {} mock_tracking_data = { "generated_mocks": [], @@ -496,6 +387,7 @@ def _collect_test_results_from_stats(terminalreporter): ] for status, reports in terminalreporter.stats.items(): + print(f"[DEBUG] Status '{status}' has {len(reports)} reports") for report in reports: # Only process 'call' phase reports for actual test results if getattr(report, "when", None) != "call": @@ -609,658 +501,3 @@ def _collect_test_results_from_stats(terminalreporter): ) return sorted_results, mock_tracking_data - - -def get_braintrust_url( - test_suite: str, - test_id: str, - test_name: str, - span_id: Optional[str] = None, - root_span_id: Optional[str] = None, -) -> Optional[str]: - """Generate Braintrust URL for a test. - - Args: - test_suite: Either "ask_holmes" or "investigate" - test_id: Test ID like "01" - test_name: Test name like "how_many_pods" - - Returns: - Braintrust URL string, or None if Braintrust is not configured - """ - braintrust_api_key = os.environ.get("BRAINTRUST_API_KEY") - if not braintrust_api_key: - return None - - experiment_name = get_experiment_name(test_suite) - braintrust_org = os.environ.get("BRAINTRUST_ORG", "robustadev") - - # Build URL with available parameters - url = f"https://www.braintrust.dev/app/{braintrust_org}/p/{PROJECT}/experiments/{experiment_name}?c=" - - # Add span IDs if available - if span_id and root_span_id: - # Use span_id as r parameter and root_span_id as s parameter - url += f"&r={span_id}&s={root_span_id}" - - return url - - -def _generate_markdown_report(sorted_results): - """Generate markdown report from sorted test results.""" - markdown = "## Results of HolmesGPT evals\n\n" - - # Count results by test type and status - ask_holmes_total = ask_holmes_passed = ask_holmes_regressions = ( - ask_holmes_mock_failures - ) = 0 - investigate_total = investigate_passed = investigate_regressions = ( - investigate_mock_failures - ) = 0 - workload_health_total = workload_health_passed = workload_health_regressions = ( - workload_health_mock_failures - ) = 0 - - for result in sorted_results: - status = TestStatus(result) - - if result["test_type"] == "ask": - ask_holmes_total += 1 - if status.passed: - ask_holmes_passed += 1 - elif status.is_regression: - ask_holmes_regressions += 1 - elif status.is_mock_failure: - ask_holmes_mock_failures += 1 - elif result["test_type"] == "investigate": - investigate_total += 1 - if status.passed: - investigate_passed += 1 - elif status.is_regression: - investigate_regressions += 1 - elif status.is_mock_failure: - investigate_mock_failures += 1 - elif result["test_type"] == "workload_health": - workload_health_total += 1 - if status.passed: - workload_health_passed += 1 - elif status.is_regression: - workload_health_regressions += 1 - elif status.is_mock_failure: - workload_health_mock_failures += 1 - - # Generate summary lines - if ask_holmes_total > 0: - markdown += f"- ask_holmes: {ask_holmes_passed}/{ask_holmes_total} test cases were successful, {ask_holmes_regressions} regressions" - if ask_holmes_mock_failures > 0: - markdown += f", {ask_holmes_mock_failures} mock failures" - markdown += "\n" - if investigate_total > 0: - markdown += f"- investigate: {investigate_passed}/{investigate_total} test cases were successful, {investigate_regressions} regressions" - if investigate_mock_failures > 0: - markdown += f", {investigate_mock_failures} mock failures" - markdown += "\n" - if workload_health_total > 0: - markdown += f"- workload_health: {workload_health_passed}/{workload_health_total} test cases were successful, {workload_health_regressions} regressions" - if workload_health_mock_failures > 0: - markdown += f", {workload_health_mock_failures} mock failures" - markdown += "\n" - - # Generate detailed table - markdown += "\n\n| Test suite | Test case | Status |\n" - markdown += "| --- | --- | --- |\n" - - for result in sorted_results: - test_suite = result["test_type"] - test_name = f"{result['test_id']}: {result['test_name']}" - - # Add Braintrust link to test name if available - test_suite_full = ( - "ask_holmes" if result["test_type"] == "ask" else "investigate" - ) - braintrust_url = get_braintrust_url( - test_suite_full, - result["test_id"], - result["test_name"], - result.get("braintrust_span_id"), - result.get("braintrust_root_span_id"), - ) - if braintrust_url: - test_name = f"[{test_name}]({braintrust_url})" - - status = TestStatus(result) - markdown += f"| {test_suite} | {test_name} | {status.markdown_symbol} |\n" - - markdown += "\n\n**Legend**\n" - markdown += "\n- :white_check_mark: the test was successful" - markdown += ( - "\n- :warning: the test failed but is known to be flaky or known to fail" - ) - markdown += ( - "\n- :wrench: the test failed due to mock data issues (not a code regression)" - ) - markdown += "\n- :x: the test failed and should be fixed before merging the PR" - - return markdown, sorted_results, ask_holmes_regressions + investigate_regressions - - -def _handle_github_output(sorted_results): - """Generate and write GitHub Actions report files.""" - # Generate markdown report - markdown, _, total_regressions = _generate_markdown_report(sorted_results) - - # Write report files if Braintrust is configured - braintrust_api_key = os.environ.get("BRAINTRUST_API_KEY") - if braintrust_api_key: - with open("evals_report.txt", "w", encoding="utf-8") as file: - file.write(markdown) - - # Write regressions file if needed - if total_regressions > 0: - with open("regressions.txt", "w", encoding="utf-8") as file: - file.write(f"{total_regressions}") - - -def _handle_console_output(sorted_results, terminalreporter=None): - """Display Rich table and Braintrust links for developers.""" - if not sorted_results: - return - - # Create Rich table - console = Console() - table = Table( - title="๐Ÿ” HOLMES TESTS SUMMARY", - show_header=True, - header_style="bold magenta", - show_lines=True, - ) - - # Add columns with specific widths (reduced to fit terminal width) - table.add_column("Test", style="cyan", width=12) - table.add_column("Status", justify="center", width=13) - table.add_column("Time", justify="right", width=5) - table.add_column("Expected", style="green", width=22) - table.add_column("Actual", style="yellow", width=22) - table.add_column("Analysis", style="red", width=28) - - # Add rows to table - for result in sorted_results: - status = TestStatus(result) - pass_fail = "โœ… PASS" if status.passed else "โŒ FAIL" - - # Create TestResult object for analysis function - test_result = TestResult( - nodeid=result.get("nodeid", ""), - expected=result["expected"], - actual=result["actual"], - pass_fail=pass_fail, - tools_called=result["tools_called"], - logs="", # We don't have logs in this context - test_type=result["test_type"], - error_message=None, - execution_time=result.get("execution_time"), - expected_correctness_score=result["expected_correctness_score"], - actual_correctness_score=result["actual_correctness_score"], - mock_data_failure=result.get("mock_data_failure", False), - ) - - # Wrap long content for table readability - expected_wrapped = ( - "\n".join(textwrap.wrap(result["expected"], width=20)) - if result["expected"] - else "" - ) - actual_wrapped = ( - "\n".join(textwrap.wrap(result["actual"], width=20)) - if result["actual"] - else "" - ) - - # Combine test ID and name using TestResult properties - combined_test_name = ( - f"{test_result.test_id}_{test_result.test_name} ({result['test_type']})" - ) - # Wrap test name to fit column - test_name_wrapped = "\n".join(textwrap.wrap(combined_test_name, width=10)) - - # Format execution time - time_str = ( - f"{result.get('execution_time'):.1f}s" - if result.get("execution_time") - else "N/A" - ) - - # Get analysis for failed tests - analysis = _get_analysis_for_result(test_result) - - table.add_row( - test_name_wrapped, - status.console_status, - time_str, - expected_wrapped, - actual_wrapped, - analysis, - ) - - # Use force_terminal to ensure output is displayed even when captured - console.print(table) - - -def _get_analysis_for_result(result): - """Get analysis text for a test result, with proper text wrapping.""" - if "PASS" in result.pass_fail: - return "" - - try: - analysis = _get_llm_analysis(result) - # Wrap analysis text for table readability - return "\n".join(textwrap.wrap(analysis, width=26)) - except Exception as e: - return f"Analysis failed: {str(e)}" - - -def _get_llm_analysis(result: TestResult) -> str: - """Get LLM analysis of test failure using GPT-4o. - - Args: - result: TestResult object containing test details - - Returns: - Analysis text explaining why the test failed - """ - # Check if this is a MockDataError case and add context - mock_data_context = "" - if result.mock_data_failure: - mock_data_context = "\n\nIMPORTANT CONTEXT: This test failed due to MockDataError - no mock data files were found for the tool calls that the agent tried to make. This is a test infrastructure issue, not a problem with the agent's logic." - - prompt = textwrap.dedent(f"""\ - Analyze this failed eval for an AIOps agent why it failed. - TEST: {result.test_name} - EXPECTED: {result.expected} - ACTUAL: {result.actual} - TOOLS CALLED: {', '.join(result.tools_called)} - ERROR: {result.error_message or 'Test assertion failed'} - - LOGS: - {result.logs if result.logs else 'No logs available'}{mock_data_context} - - Please provide a concise analysis (2-3 sentences) and categorize this as one of: - - MockDataError - the test failed because mock data files were missing for the tool calls (this is a test infrastructure issue). - To fix (show bullet points with each option - any are valid solutions so user should see all options): - - Run with RUN_LIVE=true - - Use --generate-mocks (may cause inconsistent data) - - Use --regenerate-all-mocks (ensures consistency) - - Problem with mock data - the test is failing due to incorrect or incomplete mock data, but the agent itself did the correct queries you would expect it to do - - Setup issue - the test is failing due to an issue with the test setup, such as missing tools or incorrect before_test/after_test configuration - - Real failure - the test is failing because the agent did not perform as expected, and this is a real issue that needs to be fixed - """) - - try: - response = completion( - model="gpt-4o", - messages=[{"role": "user", "content": prompt}], - max_tokens=200, - temperature=0.1, - ) - return response.choices[0].message.content.strip() - except Exception as e: - return f"Analysis failed: {e}" - - -def _report_mock_operations(config, mock_tracking_data, terminalreporter=None): - """Report mock file operations and statistics.""" - # Use default parameter to safely handle missing options - generate_mocks = False - regenerate_all_mocks = False - - try: - generate_mocks = config.getoption("--generate-mocks", default=False) - regenerate_all_mocks = config.getoption("--regenerate-all-mocks", default=False) - except (AttributeError, ValueError): - # Options not available, use defaults - pass - - if not generate_mocks and not regenerate_all_mocks: - return - - regenerate_mode = regenerate_all_mocks - generated_mocks = mock_tracking_data["generated_mocks"] - mock_failures = mock_tracking_data["mock_failures"] - - # If no terminalreporter, skip output - if not terminalreporter: - return - - # Header - _safe_print(terminalreporter, f"\n{'=' * 80}") - _safe_print( - terminalreporter, - f"{'๐Ÿ”„ MOCK REGENERATION SUMMARY' if regenerate_mode else '๐Ÿ”ง MOCK GENERATION SUMMARY'}", - ) - _safe_print(terminalreporter, f"{'=' * 80}") - - # Note: Cleared directories are now handled by shared_test_infrastructure fixture - # and reported during setup phase to ensure single execution across workers - - # Generated mocks - if generated_mocks: - _safe_print( - terminalreporter, f"โœ… Generated {len(generated_mocks)} mock files:\n" - ) - - # Group by test case - by_test_case = {} - for mock_info in generated_mocks: - parts = mock_info.split(":", 2) - if len(parts) == 3: - test_case, tool_name, filename = parts - by_test_case.setdefault(test_case, []).append( - f"{tool_name} -> {filename}" - ) - - for test_case, mock_files in sorted(by_test_case.items()): - _safe_print(terminalreporter, f"๐Ÿ“ {test_case}:") - for mock_file in mock_files: - _safe_print(terminalreporter, f" - {mock_file}") - _safe_print(terminalreporter) - else: - mode_text = "regeneration" if regenerate_mode else "generation" - _safe_print( - terminalreporter, - f"โœ… Mock {mode_text} was enabled but no new mock files were created", - ) - - # Failures - if mock_failures: - _safe_print( - terminalreporter, f"โš ๏ธ {len(mock_failures)} mock-related failures occurred:" - ) - for failure in mock_failures: - _safe_print(terminalreporter, f" - {failure}") - _safe_print(terminalreporter) - - # Checklist - checklist = [ - "Review generated mock files before committing", - "Ensure mock data represents realistic scenarios", - "Check data consistency across related mocks (e.g., if a pod appears in", - " one mock, it should appear in all related mocks from the same test run)", - "Verify timestamps, IDs, and names match between interconnected mock files", - "If pod/resource names change across tool calls, regenerate ALL mocks with --regenerate-all-mocks", - ] - - _safe_print(terminalreporter, "๐Ÿ“‹ REVIEW CHECKLIST:") - for item in checklist: - _safe_print(terminalreporter, f" โ–ก {item}") - _safe_print(terminalreporter, "=" * 80) - - -def _format_error_output(error_details: str) -> str: - """Format error details with truncation if needed""" - from tests.llm.utils.test_helpers import truncate_output - - return truncate_output(error_details, max_lines=MAX_ERROR_LINES) - - -def _run_test_setup(test_cases): - """Run before_test for each test case in parallel""" - from tests.llm.utils.commands import before_test - from concurrent.futures import ThreadPoolExecutor, as_completed - import time - - print(f"Setting up infrastructure for {len(test_cases)} test cases") - - start_time = time.time() - successful_test_cases = 0 - failed_test_cases = 0 - timed_out_test_cases = 0 - - with ThreadPoolExecutor(max_workers=min(len(test_cases), MAX_WORKERS)) as executor: - # Submit all setup tasks - future_to_test_case = { - executor.submit(before_test, test_case): test_case - for test_case in test_cases - } - - # Wait for all tasks to complete and handle results - for future in as_completed(future_to_test_case): - test_case = future_to_test_case[future] - try: - result = future.result() # Single CommandResult for the test case - remaining_cases = ( - len(test_cases) - - successful_test_cases - - failed_test_cases - - timed_out_test_cases - ) - if result.success: - successful_test_cases += 1 - print( - f"โœ… Setup {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); setups remaining: {remaining_cases}" - ) - elif result.error_type == "timeout": - timed_out_test_cases += 1 - print( - f"โฐ Setup {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; setups remaining: {remaining_cases}" - ) - - # Show the exact command that timed out - truncated_error = _format_error_output(result.error_details) - print(textwrap.indent(truncated_error, " ")) - logging.error( - f"[{test_case.id}] Setup timeout: {result.error_details}" - ) - - # Emit warning to make it visible in pytest output - warnings.warn( - f"Setup timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", - UserWarning, - stacklevel=2, - ) - else: - failed_test_cases += 1 - print( - f"โŒ Setup {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); setups remaining: {remaining_cases}" - ) - - # Limit error details to 10 lines and add proper formatting - truncated_error = _format_error_output(result.error_details) - print(textwrap.indent(truncated_error, " ")) - logging.error( - f"[{test_case.id}] Setup failed: {result.error_details}" - ) - - # Emit warning to make it visible in pytest output - warnings.warn( - f"Setup failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", - UserWarning, - stacklevel=2, - ) - - except Exception as e: - failed_test_cases += 1 - print(f"โŒ Setup {test_case.id}: EXCEPTION - {e}") - logging.error(f"Setup exception for {test_case.id}: {str(e)}") - - # Emit warning to make it visible in pytest output - warnings.warn( - f"Setup exception for test {test_case.id}: {str(e)}", - UserWarning, - stacklevel=2, - ) - - elapsed_time = time.time() - start_time - print( - f"\n๐Ÿ• Setup completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" - ) - - -def _run_test_cleanup(test_cases): - """Run after_test for each test case in parallel""" - from tests.llm.utils.commands import after_test - from concurrent.futures import ThreadPoolExecutor, as_completed - import time - - print(f"Cleaning up infrastructure after tests for {len(test_cases)} test cases") - - start_time = time.time() - successful_test_cases = 0 - failed_test_cases = 0 - timed_out_test_cases = 0 - - with ThreadPoolExecutor(max_workers=min(len(test_cases), MAX_WORKERS)) as executor: - # Submit all cleanup tasks - future_to_test_case = { - executor.submit(after_test, test_case): test_case - for test_case in test_cases - } - - # Wait for all tasks to complete and handle results - for future in as_completed(future_to_test_case): - test_case = future_to_test_case[future] - try: - result = future.result() # Single CommandResult for the test case - remaining_cases = ( - len(test_cases) - - successful_test_cases - - failed_test_cases - - timed_out_test_cases - ) - - if result.success: - successful_test_cases += 1 - print( - f"โœ… Cleanup {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); cleanups remaining: {remaining_cases}" - ) - elif result.error_type == "timeout": - timed_out_test_cases += 1 - print( - f"โฐ Cleanup {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; cleanups remaining: {remaining_cases}" - ) - - # Show the exact command that timed out - truncated_error = _format_error_output(result.error_details) - print(textwrap.indent(truncated_error, " ")) - logging.error( - f"[{test_case.id}] Cleanup timeout: {result.error_details}" - ) - - # Emit warning to make it visible in pytest output - warnings.warn( - f"Cleanup timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", - UserWarning, - stacklevel=2, - ) - else: - failed_test_cases += 1 - print( - f"โŒ Cleanup {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); cleanups remaining: {remaining_cases}" - ) - - # Limit error details to 10 lines and add proper formatting - truncated_error = _format_error_output(result.error_details) - print(textwrap.indent(truncated_error, " ")) - logging.error( - f"[{test_case.id}] Cleanup failed: {result.error_details}" - ) - - # Emit warning to make it visible in pytest output - warnings.warn( - f"Cleanup failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", - UserWarning, - stacklevel=2, - ) - - except Exception as e: - failed_test_cases += 1 - print(f"โŒ Cleanup {test_case.id}: EXCEPTION - {e}") - logging.error(f"Cleanup exception for {test_case.id}: {str(e)}") - - # Emit warning to make it visible in pytest output - warnings.warn( - f"Cleanup exception for test {test_case.id}: {str(e)}", - UserWarning, - stacklevel=2, - ) - - elapsed_time = time.time() - start_time - print( - f"\n๐Ÿ• Cleanup completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" - ) - - -def _extract_test_cases_needing_setup(session): - """Extract unique test cases that need setup from session items""" - from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] - - seen_ids = set() - test_cases = [] - - for item in session.items: - if ( - item.get_closest_marker("llm") - and hasattr(item, "callspec") - and "test_case" in item.callspec.params - ): - test_case = item.callspec.params["test_case"] - if ( - isinstance(test_case, HolmesTestCase) - and test_case.before_test - and test_case.id not in seen_ids - ): - test_cases.append(test_case) - seen_ids.add(test_case.id) - - return test_cases - - -def _clear_mock_directories(session): - """Clear mock directories for all test cases when --regenerate-all-mocks is set""" - from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] - import glob - - print("\n๐Ÿงน Clearing mock files for --regenerate-all-mocks") - - cleared_directories = set() - total_files_removed = 0 - - # Extract all unique test case folders - test_folders = set() - for item in session.items: - if ( - item.get_closest_marker("llm") - and hasattr(item, "callspec") - and "test_case" in item.callspec.params - ): - test_case = item.callspec.params["test_case"] - if isinstance(test_case, HolmesTestCase): - test_folders.add(test_case.folder) - - # Clear mock files from each folder - for folder in test_folders: - patterns = [ - os.path.join(folder, "*.txt"), - os.path.join(folder, "*.json"), - ] - - folder_files_removed = 0 - for pattern in patterns: - for file_path in glob.glob(pattern): - try: - os.remove(file_path) - folder_files_removed += 1 - total_files_removed += 1 - except Exception as e: - logging.warning(f"Could not remove {file_path}: {e}") - - if folder_files_removed > 0: - cleared_directories.add(folder) - print( - f" โœ… Cleared {folder_files_removed} mock files from {os.path.basename(folder)}" - ) - - print( - f" ๐Ÿ“Š Total: Cleared {total_files_removed} files from {len(cleared_directories)} directories\n" - ) - - return list(cleared_directories) diff --git a/tests/llm/reporting/__init__.py b/tests/llm/reporting/__init__.py new file mode 100644 index 0000000000..fd6cf1ed22 --- /dev/null +++ b/tests/llm/reporting/__init__.py @@ -0,0 +1 @@ +"""Reporting modules for test results and analysis.""" diff --git a/tests/llm/reporting/github_reporter.py b/tests/llm/reporting/github_reporter.py new file mode 100644 index 0000000000..0015a4fd31 --- /dev/null +++ b/tests/llm/reporting/github_reporter.py @@ -0,0 +1,126 @@ +"""GitHub Actions reporting functionality.""" + +import os +from typing import List, Tuple + +from tests.llm.utils.test_results import TestStatus +from tests.llm.utils.braintrust import get_braintrust_url + + +def handle_github_output(sorted_results: List[dict]) -> None: + """Generate and write GitHub Actions report files.""" + # Generate markdown report + markdown, _, total_regressions = generate_markdown_report(sorted_results) + + # Write report files if Braintrust is configured + braintrust_api_key = os.environ.get("BRAINTRUST_API_KEY") + if braintrust_api_key: + with open("evals_report.txt", "w", encoding="utf-8") as file: + file.write(markdown) + + # Write regressions file if needed + if total_regressions > 0: + with open("regressions.txt", "w", encoding="utf-8") as file: + file.write(f"{total_regressions}") + + +def generate_markdown_report(sorted_results: List[dict]) -> Tuple[str, List[dict], int]: + """Generate markdown report from sorted test results.""" + markdown = "## Results of HolmesGPT evals\n\n" + + # Count results by test type and status + ask_holmes_total = ask_holmes_passed = ask_holmes_regressions = ( + ask_holmes_mock_failures + ) = 0 + investigate_total = investigate_passed = investigate_regressions = ( + investigate_mock_failures + ) = 0 + workload_health_total = workload_health_passed = workload_health_regressions = ( + workload_health_mock_failures + ) = 0 + + for result in sorted_results: + status = TestStatus(result) + + if result["test_type"] == "ask": + ask_holmes_total += 1 + if status.passed: + ask_holmes_passed += 1 + elif status.is_regression: + ask_holmes_regressions += 1 + elif status.is_mock_failure: + ask_holmes_mock_failures += 1 + elif result["test_type"] == "investigate": + investigate_total += 1 + if status.passed: + investigate_passed += 1 + elif status.is_regression: + investigate_regressions += 1 + elif status.is_mock_failure: + investigate_mock_failures += 1 + elif result["test_type"] == "workload_health": + workload_health_total += 1 + if status.passed: + workload_health_passed += 1 + elif status.is_regression: + workload_health_regressions += 1 + elif status.is_mock_failure: + workload_health_mock_failures += 1 + + # Generate summary lines + if ask_holmes_total > 0: + markdown += f"- ask_holmes: {ask_holmes_passed}/{ask_holmes_total} test cases were successful, {ask_holmes_regressions} regressions" + if ask_holmes_mock_failures > 0: + markdown += f", {ask_holmes_mock_failures} mock failures" + markdown += "\n" + if investigate_total > 0: + markdown += f"- investigate: {investigate_passed}/{investigate_total} test cases were successful, {investigate_regressions} regressions" + if investigate_mock_failures > 0: + markdown += f", {investigate_mock_failures} mock failures" + markdown += "\n" + if workload_health_total > 0: + markdown += f"- workload_health: {workload_health_passed}/{workload_health_total} test cases were successful, {workload_health_regressions} regressions" + if workload_health_mock_failures > 0: + markdown += f", {workload_health_mock_failures} mock failures" + markdown += "\n" + + # Generate detailed table + markdown += "\n\n| Test suite | Test case | Status |\n" + markdown += "| --- | --- | --- |\n" + + for result in sorted_results: + test_suite = result["test_type"] + test_name = f"{result['test_id']}: {result['test_name']}" + + # Add Braintrust link to test name if available + test_suite_full = ( + "ask_holmes" if result["test_type"] == "ask" else "investigate" + ) + braintrust_url = get_braintrust_url( + test_suite_full, + result["test_id"], + result["test_name"], + result.get("braintrust_span_id"), + result.get("braintrust_root_span_id"), + ) + if braintrust_url: + test_name = f"[{test_name}]({braintrust_url})" + + status = TestStatus(result) + markdown += f"| {test_suite} | {test_name} | {status.markdown_symbol} |\n" + + markdown += "\n\n**Legend**\n" + markdown += "\n- :white_check_mark: the test was successful" + markdown += ( + "\n- :warning: the test failed but is known to be flaky or known to fail" + ) + markdown += ( + "\n- :wrench: the test failed due to mock data issues (not a code regression)" + ) + markdown += "\n- :x: the test failed and should be fixed before merging the PR" + + return ( + markdown, + sorted_results, + ask_holmes_regressions + investigate_regressions + workload_health_regressions, + ) diff --git a/tests/llm/reporting/property_manager.py b/tests/llm/reporting/property_manager.py new file mode 100644 index 0000000000..9958965e9a --- /dev/null +++ b/tests/llm/reporting/property_manager.py @@ -0,0 +1,75 @@ +"""Property management utilities for cleaner test metadata handling.""" + +from typing import Any, List, Tuple + + +class TestPropertyManager: + """Helper class for managing test properties in a cleaner way.""" + + def __init__(self, request): + self.request = request + + def add(self, key: str, value: Any) -> None: + """Add a property to the test node.""" + if hasattr(self.request.node, "user_properties"): + self.request.node.user_properties.append((key, value)) + + def add_multiple(self, properties: List[Tuple[str, Any]]) -> None: + """Add multiple properties at once.""" + for key, value in properties: + self.add(key, value) + + def add_test_result( + self, + expected: str, + actual: str, + tools_called: List[str], + expected_score: float = 1.0, + actual_score: float = 0.0, + mock_failure: bool = False, + ) -> None: + """Add standard test result properties.""" + self.add_multiple( + [ + ("expected", expected), + ("actual", actual), + ("tools_called", tools_called), + ("expected_correctness_score", expected_score), + ("actual_correctness_score", actual_score), + ("mock_data_failure", mock_failure), + ] + ) + + def add_braintrust_info(self, span_id: str, root_span_id: str) -> None: + """Add Braintrust tracking information.""" + self.add_multiple( + [ + ("braintrust_span_id", span_id), + ("braintrust_root_span_id", root_span_id), + ] + ) + + def mark_mock_failure( + self, expected: str, actual_msg: str = "Mock data not found" + ) -> None: + """Mark a test as failed due to mock data issues.""" + self.add_test_result( + expected=expected, + actual=actual_msg, + tools_called=[], + actual_score=0, + mock_failure=True, + ) + + +# Pytest fixture for easy access +def pytest_plugin(): + """Plugin registration for property manager fixture.""" + import pytest + + @pytest.fixture + def test_props(request): + """Provide a TestPropertyManager instance for the current test.""" + return TestPropertyManager(request) + + return test_props diff --git a/tests/llm/reporting/terminal_reporter.py b/tests/llm/reporting/terminal_reporter.py new file mode 100644 index 0000000000..8e861d58eb --- /dev/null +++ b/tests/llm/reporting/terminal_reporter.py @@ -0,0 +1,157 @@ +"""Terminal reporting functionality for test results.""" + +import textwrap +from typing import List + +from rich.console import Console +from rich.table import Table + +from tests.llm.utils.test_results import TestStatus, TestResult + + +def handle_console_output(sorted_results: List[dict], terminalreporter=None) -> None: + """Display Rich table and Braintrust links for developers.""" + if not sorted_results: + return + + # Create Rich table + console = Console() + table = Table( + title="๐Ÿ” HOLMES TESTS SUMMARY", + show_header=True, + header_style="bold magenta", + show_lines=True, + ) + + # Add columns with specific widths (reduced to fit terminal width) + table.add_column("Test", style="cyan", width=12) + table.add_column("Status", justify="center", width=13) + table.add_column("Time", justify="right", width=5) + table.add_column("Expected", style="green", width=22) + table.add_column("Actual", style="yellow", width=22) + table.add_column("Analysis", style="red", width=28) + + # Add rows to table + for result in sorted_results: + status = TestStatus(result) + pass_fail = "โœ… PASS" if status.passed else "โŒ FAIL" + + # Create TestResult object for analysis function + test_result = TestResult( + nodeid=result.get("nodeid", ""), + expected=result["expected"], + actual=result["actual"], + pass_fail=pass_fail, + tools_called=result["tools_called"], + logs="", # We don't have logs in this context + test_type=result["test_type"], + error_message=None, + execution_time=result.get("execution_time"), + expected_correctness_score=result["expected_correctness_score"], + actual_correctness_score=result["actual_correctness_score"], + mock_data_failure=result.get("mock_data_failure", False), + ) + + # Wrap long content for table readability + expected_wrapped = ( + "\n".join(textwrap.wrap(result["expected"], width=20)) + if result["expected"] + else "" + ) + actual_wrapped = ( + "\n".join(textwrap.wrap(result["actual"], width=20)) + if result["actual"] + else "" + ) + + # Combine test ID and name using TestResult properties + combined_test_name = ( + f"{test_result.test_id}_{test_result.test_name} ({result['test_type']})" + ) + # Wrap test name to fit column + test_name_wrapped = "\n".join(textwrap.wrap(combined_test_name, width=10)) + + # Format execution time + time_str = ( + f"{result.get('execution_time'):.1f}s" + if result.get("execution_time") + else "N/A" + ) + + # Get analysis for failed tests + analysis = _get_analysis_for_result(test_result) + + table.add_row( + test_name_wrapped, + status.console_status, + time_str, + expected_wrapped, + actual_wrapped, + analysis, + ) + + # Use force_terminal to ensure output is displayed even when captured + console.print(table) + + +def _get_analysis_for_result(result: TestResult) -> str: + """Get analysis text for a test result, with proper text wrapping.""" + if "PASS" in result.pass_fail: + return "" + + try: + analysis = _get_llm_analysis(result) + # Wrap analysis text for table readability + return "\n".join(textwrap.wrap(analysis, width=26)) + except Exception as e: + return f"Analysis failed: {str(e)}" + + +def _get_llm_analysis(result: TestResult) -> str: + """Get LLM analysis of test failure using GPT-4o. + + Args: + result: TestResult object containing test details + + Returns: + Analysis text explaining why the test failed + """ + from litellm import completion + + # Check if this is a MockDataError case and add context + mock_data_context = "" + if result.mock_data_failure: + mock_data_context = "\n\nIMPORTANT CONTEXT: This test failed due to MockDataError - no mock data files were found for the tool calls that the agent tried to make. This is a test infrastructure issue, not a problem with the agent's logic." + + prompt = textwrap.dedent(f"""\ + Analyze this failed eval for an AIOps agent why it failed. + TEST: {result.test_name} + EXPECTED: {result.expected} + ACTUAL: {result.actual} + TOOLS CALLED: {', '.join(result.tools_called)} + ERROR: {result.error_message or 'Test assertion failed'} + + LOGS: + {result.logs if result.logs else 'No logs available'}{mock_data_context} + + Please provide a concise analysis (2-3 sentences) and categorize this as one of: + - MockDataError - the test failed because mock data files were missing for the tool calls (this is a test infrastructure issue). + To fix (show bullet points with each option - any are valid solutions so user should see all options): + - Run with RUN_LIVE=true + - Use --generate-mocks (may cause inconsistent data) + - Use --regenerate-all-mocks (ensures consistency) + - Problem with mock data - the test is failing due to incorrect or incomplete mock data, but the agent itself did the correct queries you would expect it to do + - Setup issue - the test is failing due to an issue with the test setup, such as missing tools or incorrect before_test/after_test configuration + - Real failure - the test is failing because the agent did not perform as expected, and this is a real issue that needs to be fixed + """) + + try: + response = completion( + model="gpt-4o", + messages=[{"role": "user", "content": prompt}], + max_tokens=200, + temperature=0.1, + ) + return response.choices[0].message.content.strip() + except Exception as e: + return f"Analysis failed: {e}" diff --git a/tests/llm/test_ask_holmes.py b/tests/llm/test_ask_holmes.py index e6f5565391..ae6103aaae 100644 --- a/tests/llm/test_ask_holmes.py +++ b/tests/llm/test_ask_holmes.py @@ -22,6 +22,11 @@ MockGenerationConfig, ) from tests.llm.utils.test_case_utils import AskHolmesTestCase, Evaluation, MockHelper +from tests.llm.utils.property_manager import ( + set_initial_properties, + update_test_results, + update_mock_error, +) from os import path from tests.llm.utils.tags import add_tags_to_eval from holmes.core.tracing import SpanType @@ -79,6 +84,9 @@ def test_ask_holmes( mock_generation_config: MockGenerationConfig, shared_test_infrastructure, # type: ignore ): + # Set initial properties early so they're available even if test fails + set_initial_properties(request, test_case) + print(f"\n๐Ÿงช TEST: {test_case.id}") print(" CONFIGURATION:") print( @@ -178,29 +186,8 @@ def test_ask_holmes( ) if is_mock_error: - # Store minimal data for summary before failing - expected = test_case.expected_output - if not isinstance(expected, list): - expected = [expected] - debug_expected = "\n- ".join(expected) - - expected_correctness_score = ( - test_case.evaluation.correctness.expected_score - if isinstance(test_case.evaluation.correctness, Evaluation) - else test_case.evaluation.correctness - ) - - # Record the mock failure in user_properties - request.node.user_properties.append(("expected", debug_expected)) - request.node.user_properties.append( - ("actual", f"Mock data error: {str(e)}") - ) - request.node.user_properties.append(("tools_called", [])) - request.node.user_properties.append( - ("expected_correctness_score", expected_correctness_score) - ) - request.node.user_properties.append(("actual_correctness_score", 0)) - request.node.user_properties.append(("mock_data_failure", True)) + # Update properties for mock error + update_mock_error(request, e) # Cleanup is handled by session-scoped fixture now raise @@ -266,27 +253,8 @@ def test_ask_holmes( # Print detailed tool output print_tool_calls_detailed(result.tool_calls) - # Store data for summary plugin - expected_correctness_score = ( - test_case.evaluation.correctness.expected_score - if isinstance(test_case.evaluation.correctness, Evaluation) - else test_case.evaluation.correctness - ) - debug_expected = "\n- ".join(expected) - request.node.user_properties.append(("expected", debug_expected)) - request.node.user_properties.append(("actual", output or "")) - request.node.user_properties.append( - ( - "tools_called", - tools_called if isinstance(tools_called, list) else [str(tools_called)], - ) - ) - request.node.user_properties.append( - ("expected_correctness_score", expected_correctness_score) - ) - request.node.user_properties.append( - ("actual_correctness_score", scores.get("correctness", 0)) - ) + # Update test results + update_test_results(request, output, tools_called, scores) # Check if the output contains MockDataError (indicating a mock failure) if output and any( @@ -300,13 +268,22 @@ def test_ask_holmes( # Record mock failure in user_properties request.node.user_properties.append(("mock_data_failure", True)) # Fail the test + # Get expected from test_case since debug_expected is no longer in local scope + expected_output = test_case.expected_output + if isinstance(expected_output, list): + expected_output = "\n- ".join(expected_output) pytest.fail( - f"Test {test_case.id} failed due to mock data error\nActual: {output}\nExpected: {debug_expected}" + f"Test {test_case.id} failed due to mock data error\nActual: {output}\nExpected: {expected_output}" ) + # Get expected for assertion message + expected_output = test_case.expected_output + if isinstance(expected_output, list): + expected_output = "\n- ".join(expected_output) + assert ( int(scores.get("correctness", 0)) == 1 - ), f"Test {test_case.id} failed (score: {scores.get('correctness', 0)})\nActual: {output}\nExpected: {debug_expected}" + ), f"Test {test_case.id} failed (score: {scores.get('correctness', 0)})\nActual: {output}\nExpected: {expected_output}" def ask_holmes( diff --git a/tests/llm/test_investigate.py b/tests/llm/test_investigate.py index 3b15ad5218..cd10a35771 100644 --- a/tests/llm/test_investigate.py +++ b/tests/llm/test_investigate.py @@ -21,7 +21,8 @@ from tests.llm.utils.system import get_machine_state_tags from tests.llm.utils.mock_dal import MockSupabaseDal from tests.llm.utils.mock_toolset import MockToolsetManager -from tests.llm.utils.test_case_utils import InvestigateTestCase, MockHelper, Evaluation +from tests.llm.utils.test_case_utils import InvestigateTestCase, MockHelper +from tests.llm.utils.property_manager import set_initial_properties, update_test_results from os import path from unittest.mock import patch @@ -109,6 +110,9 @@ def test_investigate( request, mock_generation_config, ): + # Set initial properties early so they're available even if test fails + set_initial_properties(request, test_case) + # Use unified tracing API for evals from holmes.core.tracing import TracingFactory @@ -207,25 +211,8 @@ def test_investigate( print(f"\n** SCORES **\n{scores}") # Store data for summary plugin - expected_correctness_score = ( - test_case.evaluation.correctness.expected_score - if isinstance(test_case.evaluation.correctness, Evaluation) - else test_case.evaluation.correctness - ) - request.node.user_properties.append(("expected", debug_expected)) - request.node.user_properties.append(("actual", output or "")) - request.node.user_properties.append( - ( - "tools_called", - tools_called if isinstance(tools_called, list) else [str(tools_called)], - ) - ) - request.node.user_properties.append( - ("expected_correctness_score", expected_correctness_score) - ) - request.node.user_properties.append( - ("actual_correctness_score", scores.get("correctness", 0)) - ) + # Update test results + update_test_results(request, output, tools_called, scores) assert result.sections, "Missing sections" assert ( diff --git a/tests/llm/test_workload_health.py b/tests/llm/test_workload_health.py index 1e4b1e3849..99ea18fc53 100644 --- a/tests/llm/test_workload_health.py +++ b/tests/llm/test_workload_health.py @@ -23,6 +23,7 @@ HealthCheckTestCase, MockHelper, ) +from tests.llm.utils.property_manager import set_initial_properties, update_test_results from os import path from braintrust import Span, SpanTypeAttribute from unittest.mock import patch @@ -101,6 +102,9 @@ def test_health_check( request, mock_generation_config, ): + # Set initial properties early so they're available even if test fails + set_initial_properties(request, test_case) + dataset_name = braintrust_util.get_dataset_name("health_check") bt_helper = braintrust_util.BraintrustEvalHelper( project_name=PROJECT, dataset_name=dataset_name @@ -171,15 +175,8 @@ def test_health_check( print(f"\n** OUTPUT **\n{output}") print(f"\n** SCORES **\n{scores}") - # Store data for summary plugin - request.node.user_properties.append(("expected", debug_expected)) - request.node.user_properties.append(("actual", output or "")) - request.node.user_properties.append( - ( - "tools_called", - tools_called if isinstance(tools_called, list) else [str(tools_called)], - ) - ) + # Update test results + update_test_results(request, output, tools_called, scores) if test_case.evaluation.correctness: expected_correctness = test_case.evaluation.correctness diff --git a/tests/llm/utils/braintrust.py b/tests/llm/utils/braintrust.py index f922b18653..8d7a29095e 100644 --- a/tests/llm/utils/braintrust.py +++ b/tests/llm/utils/braintrust.py @@ -2,9 +2,8 @@ import braintrust from braintrust import Dataset, Experiment, ReadonlyExperiment, Span import logging -from typing import Any, Dict, List, Optional, Union +from typing import Any, List, Optional, Union -from pydantic import BaseModel from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore from tests.llm.utils.system import get_machine_state_tags, readable_timestamp @@ -176,24 +175,39 @@ def get_dataset_name(test_suite: str): return f"{test_suite}:{system_metadata.get('branch', 'unknown_branch')}" -class ExperimentData(BaseModel): - experiment_name: str - records: List[Dict[str, Any]] - test_cases: List[Dict[str, Any]] +def get_braintrust_url( + test_suite: str, + test_id: str, + test_name: str, + span_id: Optional[str] = None, + root_span_id: Optional[str] = None, +) -> Optional[str]: + """Generate Braintrust URL for a test. + + Args: + test_suite: Either "ask_holmes" or "investigate" + test_id: Test ID like "01" + test_name: Test name like "how_many_pods" + span_id: Optional span ID for direct linking + root_span_id: Optional root span ID for direct linking + + Returns: + Braintrust URL string, or None if Braintrust is not configured + """ + from tests.llm.utils.constants import PROJECT + if not BRAINTRUST_API_KEY: + return None -def get_experiment_results(project_name: str, test_suite: str) -> ExperimentData: experiment_name = get_experiment_name(test_suite) - experiment = braintrust.init( - project=project_name, experiment=experiment_name, open=True - ) - dataset = braintrust.init_dataset( - project=project_name, name=get_dataset_name(test_suite) - ) - records = list(experiment.fetch()) - test_cases = list(dataset.fetch()) - return ExperimentData( - experiment_name=experiment_name, - records=records, # type: ignore - test_cases=test_cases, # type: ignore - ) + braintrust_org = os.environ.get("BRAINTRUST_ORG", "robustadev") + + # Build URL with available parameters + url = f"https://www.braintrust.dev/app/{braintrust_org}/p/{PROJECT}/experiments/{experiment_name}?c=" + + # Add span IDs if available + if span_id and root_span_id: + # Use span_id as r parameter and root_span_id as s parameter + url += f"&r={span_id}&s={root_span_id}" + + return url diff --git a/tests/llm/utils/langfuse.py b/tests/llm/utils/langfuse.py index f84386497a..47ec3a24ed 100644 --- a/tests/llm/utils/langfuse.py +++ b/tests/llm/utils/langfuse.py @@ -91,17 +91,3 @@ def upload_test_cases(test_cases: List[HolmesTestCase], dataset_name: str): expected_output={"answer": test_case.expected_output}, metadata={"test_case": test_case.model_dump()}, ) - - -def resolve_dataset_item( - test_case: HolmesTestCase, dataset_name: str -) -> Optional[DatasetItemClient]: - dataset = langfuse.get_dataset(dataset_name) - for item in dataset.items: - if ( - item.metadata - and item.metadata.get("test_case") - and item.metadata.get("test_case").get("id") == test_case.id - ): - return item - return diff --git a/tests/llm/utils/mock_toolset.py b/tests/llm/utils/mock_toolset.py index f92823d7f7..56f00757d0 100644 --- a/tests/llm/utils/mock_toolset.py +++ b/tests/llm/utils/mock_toolset.py @@ -73,6 +73,67 @@ def __init__(self, generate_mocks_enabled, regenerate_all_enabled, mock_mode): self.mode = mock_mode +def clear_all_mocks(session) -> List[str]: + """Clear mock files for all test cases when --regenerate-all-mocks is set. + + This is a session-level operation that clears all mock files across all test cases. + Used during pytest session setup. + + Args: + session: pytest session object containing all test items + + Returns: + List of directories that had files cleared + """ + from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] + + print("\n๐Ÿงน Clearing mock files for --regenerate-all-mocks") + + cleared_directories = set() + total_files_removed = 0 + + # Extract all unique test case folders + test_folders = set() + for item in session.items: + if ( + item.get_closest_marker("llm") + and hasattr(item, "callspec") + and "test_case" in item.callspec.params + ): + test_case = item.callspec.params["test_case"] + if isinstance(test_case, HolmesTestCase): + test_folders.add(test_case.folder) + + # Clear mock files from each folder + for folder in test_folders: + patterns = [ + os.path.join(folder, "*.txt"), + os.path.join(folder, "*.json"), + ] + + folder_files_removed = 0 + for pattern in patterns: + for file_path in glob.glob(pattern): + try: + os.remove(file_path) + folder_files_removed += 1 + total_files_removed += 1 + except Exception as e: + logging.warning(f"Could not remove {file_path}: {e}") + + if folder_files_removed > 0: + cleared_directories.add(folder) + print( + f" โœ… Cleared {folder_files_removed} mock files from {os.path.basename(folder)}" + ) + + print( + f" ๐Ÿ“Š Total: Cleared {total_files_removed} files from {len(cleared_directories)} directories\n" + ) + + return list(cleared_directories) + + class MockMetadata(BaseModel): """Metadata stored in mock files.""" @@ -201,8 +262,8 @@ def write_mock( return mock_file_path - def clear_mocks(self, request: pytest.FixtureRequest) -> List[str]: - """Clear all mock files in the test case folder.""" + def clear_mocks_for_test(self, request: pytest.FixtureRequest) -> List[str]: + """Clear all mock files for a single test case folder.""" cleared_files = [] patterns = [ os.path.join(self.test_case_folder, "*.txt"), @@ -568,6 +629,110 @@ def _wrap_toolsets( # For backward compatibility MockToolsets = MockToolsetManager + +def report_mock_operations( + config, mock_tracking_data: Dict[str, List], terminalreporter=None +) -> None: + """Report mock file operations and statistics.""" + # Use default parameter to safely handle missing options + generate_mocks = False + regenerate_all_mocks = False + + try: + generate_mocks = config.getoption("--generate-mocks", default=False) + regenerate_all_mocks = config.getoption("--regenerate-all-mocks", default=False) + except (AttributeError, ValueError): + # Options not available, use defaults + pass + + if not generate_mocks and not regenerate_all_mocks: + return + + regenerate_mode = regenerate_all_mocks + generated_mocks = mock_tracking_data["generated_mocks"] + mock_failures = mock_tracking_data["mock_failures"] + + # If no terminalreporter, skip output + if not terminalreporter: + return + + # Header + _safe_print(terminalreporter, f"\n{'=' * 80}") + _safe_print( + terminalreporter, + f"{'๐Ÿ”„ MOCK REGENERATION SUMMARY' if regenerate_mode else '๐Ÿ”ง MOCK GENERATION SUMMARY'}", + ) + _safe_print(terminalreporter, f"{'=' * 80}") + + # Note: Cleared directories are now handled by shared_test_infrastructure fixture + # and reported during setup phase to ensure single execution across workers + + # Generated mocks + if generated_mocks: + _safe_print( + terminalreporter, f"โœ… Generated {len(generated_mocks)} mock files:\n" + ) + + # Group by test case + by_test_case: Dict[str, List[str]] = {} + for mock_info in generated_mocks: + parts = mock_info.split(":", 2) + if len(parts) == 3: + test_case, tool_name, filename = parts + by_test_case.setdefault(test_case, []).append( + f"{tool_name} -> {filename}" + ) + + for test_case, mock_files in sorted(by_test_case.items()): + _safe_print(terminalreporter, f"๐Ÿ“ {test_case}:") + for mock_file in mock_files: + _safe_print(terminalreporter, f" - {mock_file}") + _safe_print(terminalreporter) + else: + mode_text = "regeneration" if regenerate_mode else "generation" + _safe_print( + terminalreporter, + f"โœ… Mock {mode_text} was enabled but no new mock files were created", + ) + + # Failures + if mock_failures: + _safe_print( + terminalreporter, f"โš ๏ธ {len(mock_failures)} mock-related failures occurred:" + ) + for failure in mock_failures: + _safe_print(terminalreporter, f" - {failure}") + _safe_print(terminalreporter) + + # Checklist + checklist = [ + "Review generated mock files before committing", + "Ensure mock data represents realistic scenarios", + "Check data consistency across related mocks (e.g., if a pod appears in", + " one mock, it should appear in all related mocks from the same test run)", + "Verify timestamps, IDs, and names match between interconnected mock files", + "If pod/resource names change across tool calls, regenerate ALL mocks with --regenerate-all-mocks", + ] + + _safe_print(terminalreporter, "๐Ÿ“‹ REVIEW CHECKLIST:") + for item in checklist: + _safe_print(terminalreporter, f" โ–ก {item}") + _safe_print(terminalreporter, "=" * 80) + + +def _safe_print(terminalreporter, message: str = "") -> None: + """Safely print to terminal reporter to avoid I/O errors""" + try: + terminalreporter.write_line(message) + except Exception: + # If write_line fails, try direct write + try: + terminalreporter._tw.write(message + "\n") + except Exception: + # Last resort - ignore if all writing fails + pass + + # Export list __all__ = [ "MockMode", @@ -581,4 +746,6 @@ def _wrap_toolsets( "MockableToolWrapper", "ToolsetConfigurator", "sanitize_filename", + "clear_all_mocks", + "report_mock_operations", ] diff --git a/tests/llm/utils/property_manager.py b/tests/llm/utils/property_manager.py new file mode 100644 index 0000000000..f5efe0af42 --- /dev/null +++ b/tests/llm/utils/property_manager.py @@ -0,0 +1,62 @@ +"""Manage test properties for pytest reporting.""" + +from typing import List, Any, Union +from tests.llm.utils.test_case_utils import Evaluation, HolmesTestCase # type: ignore[attr-defined] + + +def set_initial_properties(request, test_case: HolmesTestCase) -> None: + """Set initial properties at the beginning of a test so they're available even if test fails early.""" + expected = test_case.expected_output + if not isinstance(expected, list): + expected = [expected] + debug_expected = "\n- ".join(expected) + + expected_correctness_score = ( + test_case.evaluation.correctness.expected_score + if isinstance(test_case.evaluation.correctness, Evaluation) + else test_case.evaluation.correctness + ) + + # Store basic properties that should always be available + request.node.user_properties.append(("expected", debug_expected)) + request.node.user_properties.append( + ("expected_correctness_score", expected_correctness_score) + ) + request.node.user_properties.append( + ("actual", "Test not executed") + ) # Will be overwritten if test runs + request.node.user_properties.append( + ("actual_correctness_score", 0) + ) # Will be overwritten if test runs + request.node.user_properties.append( + ("tools_called", []) + ) # Will be overwritten if test runs + + +def update_property(request, key: str, value: Any) -> None: + """Update an existing property value instead of appending a duplicate.""" + for i, (prop_key, prop_value) in enumerate(request.node.user_properties): + if prop_key == key: + request.node.user_properties[i] = (key, value) + return + # If property doesn't exist, append it + request.node.user_properties.append((key, value)) + + +def update_test_results( + request, output: str, tools_called: Union[List[str], str], scores: dict +) -> None: + """Update test result properties after test execution.""" + update_property(request, "actual", output or "") + update_property( + request, + "tools_called", + tools_called if isinstance(tools_called, list) else [str(tools_called)], + ) + update_property(request, "actual_correctness_score", scores.get("correctness", 0)) + + +def update_mock_error(request, error: Exception) -> None: + """Update properties when a mock error occurs.""" + update_property(request, "actual", f"Mock data error: {str(error)}") + request.node.user_properties.append(("mock_data_failure", True)) diff --git a/tests/llm/utils/setup_cleanup.py b/tests/llm/utils/setup_cleanup.py new file mode 100644 index 0000000000..c75255b41a --- /dev/null +++ b/tests/llm/utils/setup_cleanup.py @@ -0,0 +1,156 @@ +"""Setup and cleanup infrastructure for test cases.""" + +import logging +import textwrap +import time +import warnings +from concurrent.futures import ThreadPoolExecutor, as_completed +from typing import List + +from tests.llm.utils.commands import before_test, after_test # type: ignore[attr-defined] +from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] +from tests.llm.utils.test_helpers import truncate_output + +# Configuration +MAX_ERROR_LINES = 10 +MAX_WORKERS = 30 + + +def format_error_output(error_details: str) -> str: + """Format error details with truncation if needed.""" + return truncate_output(error_details, max_lines=MAX_ERROR_LINES) + + +def run_test_commands(test_cases, command_func, operation_name): + """Generic function to run test commands (setup/cleanup) in parallel. + + Args: + test_cases: List of test cases to process + command_func: Function to call for each test case (before_test or after_test) + operation_name: Name of operation for logging ("Setup" or "Cleanup") + """ + operation_lower = operation_name.lower() + operation_plural = f"{operation_lower}s" + + print( + f"{'Setting up' if operation_name == 'Setup' else 'Cleaning up'} infrastructure {'for' if operation_name == 'Setup' else 'after tests for'} {len(test_cases)} test cases" + ) + + start_time = time.time() + successful_test_cases = 0 + failed_test_cases = 0 + timed_out_test_cases = 0 + + with ThreadPoolExecutor(max_workers=min(len(test_cases), MAX_WORKERS)) as executor: + # Submit all tasks + future_to_test_case = { + executor.submit(command_func, test_case): test_case + for test_case in test_cases + } + + # Wait for all tasks to complete and handle results + for future in as_completed(future_to_test_case): + test_case = future_to_test_case[future] + try: + result = future.result() # Single CommandResult for the test case + remaining_cases = ( + len(test_cases) + - successful_test_cases + - failed_test_cases + - timed_out_test_cases + ) + if result.success: + successful_test_cases += 1 + print( + f"โœ… {operation_name} {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); {operation_plural} remaining: {remaining_cases}" + ) + elif result.error_type == "timeout": + timed_out_test_cases += 1 + print( + f"โฐ {operation_name} {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; {operation_plural} remaining: {remaining_cases}" + ) + + # Show the exact command that timed out + truncated_error = format_error_output(result.error_details) + print(textwrap.indent(truncated_error, " ")) + logging.error( + f"[{test_case.id}] {operation_name} timeout: {result.error_details}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"{operation_name} timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", + UserWarning, + stacklevel=2, + ) + else: + failed_test_cases += 1 + print( + f"โŒ {operation_name} {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); {operation_plural} remaining: {remaining_cases}" + ) + + # Limit error details to 10 lines and add proper formatting + truncated_error = format_error_output(result.error_details) + print(textwrap.indent(truncated_error, " ")) + logging.error( + f"[{test_case.id}] {operation_name} failed: {result.error_details}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"{operation_name} failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", + UserWarning, + stacklevel=2, + ) + + except Exception as e: + failed_test_cases += 1 + print(f"โŒ {operation_name} {test_case.id}: EXCEPTION - {e}") + logging.error( + f"{operation_name} exception for {test_case.id}: {str(e)}" + ) + + # Emit warning to make it visible in pytest output + warnings.warn( + f"{operation_name} exception for test {test_case.id}: {str(e)}", + UserWarning, + stacklevel=2, + ) + + elapsed_time = time.time() - start_time + print( + f"\n๐Ÿ• {operation_name} completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" + ) + + +def run_test_setup(test_cases: List[HolmesTestCase]) -> None: + """Run before_test for each test case in parallel.""" + run_test_commands(test_cases, before_test, "Setup") + + +def run_test_cleanup(test_cases: List[HolmesTestCase]) -> None: + """Run after_test for each test case in parallel.""" + run_test_commands(test_cases, after_test, "Cleanup") + + +def extract_test_cases_needing_setup(session) -> List[HolmesTestCase]: + """Extract unique test cases that need setup from session items.""" + seen_ids = set() + test_cases = [] + + for item in session.items: + if ( + item.get_closest_marker("llm") + and hasattr(item, "callspec") + and "test_case" in item.callspec.params + ): + test_case = item.callspec.params["test_case"] + if ( + isinstance(test_case, HolmesTestCase) + and test_case.before_test + and test_case.id not in seen_ids + ): + test_cases.append(test_case) + seen_ids.add(test_case.id) + + return test_cases diff --git a/tests/llm/utils/test_mock_toolset.py b/tests/llm/utils/test_mock_toolset.py index 8f86060b83..277d73a00d 100644 --- a/tests/llm/utils/test_mock_toolset.py +++ b/tests/llm/utils/test_mock_toolset.py @@ -203,7 +203,7 @@ def test_clear_mocks(self): mock_request.node.user_properties = [] # Clear mocks - cleared = manager.clear_mocks(mock_request) + cleared = manager.clear_mocks_for_test(mock_request) assert len(cleared) == 2 assert not os.path.exists(os.path.join(tmpdir, "mock1.txt")) diff --git a/tests/llm/utils/test_results.py b/tests/llm/utils/test_results.py new file mode 100644 index 0000000000..bfc3bbd8de --- /dev/null +++ b/tests/llm/utils/test_results.py @@ -0,0 +1,101 @@ +"""Data models and utilities for test result processing.""" + +from dataclasses import dataclass +from typing import List, Optional + + +@dataclass +class TestResult: + nodeid: str + expected: str + actual: str + pass_fail: str + tools_called: List[str] + logs: str + test_type: str = "" + error_message: Optional[str] = None + execution_time: Optional[float] = None + expected_correctness_score: float = 1.0 + actual_correctness_score: float = 0.0 + mock_data_failure: bool = False + + @property + def test_id(self) -> str: + """Extract test ID from pytest nodeid. + + Example: 'test_ask_holmes[01_how_many_pods]' -> '01' + """ + if "[" in self.nodeid and "]" in self.nodeid: + test_case = self.nodeid.split("[")[1].split("]")[0] + # Extract number from start of test case name + return test_case.split("_")[0] if "_" in test_case else test_case + return "unknown" + + @property + def test_name(self) -> str: + """Extract readable test name from pytest nodeid. + + Example: 'test_ask_holmes[01_how_many_pods]' -> 'how_many_pods' + """ + try: + if "[" in self.nodeid and "]" in self.nodeid: + test_case = self.nodeid.split("[")[1].split("]")[0] + # Remove number prefix and convert underscores to spaces + parts = test_case.split("_")[1:] if "_" in test_case else [test_case] + return "_".join(parts) + except (IndexError, AttributeError): + pass + return self.nodeid.split("::")[-1] if "::" in self.nodeid else self.nodeid + + +class TestStatus: + """Encapsulates test status determination logic.""" + + def __init__(self, result: dict): + self.actual_score = int(result.get("actual_correctness_score", 0)) + self.expected_score = int(result.get("expected_correctness_score", 1)) + self.is_mock_failure = result.get("mock_data_failure", False) + + @property + def passed(self) -> bool: + return ( + self.actual_score == 1 + ) # TODO: possibly add `and not self.is_mock_failure` + + @property + def is_regression(self) -> bool: + if self.passed or self.is_mock_failure: + return False + # Known failure (expected to fail) + if self.actual_score == 0 and self.expected_score == 0: + return False + return True + + @property + def markdown_symbol(self) -> str: + if self.is_mock_failure: + return ":wrench:" + elif self.passed: + return ":white_check_mark:" + elif self.actual_score == 0 and self.expected_score == 0: + return ":warning:" + else: + return ":x:" + + @property + def console_status(self) -> str: + if self.is_mock_failure: + return "[yellow]MOCK FAILURE[/yellow]" + elif self.passed: + return "[green]PASS[/green]" + else: + return "[red]FAIL[/red]" + + @property + def short_status(self) -> str: + if self.is_mock_failure: + return "MOCK FAILURE" + elif self.passed: + return "PASS" + else: + return "FAIL" From ca82bee7cb4ea1e7f4789ed8788e3031c88cae72 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:13:16 +0300 Subject: [PATCH 03/13] fixes --- CLAUDE.md | 54 ++++++++++++++++++++++++- docs/development/evals/index.md | 53 +++++++++++++++++++++++++ holmes/plugins/toolsets/kafka.py | 8 ++++ tests/llm/conftest.py | 8 ++-- tests/llm/utils/commands.py | 22 ++--------- tests/llm/utils/setup_cleanup.py | 67 ++++++++++++++++++++------------ 6 files changed, 163 insertions(+), 49 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a50dac0f3b..04c0ab8d28 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,10 +118,62 @@ export RUN_LIVE=true poetry run pytest tests/llm/test_ask_holmes.py # Test with different models -export MODEL=anthropic/claude-3.5 +export MODEL=anthropic/claude-3.5-sonnet-20241022 poetry run pytest tests/llm/test_ask_holmes.py ``` +### Evaluation CLI Reference + +**Custom Pytest Flags**: +- `--generate-mocks`: Generate mock data files during test execution +- `--regenerate-all-mocks`: Regenerate all mock files (implies --generate-mocks) +- `--skip-setup`: Skip before_test commands (useful for iterative testing) +- `--skip-cleanup`: Skip after_test commands (useful for debugging) + +**Environment Variables**: +- `MODEL`: LLM model to use (e.g., `gpt-4o`, `anthropic/claude-3-5-sonnet-20241022`) +- `CLASSIFIER_MODEL`: Model for scoring answers (defaults to MODEL) +- `RUN_LIVE=true`: Execute real commands instead of using mocks +- `ITERATIONS=`: Run each test multiple times +- `UPLOAD_DATASET=true`: Sync dataset to Braintrust +- `EXPERIMENT_ID`: Custom experiment name for tracking +- `BRAINTRUST_API_KEY`: Enable Braintrust integration + +**Common Evaluation Patterns**: + +```bash + +# Generate/update mocks for specific tests +poetry run pytest tests/llm/test_ask_holmes.py -k "test_name" --generate-mocks + +# Run tests multiple times for reliability +ITERATIONS=100 poetry run pytest tests/llm/test_ask_holmes.py -k "flaky_test" + +# Model comparison workflow +EXPERIMENT_ID=gpt4o_baseline MODEL=gpt-4o poetry run pytest tests/llm/ -n 6 +EXPERIMENT_ID=claude35_test MODEL=anthropic/claude-3-5-sonnet-20241022 poetry run pytest tests/llm/ -n 6 + +# Debug with verbose output +poetry run pytest -vv -s tests/llm/test_ask_holmes.py -k "failing_test" --no-cov + +# List tests by marker +poetry run pytest -m "llm and not network" --collect-only -q +``` + +**Available Test Markers**: +- `llm`: LLM behavior tests +- `datetime`: Datetime functionality +- `logs`: Log processing +- `context_window`: Context window handling +- `synthetic`: Synthetic data tests +- `network`: Network-dependent tests +- `runbooks`: Runbook functionality +- `misleading-history`: Misleading data scenarios +- `k8s-misconfig`: Kubernetes misconfigurations +- `chain-of-causation`: Causation analysis +- `slackbot`: Slack integration +- `counting`: Resource counting tests + **Test Infrastructure Notes**: - All test state tracking uses pytest's `user_properties` to ensure compatibility with pytest-xdist parallel execution - Mock file tracking and test results are stored in `user_properties` and aggregated in the terminal summary diff --git a/docs/development/evals/index.md b/docs/development/evals/index.md index dc0fd4ff55..ec20de5a24 100644 --- a/docs/development/evals/index.md +++ b/docs/development/evals/index.md @@ -88,6 +88,59 @@ poetry run pytest ./tests/llm/test_ask_holmes.py -k "01_how_many_pods" --no-cov > It is possible to investigate and debug why an eval fails by the output provided in the console. The output includes the correctness score, the reasoning for the score, information about what tools were called, the expected answer, as well as the LLM's answer. +### Custom Evaluation Flags + +HolmesGPT provides custom pytest flags for evaluation workflows: + +| Flag | Description | Usage | +|------|-------------|-------| +| `--generate-mocks` | Generate mock data files during test execution | Use when adding new tests or updating existing ones | +| `--regenerate-all-mocks` | Regenerate all mock files (implies --generate-mocks) | Use to ensure mock consistency across all tests | +| `--skip-setup` | Skip `before_test` commands | Use for faster iteration during development | +| `--skip-cleanup` | Skip `after_test` commands | Use for debugging test failures | + +### Common Evaluation Patterns + +#### Rapid Test Development Workflow + +When developing or debugging tests, use this pattern for faster iteration: + +```bash +# 1. Initial run with setup (skip cleanup to keep resources) +poetry run pytest tests/llm/test_ask_holmes.py -k "specific_test" --skip-cleanup + +# 2. Quick iterations without setup/cleanup +poetry run pytest tests/llm/test_ask_holmes.py -k "specific_test" --skip-setup --skip-cleanup + +# 3. Final cleanup when done +poetry run pytest tests/llm/test_ask_holmes.py -k "specific_test" --skip-setup +``` + +#### Mock Generation + +```bash +# Generate mocks for specific test +poetry run pytest tests/llm/test_ask_holmes.py -k "test_name" --generate-mocks + +# Generate mocks with multiple iterations to cover all investigative paths +ITERATIONS=100 poetry run pytest tests/llm/test_ask_holmes.py -k "test_name" --generate-mocks + +# Regenerate all mocks for consistency +poetry run pytest tests/llm/ --regenerate-all-mocks +``` + +#### Parallel Execution + +For faster test runs, use pytest's parallel execution: + +```bash +# Run with 6 parallel workers +poetry run pytest tests/llm/ -n 6 --no-cov --disable-warnings + +# Run with auto-detected worker count +poetry run pytest tests/llm/ -n auto --no-cov --disable-warnings +``` + ### Environment Variables Configure evaluations using these environment variables: diff --git a/holmes/plugins/toolsets/kafka.py b/holmes/plugins/toolsets/kafka.py index 722422d671..47e72550f7 100644 --- a/holmes/plugins/toolsets/kafka.py +++ b/holmes/plugins/toolsets/kafka.py @@ -591,6 +591,9 @@ def prerequisites_callable(self, config: Dict[str, Any]) -> Tuple[bool, str]: admin_config = { "bootstrap.servers": cluster.kafka_broker, "client.id": cluster.kafka_client_id, + "socket.timeout.ms": 15000, # 15 second timeout + "metadata.request.timeout.ms": 15000, # 15 second metadata timeout + "api.version.request.timeout.ms": 15000, # 15 second API version timeout } if cluster.kafka_security_protocol: @@ -604,6 +607,11 @@ def prerequisites_callable(self, config: Dict[str, Any]) -> Tuple[bool, str]: admin_config["sasl.password"] = cluster.kafka_password client = AdminClient(admin_config) + # Test the connection by trying to list topics with a timeout + # This will fail fast if the broker is not reachable + future = client.list_topics(timeout=15) # 15 second timeout + if future is None: + raise Exception("Failed to connect to Kafka broker") self.clients[cluster.name] = client # Store in dictionary except Exception as e: message = ( diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index 49650c5de4..d9dcefff2b 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -19,8 +19,8 @@ from tests.llm.reporting.github_reporter import handle_github_output from tests.llm.utils.braintrust import get_braintrust_url from tests.llm.utils.setup_cleanup import ( - run_test_setup, - run_test_cleanup, + run_all_test_setup, + run_all_test_cleanup, extract_test_cases_needing_setup, ) @@ -103,7 +103,7 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo skip_setup = request.config.getoption("--skip-setup") if test_cases and not skip_setup: - run_test_setup(test_cases) + run_all_test_setup(test_cases) elif skip_setup: print("\nโญ๏ธ Skipping test setup due to --skip-setup flag") @@ -146,7 +146,7 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo cleanup_test_cases.append(test_case) if cleanup_test_cases: - run_test_cleanup(cleanup_test_cases) + run_all_test_cleanup(cleanup_test_cases) elif skip_cleanup: print("\nโญ๏ธ Skipping test cleanup due to --skip-cleanup flag") diff --git a/tests/llm/utils/commands.py b/tests/llm/utils/commands.py index 13c00a8e71..237fd6ee38 100644 --- a/tests/llm/utils/commands.py +++ b/tests/llm/utils/commands.py @@ -35,7 +35,7 @@ def exit_info(self) -> str: ) -def invoke_command(command: str, cwd: str) -> str: +def _invoke_command(command: str, cwd: str) -> str: try: logging.debug(f"Running `{command}` in {cwd}") result = subprocess.run( @@ -58,7 +58,7 @@ def invoke_command(command: str, cwd: str) -> str: raise e -def _run_commands( +def run_commands( test_case: HolmesTestCase, commands_str: str, operation: str ) -> CommandResult: """Generic command runner for setup/cleanup operations.""" @@ -80,7 +80,7 @@ def _run_commands( try: for command in commands: if command.strip(): # Skip empty lines - output = invoke_command(command=command, cwd=test_case.folder) + output = _invoke_command(command=command, cwd=test_case.folder) combined_output.append(f"$ {command}\n{output}") elapsed_time = time.time() - start_time @@ -132,22 +132,6 @@ def _run_commands( ) -def before_test(test_case: HolmesTestCase) -> CommandResult: - """Run before_test commands for a test case. - - Returns a CommandResult with success=True if no setup is needed or all commands succeed. - """ - return _run_commands(test_case, test_case.before_test, "setup") - - -def after_test(test_case: HolmesTestCase) -> CommandResult: - """Run after_test commands for a test case. - - Returns a CommandResult with success=True if no cleanup is needed or all commands succeed. - """ - return _run_commands(test_case, test_case.after_test, "cleanup") - - @contextmanager def set_test_env_vars(test_case: HolmesTestCase): """Context manager to set and restore environment variables for test execution.""" diff --git a/tests/llm/utils/setup_cleanup.py b/tests/llm/utils/setup_cleanup.py index c75255b41a..31af1f0b9b 100644 --- a/tests/llm/utils/setup_cleanup.py +++ b/tests/llm/utils/setup_cleanup.py @@ -6,8 +6,9 @@ import warnings from concurrent.futures import ThreadPoolExecutor, as_completed from typing import List +from enum import StrEnum -from tests.llm.utils.commands import before_test, after_test # type: ignore[attr-defined] +from tests.llm.utils.commands import run_commands # type: ignore[attr-defined] from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] from tests.llm.utils.test_helpers import truncate_output @@ -21,19 +22,26 @@ def format_error_output(error_details: str) -> str: return truncate_output(error_details, max_lines=MAX_ERROR_LINES) -def run_test_commands(test_cases, command_func, operation_name): - """Generic function to run test commands (setup/cleanup) in parallel. +class Operation(StrEnum): + """Enum for operation types.""" + + SETUP = "Setup" + CLEANUP = "Cleanup" + + +def run_all_test_commands(test_cases: List[HolmesTestCase], operation: Operation): + """Run before_test/after_test (according to operation) Args: test_cases: List of test cases to process command_func: Function to call for each test case (before_test or after_test) operation_name: Name of operation for logging ("Setup" or "Cleanup") """ - operation_lower = operation_name.lower() + operation_lower = operation.value.lower() operation_plural = f"{operation_lower}s" print( - f"{'Setting up' if operation_name == 'Setup' else 'Cleaning up'} infrastructure {'for' if operation_name == 'Setup' else 'after tests for'} {len(test_cases)} test cases" + f"{'Setting up' if operation == Operation.SETUP else 'Cleaning up'} infrastructure {'before' if operation == Operation.SETUP else 'after tests for'} {len(test_cases)} test cases: {', '.join(tc.id for tc in test_cases)}" ) start_time = time.time() @@ -42,11 +50,20 @@ def run_test_commands(test_cases, command_func, operation_name): timed_out_test_cases = 0 with ThreadPoolExecutor(max_workers=min(len(test_cases), MAX_WORKERS)) as executor: - # Submit all tasks - future_to_test_case = { - executor.submit(command_func, test_case): test_case - for test_case in test_cases - } + if operation == Operation.SETUP: + future_to_test_case = { + executor.submit( + run_commands, test_case, test_case.before_test, operation_lower + ): test_case + for test_case in test_cases + } + else: + future_to_test_case = { + executor.submit( + run_commands, test_case, test_case.after_test, operation_lower + ): test_case + for test_case in test_cases + } # Wait for all tasks to complete and handle results for future in as_completed(future_to_test_case): @@ -62,75 +79,75 @@ def run_test_commands(test_cases, command_func, operation_name): if result.success: successful_test_cases += 1 print( - f"โœ… {operation_name} {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); {operation_plural} remaining: {remaining_cases}" + f"โœ… {operation.value} {test_case.id}: {result.command} ({result.elapsed_time:.2f}s); {operation_plural} remaining: {remaining_cases}" ) elif result.error_type == "timeout": timed_out_test_cases += 1 print( - f"โฐ {operation_name} {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; {operation_plural} remaining: {remaining_cases}" + f"โฐ {operation.value} {test_case.id}: TIMEOUT after {result.elapsed_time:.2f}s; {operation_plural} remaining: {remaining_cases}" ) # Show the exact command that timed out truncated_error = format_error_output(result.error_details) print(textwrap.indent(truncated_error, " ")) logging.error( - f"[{test_case.id}] {operation_name} timeout: {result.error_details}" + f"[{test_case.id}] {operation.value} timeout: {result.error_details}" ) # Emit warning to make it visible in pytest output warnings.warn( - f"{operation_name} timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", + f"{operation.value} timeout for test {test_case.id}: Command '{result.command}' timed out after {result.elapsed_time:.2f}s. Output: {result.error_details}", UserWarning, stacklevel=2, ) else: failed_test_cases += 1 print( - f"โŒ {operation_name} {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); {operation_plural} remaining: {remaining_cases}" + f"โŒ {operation.value} {test_case.id}: FAILED ({result.exit_info}, {result.elapsed_time:.2f}s); {operation_plural} remaining: {remaining_cases}" ) # Limit error details to 10 lines and add proper formatting truncated_error = format_error_output(result.error_details) print(textwrap.indent(truncated_error, " ")) logging.error( - f"[{test_case.id}] {operation_name} failed: {result.error_details}" + f"[{test_case.id}] {operation.value} failed: {result.error_details}" ) # Emit warning to make it visible in pytest output warnings.warn( - f"{operation_name} failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", + f"{operation.value} failed for test {test_case.id}: Command '{result.command}' failed with {result.exit_info} in {result.elapsed_time:.2f}s. Output: {result.error_details}", UserWarning, stacklevel=2, ) except Exception as e: failed_test_cases += 1 - print(f"โŒ {operation_name} {test_case.id}: EXCEPTION - {e}") + print(f"โŒ {operation.value} {test_case.id}: EXCEPTION - {e}") logging.error( - f"{operation_name} exception for {test_case.id}: {str(e)}" + f"{operation.value} exception for {test_case.id}: {str(e)}" ) # Emit warning to make it visible in pytest output warnings.warn( - f"{operation_name} exception for test {test_case.id}: {str(e)}", + f"{operation.value} exception for test {test_case.id}: {str(e)}", UserWarning, stacklevel=2, ) elapsed_time = time.time() - start_time print( - f"\n๐Ÿ• {operation_name} completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" + f"\n๐Ÿ• {operation.value} completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" ) -def run_test_setup(test_cases: List[HolmesTestCase]) -> None: +def run_all_test_setup(test_cases: List[HolmesTestCase]) -> None: """Run before_test for each test case in parallel.""" - run_test_commands(test_cases, before_test, "Setup") + run_all_test_commands(test_cases, Operation.SETUP) -def run_test_cleanup(test_cases: List[HolmesTestCase]) -> None: +def run_all_test_cleanup(test_cases: List[HolmesTestCase]) -> None: """Run after_test for each test case in parallel.""" - run_test_commands(test_cases, after_test, "Cleanup") + run_all_test_commands(test_cases, Operation.CLEANUP) def extract_test_cases_needing_setup(session) -> List[HolmesTestCase]: From 4edef7a25b53da112326def75fae96a546380ce8 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:16:33 +0300 Subject: [PATCH 04/13] update docs --- docs/development/evals/index.md | 44 +++++++++++++++++++++++++++++++ docs/development/evals/writing.md | 23 ++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/docs/development/evals/index.md b/docs/development/evals/index.md index ec20de5a24..c753f1a114 100644 --- a/docs/development/evals/index.md +++ b/docs/development/evals/index.md @@ -191,6 +191,35 @@ Live testing requires a Kubernetes cluster and will execute `before-test` and `a 3. **Compare Results**: Use evaluation tracking tools to analyze performance differences +## Test Markers + +Filter tests using pytest markers: + +```bash +# Run only LLM tests +poetry run pytest -m "llm" + +# Run tests that don't require network +poetry run pytest -m "not network" + +# Combine markers +poetry run pytest -m "llm and not synthetic" +``` + +**Available markers:** +- `llm` - LLM behavior tests +- `datetime` - Datetime functionality tests +- `logs` - Log processing tests +- `context_window` - Context window handling tests +- `synthetic` - Tests using synthetic data +- `network` - Tests requiring network connectivity +- `runbooks` - Runbook functionality tests +- `misleading-history` - Tests with misleading historical data +- `k8s-misconfig` - Kubernetes misconfiguration tests +- `chain-of-causation` - Chain of causation analysis tests +- `slackbot` - Slack integration tests +- `counting` - Resource counting tests + ## Troubleshooting ### Common Issues @@ -212,3 +241,18 @@ This shows detailed output including: - Tool calls made by the LLM - Evaluation scores and rationales - Debugging information + +### Common Pytest Flags + +| Flag | Description | +|------|--------------| +| `-n ` | Run tests in parallel with specified workers | +| `-k ` | Run tests matching the pattern | +| `-m ` | Run tests with specific marker | +| `-v/-vv` | Verbose output (more v's = more verbose) | +| `-s` | Show print statements | +| `--no-cov` | Disable coverage reporting | +| `--disable-warnings` | Disable warning summary | +| `--collect-only` | List tests without running | +| `-q` | Quiet mode | +| `--timeout=` | Set test timeout | diff --git a/docs/development/evals/writing.md b/docs/development/evals/writing.md index 9f19e2b8c0..9e490b25ac 100644 --- a/docs/development/evals/writing.md +++ b/docs/development/evals/writing.md @@ -287,6 +287,29 @@ pytest ./tests/llm/test_ask_holmes.py -k "your_test" --generate-mocks # Or regenerate ALL mocks to ensure consistency pytest ./tests/llm/test_ask_holmes.py -k "your_test" --regenerate-all-mocks + +# Skip setup/cleanup for faster debugging +pytest ./tests/llm/test_ask_holmes.py -k "your_test" --skip-setup --skip-cleanup + +# Run with specific number of iterations +ITERATIONS=10 pytest ./tests/llm/test_ask_holmes.py -k "your_test" ``` +### CLI Flags Reference + +**Custom HolmesGPT Flags:** +- `--generate-mocks` - Generate mock files during test execution +- `--regenerate-all-mocks` - Regenerate all mock files (implies --generate-mocks) +- `--skip-setup` - Skip `before_test` commands +- `--skip-cleanup` - Skip `after_test` commands + +**Common Pytest Flags:** +- `-n ` - Run tests in parallel +- `-k ` - Run tests matching pattern +- `-m ` - Run tests with specific marker +- `-v/-vv` - Verbose output +- `-s` - Show print statements +- `--no-cov` - Disable coverage +- `--collect-only` - List tests without running + This completes the evaluation writing guide. The next step is setting up reporting and analysis using Braintrust. From f238489fe5354fdc5429a4e57c41bd38903b855f Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:18:45 +0300 Subject: [PATCH 05/13] PR fixes --- tests/llm/conftest.py | 14 +++----------- tests/llm/{ => utils}/reporting/__init__.py | 0 tests/llm/{ => utils}/reporting/github_reporter.py | 0 .../llm/{ => utils}/reporting/property_manager.py | 0 .../llm/{ => utils}/reporting/terminal_reporter.py | 0 5 files changed, 3 insertions(+), 11 deletions(-) rename tests/llm/{ => utils}/reporting/__init__.py (100%) rename tests/llm/{ => utils}/reporting/github_reporter.py (100%) rename tests/llm/{ => utils}/reporting/property_manager.py (100%) rename tests/llm/{ => utils}/reporting/terminal_reporter.py (100%) diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index d9dcefff2b..ef7cb09cfc 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -15,8 +15,8 @@ MockGenerationConfig, report_mock_operations, ) -from tests.llm.reporting.terminal_reporter import handle_console_output -from tests.llm.reporting.github_reporter import handle_github_output +from tests.llm.utils.reporting.terminal_reporter import handle_console_output +from tests.llm.utils.reporting.github_reporter import handle_github_output from tests.llm.utils.braintrust import get_braintrust_url from tests.llm.utils.setup_cleanup import ( run_all_test_setup, @@ -198,7 +198,7 @@ def check_llm_api_with_test_call(): @pytest.fixture(scope="session", autouse=True) -def llm_availablity_check(request): +def llm_availability_check(request): """Handle LLM test session setup: show warning, check API, and skip if needed""" # Don't show messages during collection-only mode # Check if we're in collect-only mode @@ -332,9 +332,7 @@ def braintrust_eval_link(request): def show_llm_summary_report(terminalreporter, exitstatus, config): """Generate GitHub Actions report and Rich summary table from terminalreporter.stats (xdist compatible)""" - print("\n\n[DEBUG] pytest_terminal_summary called!") if not hasattr(terminalreporter, "stats"): - print("[DEBUG] terminalreporter has no stats attribute") return # When using xdist, only the master process should display the summary @@ -353,9 +351,7 @@ def show_llm_summary_report(terminalreporter, exitstatus, config): terminalreporter ) - print(f"[DEBUG] Found {len(sorted_results)} test results") if not sorted_results: - print("[DEBUG] No sorted results found, returning") return # Handle GitHub/CI output (markdown + file writing) @@ -370,9 +366,6 @@ def show_llm_summary_report(terminalreporter, exitstatus, config): def _collect_test_results_from_stats(terminalreporter): """Collect and parse test results from terminalreporter.stats.""" - print( - f"[DEBUG] _collect_test_results_from_stats called, stats keys: {list(terminalreporter.stats.keys())}" - ) test_results = {} mock_tracking_data = { "generated_mocks": [], @@ -387,7 +380,6 @@ def _collect_test_results_from_stats(terminalreporter): ] for status, reports in terminalreporter.stats.items(): - print(f"[DEBUG] Status '{status}' has {len(reports)} reports") for report in reports: # Only process 'call' phase reports for actual test results if getattr(report, "when", None) != "call": diff --git a/tests/llm/reporting/__init__.py b/tests/llm/utils/reporting/__init__.py similarity index 100% rename from tests/llm/reporting/__init__.py rename to tests/llm/utils/reporting/__init__.py diff --git a/tests/llm/reporting/github_reporter.py b/tests/llm/utils/reporting/github_reporter.py similarity index 100% rename from tests/llm/reporting/github_reporter.py rename to tests/llm/utils/reporting/github_reporter.py diff --git a/tests/llm/reporting/property_manager.py b/tests/llm/utils/reporting/property_manager.py similarity index 100% rename from tests/llm/reporting/property_manager.py rename to tests/llm/utils/reporting/property_manager.py diff --git a/tests/llm/reporting/terminal_reporter.py b/tests/llm/utils/reporting/terminal_reporter.py similarity index 100% rename from tests/llm/reporting/terminal_reporter.py rename to tests/llm/utils/reporting/terminal_reporter.py From bb757a99313bbab6b1ad79b23e5d0ae3fdd6adee Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:20:13 +0300 Subject: [PATCH 06/13] fix github report --- tests/llm/utils/reporting/github_reporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/llm/utils/reporting/github_reporter.py b/tests/llm/utils/reporting/github_reporter.py index 0015a4fd31..68bfa15961 100644 --- a/tests/llm/utils/reporting/github_reporter.py +++ b/tests/llm/utils/reporting/github_reporter.py @@ -90,7 +90,7 @@ def generate_markdown_report(sorted_results: List[dict]) -> Tuple[str, List[dict for result in sorted_results: test_suite = result["test_type"] - test_name = f"{result['test_id']}: {result['test_name']}" + test_name = f"{result['test_id']}_{result['test_name']}" # Add Braintrust link to test name if available test_suite_full = ( From 7de59c5e3173d7c06f904502f625cd5a0156b829 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:21:52 +0300 Subject: [PATCH 07/13] Update build-and-test.yaml --- .github/workflows/build-and-test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index 6ffdfc15cc..85601a58cf 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -25,7 +25,7 @@ jobs: needs: check strategy: matrix: - python-version: ["3.10", "3.11", "3.12"] + python-version: ["3.11", "3.12"] runs-on: ubuntu-latest From 91237477da906b65cbf23b6d73081c4b96b9e099 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:23:22 +0300 Subject: [PATCH 08/13] better fix --- .github/workflows/build-and-test.yaml | 2 +- tests/llm/utils/setup_cleanup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index 85601a58cf..6ffdfc15cc 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -25,7 +25,7 @@ jobs: needs: check strategy: matrix: - python-version: ["3.11", "3.12"] + python-version: ["3.10", "3.11", "3.12"] runs-on: ubuntu-latest diff --git a/tests/llm/utils/setup_cleanup.py b/tests/llm/utils/setup_cleanup.py index 31af1f0b9b..372ef90463 100644 --- a/tests/llm/utils/setup_cleanup.py +++ b/tests/llm/utils/setup_cleanup.py @@ -6,7 +6,7 @@ import warnings from concurrent.futures import ThreadPoolExecutor, as_completed from typing import List -from enum import StrEnum +from strenum import StrEnum from tests.llm.utils.commands import run_commands # type: ignore[attr-defined] from tests.llm.utils.test_case_utils import HolmesTestCase # type: ignore[attr-defined] From 3d398066f4acdbcf97653df459987ce9721620cf Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 12:34:47 +0300 Subject: [PATCH 09/13] remove unrelated change --- holmes/plugins/toolsets/kafka.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/holmes/plugins/toolsets/kafka.py b/holmes/plugins/toolsets/kafka.py index 47e72550f7..722422d671 100644 --- a/holmes/plugins/toolsets/kafka.py +++ b/holmes/plugins/toolsets/kafka.py @@ -591,9 +591,6 @@ def prerequisites_callable(self, config: Dict[str, Any]) -> Tuple[bool, str]: admin_config = { "bootstrap.servers": cluster.kafka_broker, "client.id": cluster.kafka_client_id, - "socket.timeout.ms": 15000, # 15 second timeout - "metadata.request.timeout.ms": 15000, # 15 second metadata timeout - "api.version.request.timeout.ms": 15000, # 15 second API version timeout } if cluster.kafka_security_protocol: @@ -607,11 +604,6 @@ def prerequisites_callable(self, config: Dict[str, Any]) -> Tuple[bool, str]: admin_config["sasl.password"] = cluster.kafka_password client = AdminClient(admin_config) - # Test the connection by trying to list topics with a timeout - # This will fail fast if the broker is not reachable - future = client.list_topics(timeout=15) # 15 second timeout - if future is None: - raise Exception("Failed to connect to Kafka broker") self.clients[cluster.name] = client # Store in dictionary except Exception as e: message = ( From 2cf2f538afee4332606b9f60cb3cc65d5fc6780d Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 13:24:56 +0300 Subject: [PATCH 10/13] further improvements --- tests/llm/conftest.py | 9 +++++---- tests/llm/utils/setup_cleanup.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index 3536e4eb78..9b10ee0378 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -73,20 +73,20 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo # If we're in collect-only mode or RUN_LIVE is not set, skip setup/cleanup entirely if collect_only or mock_generation_config.mode == MockMode.MOCK: log( - f"โš™๏ธ Skipping shared test infrastructure setup/cleanup (mode: {mock_generation_config.mode}, collect_only: {collect_only})" + f"\nโš™๏ธ Skipping shared test infrastructure setup/cleanup on worker (mode: {mock_generation_config.mode}, collect_only: {collect_only})" ) # Must yield twice even when skipping due to how pytest-shared-session-scope works initial = yield cleanup_token = yield {"test_cases_for_cleanup": []} return - log( - f"โš™๏ธ Running shared test infrastructure setup/cleanup (mode: {mock_generation_config.mode}, collect_only: {collect_only})" - ) # First yield: get initial value (SetupToken.FIRST if first worker, data if subsequent) initial = yield if initial is SetupToken.FIRST: + log( + "\nโš™๏ธ Running shared test infrastructure setup/cleanup on worker chosen for setup" + ) # This is the first worker to run the fixture test_cases = extract_test_cases_needing_setup(request.session) @@ -113,6 +113,7 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo "cleared_mock_directories": cleared_directories, } else: + log("โš™๏ธ NOT running shared test infrastructure setup/cleanup on other worker.") # This is a worker using the fixture after the first worker data = initial diff --git a/tests/llm/utils/setup_cleanup.py b/tests/llm/utils/setup_cleanup.py index f0220d7eb4..0463380b13 100644 --- a/tests/llm/utils/setup_cleanup.py +++ b/tests/llm/utils/setup_cleanup.py @@ -20,6 +20,7 @@ def log(msg): """Force a log to be written even with xdist, which captures stdout.""" sys.stderr.write(msg) + sys.stderr.write("\n") def format_error_output(error_details: str) -> str: @@ -46,7 +47,7 @@ def run_all_test_commands(test_cases: List[HolmesTestCase], operation: Operation operation_plural = f"{operation_lower}s" log( - f"โš™๏ธ {'Setting up' if operation == Operation.SETUP else 'Cleaning up'} infrastructure {'before' if operation == Operation.SETUP else 'after tests for'} {len(test_cases)} test cases: {', '.join(tc.id for tc in test_cases)}" + f"\nโš™๏ธ {'Setting up' if operation == Operation.SETUP else 'Cleaning up'} infrastructure {'before' if operation == Operation.SETUP else 'after tests for'} {len(test_cases)} test cases: {', '.join(tc.id for tc in test_cases)}" ) start_time = time.time() @@ -80,7 +81,7 @@ def run_all_test_commands(test_cases: List[HolmesTestCase], operation: Operation - successful_test_cases - failed_test_cases - timed_out_test_cases - ) + ) - 1 # Subtract 1 for the current test case if result.success: successful_test_cases += 1 log( @@ -139,7 +140,7 @@ def run_all_test_commands(test_cases: List[HolmesTestCase], operation: Operation elapsed_time = time.time() - start_time log( - f"\nโš™๏ธ {operation.value} completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" + f"โš™๏ธ {operation.value} completed in {elapsed_time:.2f}s: {successful_test_cases} successful, {failed_test_cases} failed, {timed_out_test_cases} timeout" ) From 1b0ea0143fc69a4363cb422f244d888d546c6324 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 13:46:25 +0300 Subject: [PATCH 11/13] tweaks --- tests/llm/conftest.py | 8 +++----- tests/llm/utils/setup_cleanup.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index 9b10ee0378..9084153220 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -69,11 +69,12 @@ def mock_generation_config(request): def shared_test_infrastructure(request, mock_generation_config: MockGenerationConfig): """Shared session-scoped fixture for test infrastructure setup/cleanup coordination""" collect_only = request.config.getoption("--collect-only") + worker_id = getattr(request.config, "workerinput", {}).get("workerid", None) # If we're in collect-only mode or RUN_LIVE is not set, skip setup/cleanup entirely if collect_only or mock_generation_config.mode == MockMode.MOCK: log( - f"\nโš™๏ธ Skipping shared test infrastructure setup/cleanup on worker (mode: {mock_generation_config.mode}, collect_only: {collect_only})" + f"\nโš™๏ธ Skipping shared test infrastructure setup/cleanup on worker {worker_id} (mode: {mock_generation_config.mode}, collect_only: {collect_only})" ) # Must yield twice even when skipping due to how pytest-shared-session-scope works initial = yield @@ -84,9 +85,6 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo initial = yield if initial is SetupToken.FIRST: - log( - "\nโš™๏ธ Running shared test infrastructure setup/cleanup on worker chosen for setup" - ) # This is the first worker to run the fixture test_cases = extract_test_cases_needing_setup(request.session) @@ -113,7 +111,7 @@ def shared_test_infrastructure(request, mock_generation_config: MockGenerationCo "cleared_mock_directories": cleared_directories, } else: - log("โš™๏ธ NOT running shared test infrastructure setup/cleanup on other worker.") + log(f"โš™๏ธ Skipping before_test/after_test on worker {worker_id}") # This is a worker using the fixture after the first worker data = initial diff --git a/tests/llm/utils/setup_cleanup.py b/tests/llm/utils/setup_cleanup.py index 0463380b13..a6358e12e6 100644 --- a/tests/llm/utils/setup_cleanup.py +++ b/tests/llm/utils/setup_cleanup.py @@ -47,7 +47,7 @@ def run_all_test_commands(test_cases: List[HolmesTestCase], operation: Operation operation_plural = f"{operation_lower}s" log( - f"\nโš™๏ธ {'Setting up' if operation == Operation.SETUP else 'Cleaning up'} infrastructure {'before' if operation == Operation.SETUP else 'after tests for'} {len(test_cases)} test cases: {', '.join(tc.id for tc in test_cases)}" + f"\nโš™๏ธ {'Running before_test' if operation == Operation.SETUP else 'Running after_test'} for {len(test_cases)} unique evals: {', '.join(tc.id for tc in test_cases)}" ) start_time = time.time() From 45b66aa9720c353483e8a0e3e654e7104eece6f3 Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 14:20:03 +0300 Subject: [PATCH 12/13] fix issues tomer found --- holmes/core/tool_calling_llm.py | 6 ++++-- holmes/core/tracing.py | 9 +++++---- tests/llm/conftest.py | 5 ++++- tests/llm/test_ask_holmes.py | 9 +-------- tests/llm/test_investigate.py | 2 +- tests/llm/test_workload_health.py | 5 +++-- tests/llm/utils/setup_cleanup.py | 3 +++ tests/llm/utils/test_helpers.py | 17 ----------------- 8 files changed, 21 insertions(+), 35 deletions(-) diff --git a/holmes/core/tool_calling_llm.py b/holmes/core/tool_calling_llm.py index 407b5f1595..8804d8079e 100644 --- a/holmes/core/tool_calling_llm.py +++ b/holmes/core/tool_calling_llm.py @@ -39,7 +39,7 @@ ) from holmes.utils.tags import format_tags_in_string, parse_messages_tags from holmes.core.tools_utils.tool_executor import ToolExecutor -from holmes.core.tracing import DummySpan, SpanType +from holmes.core.tracing import DummySpan def format_tool_result_data(tool_result: StructuredToolResult) -> str: @@ -422,7 +422,7 @@ def _invoke_tool( tool_response = None # Create tool span if tracing is enabled - tool_span = trace_span.start_span(name=tool_name, type=SpanType.TOOL) + tool_span = trace_span.start_span(name=tool_name, type="tool") try: tool_response = prevent_overly_repeated_tool_call( @@ -451,6 +451,8 @@ def _invoke_tool( metadata={ "status": tool_response.status.value, "error": tool_response.error, + "description": tool.get_parameterized_one_liner(tool_params), + "structured_tool_result": tool_response, }, ) diff --git a/holmes/core/tracing.py b/holmes/core/tracing.py index 8d5524c90e..357dcee1f9 100644 --- a/holmes/core/tracing.py +++ b/holmes/core/tracing.py @@ -32,12 +32,13 @@ class SpanType(Enum): TOOL = "tool" TASK = "task" SCORE = "score" + EVAL = "eval" class DummySpan: """A no-op span implementation for when tracing is disabled.""" - def start_span(self, name: str, span_type: Optional[SpanType] = None, **kwargs): + def start_span(self, name: str, span_type=None, **kwargs): return DummySpan() def log(self, *args, **kwargs): @@ -121,17 +122,17 @@ def start_trace( # Add span type to kwargs if provided kwargs = {} if span_type: - kwargs["type"] = getattr(SpanTypeAttribute, span_type.name) + kwargs["type"] = span_type.value # Use current Braintrust context (experiment or parent span) current_span = braintrust.current_span() if not _is_noop_span(current_span): - return current_span.start_span(name=name, **kwargs) + return current_span.start_span(name=name, **kwargs) # type: ignore # Fallback to current experiment current_experiment = braintrust.current_experiment() if current_experiment: - return current_experiment.start_span(name=name, **kwargs) + return current_experiment.start_span(name=name, **kwargs) # type: ignore return DummySpan() diff --git a/tests/llm/conftest.py b/tests/llm/conftest.py index 9084153220..f1322742db 100644 --- a/tests/llm/conftest.py +++ b/tests/llm/conftest.py @@ -325,7 +325,10 @@ def braintrust_eval_link(request): ) with force_pytest_output(request): - print(f"\n๐Ÿ” View eval result: {braintrust_url}") + # Use ANSI escape codes to create a clickable link in terminals that support it + # Format: \033]8;;URL\033\\TEXT\033]8;;\033\\ + clickable_url = f"\033]8;;{braintrust_url}\033\\{braintrust_url}\033]8;;\033\\" + print(f"\n๐Ÿ” View eval result: \033[94m{clickable_url}\033[0m") print() diff --git a/tests/llm/test_ask_holmes.py b/tests/llm/test_ask_holmes.py index ae6103aaae..a2f067ae51 100644 --- a/tests/llm/test_ask_holmes.py +++ b/tests/llm/test_ask_holmes.py @@ -31,7 +31,6 @@ from tests.llm.utils.tags import add_tags_to_eval from holmes.core.tracing import SpanType from tests.llm.utils.test_helpers import ( - log_tool_calls_to_spans, print_expected_output, print_correctness_evaluation, print_tool_calls_summary, @@ -120,7 +119,7 @@ def test_ask_holmes( try: with tracer.start_trace( - name=test_case.id, span_type=SpanType.TASK + name=test_case.id, span_type=SpanType.EVAL ) as eval_span: # Store span info in user properties for conftest to access if hasattr(eval_span, "id"): @@ -132,8 +131,6 @@ def test_ask_holmes( ("braintrust_root_span_id", str(eval_span.root_span_id)) ) - # Setup is handled by session-scoped fixture now - # Mock datetime if mocked_date is provided if test_case.mocked_date: mocked_datetime = datetime.fromisoformat( @@ -162,10 +159,6 @@ def test_ask_holmes( request=request, ) - if result.tool_calls: - # Log tool calls to Braintrust spans - log_tool_calls_to_spans(result.tool_calls, eval_span) - except Exception as e: # Log error to span if available try: diff --git a/tests/llm/test_investigate.py b/tests/llm/test_investigate.py index cd10a35771..95535af25d 100644 --- a/tests/llm/test_investigate.py +++ b/tests/llm/test_investigate.py @@ -149,7 +149,7 @@ def test_investigate( os.environ, {"HOLMES_STRUCTURED_OUTPUT_CONVERSION_FEATURE_FLAG": "False"} ): with tracer.start_trace( - name=test_case.id, span_type=SpanType.TASK + name=test_case.id, span_type=SpanType.EVAL ) as eval_span: # Store span info in user properties for conftest to access if hasattr(eval_span, "id"): diff --git a/tests/llm/test_workload_health.py b/tests/llm/test_workload_health.py index 99ea18fc53..f2e68eff9a 100644 --- a/tests/llm/test_workload_health.py +++ b/tests/llm/test_workload_health.py @@ -6,6 +6,7 @@ import pytest from server import workload_health_check +from holmes.core.tracing import SpanType from holmes.core.tools_utils.tool_executor import ToolExecutor import tests.llm.utils.braintrust as braintrust_util from holmes.config import Config @@ -25,7 +26,7 @@ ) from tests.llm.utils.property_manager import set_initial_properties, update_test_results from os import path -from braintrust import Span, SpanTypeAttribute +from braintrust import Span from unittest.mock import patch from tests.llm.utils.tags import add_tags_to_eval @@ -135,7 +136,7 @@ def test_health_check( metadata = get_machine_state_tags() metadata["model"] = config.model or "Unknown" with patch.multiple("server", dal=mock_dal, config=config): - with eval_span.start_span("Holmes Run", type=SpanTypeAttribute.LLM): + with eval_span.start_span("Holmes Run", type=SpanType.LLM): result = workload_health_check(request=input) assert result, "No result returned by workload_health_check()" diff --git a/tests/llm/utils/setup_cleanup.py b/tests/llm/utils/setup_cleanup.py index a6358e12e6..df6e3789ff 100644 --- a/tests/llm/utils/setup_cleanup.py +++ b/tests/llm/utils/setup_cleanup.py @@ -1,5 +1,6 @@ """Setup and cleanup infrastructure for test cases.""" +import logging import sys import textwrap import time @@ -21,6 +22,8 @@ def log(msg): """Force a log to be written even with xdist, which captures stdout.""" sys.stderr.write(msg) sys.stderr.write("\n") + # we also log to stderr so its visible when xdist is not used + logging.info(msg) def format_error_output(error_details: str) -> str: diff --git a/tests/llm/utils/test_helpers.py b/tests/llm/utils/test_helpers.py index 31359a8ce2..8a8f688f52 100644 --- a/tests/llm/utils/test_helpers.py +++ b/tests/llm/utils/test_helpers.py @@ -58,20 +58,3 @@ def print_correctness_evaluation(correctness_eval: Any) -> None: for line in rationale.split("\n"): if line.strip(): print(f" {line}") - - -def log_tool_calls_to_spans(tool_calls: List[Any], parent_span: Any) -> None: - """Log tool calls to Braintrust spans for traceability.""" - if not tool_calls or not parent_span: - return - - for tc in tool_calls: - with parent_span.start_span(name=tc.tool_name, type="tool") as tool_span: - tool_span.log( - input={"description": tc.description}, - output={ - "data": tc.result.data - if hasattr(tc.result, "data") - else str(tc.result) - }, - ) From cd1ad765371050d85683276d109db6113d6c799d Mon Sep 17 00:00:00 2001 From: Robusta Runner Date: Thu, 24 Jul 2025 14:29:03 +0300 Subject: [PATCH 13/13] Update commands.py --- tests/llm/utils/commands.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/llm/utils/commands.py b/tests/llm/utils/commands.py index 237fd6ee38..791ba80a02 100644 --- a/tests/llm/utils/commands.py +++ b/tests/llm/utils/commands.py @@ -8,6 +8,9 @@ from tests.llm.utils.test_case_utils import HolmesTestCase +EVAL_SETUP_TIMEOUT = int(os.environ.get("EVAL_SETUP_TIMEOUT", "180")) + + class CommandResult: def __init__( self, @@ -46,6 +49,7 @@ def _invoke_command(command: str, cwd: str) -> str: check=True, stdin=subprocess.DEVNULL, cwd=cwd, + timeout=EVAL_SETUP_TIMEOUT, ) output = f"{result.stdout}\n{result.stderr}" @@ -107,7 +111,7 @@ def run_commands( except subprocess.TimeoutExpired as e: elapsed_time = time.time() - start_time error_details = "\n".join(combined_output) - error_details += f"\n$ {e.cmd}\nTIMEOUT after {e.timeout}s" + error_details += f"\n$ {e.cmd}\nTIMEOUT after {e.timeout}s; You can increase timeout with environment variable EVAL_SETUP_TIMEOUT=" return CommandResult( command=f"{operation.capitalize()} timeout: {e.cmd}",