From cfcea962005e1691e14028b50a48b0f5b5432ae2 Mon Sep 17 00:00:00 2001 From: Agent VM Date: Mon, 15 Jun 2026 19:54:27 -0400 Subject: [PATCH 1/4] test: add tests for BaseProcessor.build_test_info() Part of RPOPC-1316. Tests verify wrapper version extraction from test_info file and document current behavior where both version and wrapper_version are set to the same value. These tests establish baseline behavior before adding documentation that clarifies processors should override for benchmark-specific versions. --- tests/test_base_processor.py | 108 +++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/test_base_processor.py diff --git a/tests/test_base_processor.py b/tests/test_base_processor.py new file mode 100644 index 0000000..75a5c34 --- /dev/null +++ b/tests/test_base_processor.py @@ -0,0 +1,108 @@ +""" +Tests for BaseProcessor.build_test_info() method. + +Tests verify wrapper version extraction from test_info file +and document the current behavior where both version and +wrapper_version fields are set to the same value. +""" + +import json +import pytest +from pathlib import Path + +from chronicler.processors.base_processor import BaseProcessor +from chronicler.schema import TestInfo + +pytestmark = pytest.mark.unit + + +class MinimalProcessor(BaseProcessor): + """Minimal concrete processor for testing BaseProcessor methods.""" + + def get_test_name(self) -> str: + return "minimal_test" + + def parse_runs(self, extracted_result): + return {} + + +def test_build_test_info_extracts_wrapper_version_from_test_info(tmp_path): + """ + Verify build_test_info() extracts wrapper version from test_info file. + + The test_info file contains wrapper repository versions (e.g., "v2.8") + which should be extracted and set to both version and wrapper_version + fields (current behavior). + """ + # Setup: Create test_info file with wrapper version + test_info_data = { + "minimal_test": { + "test_name": "minimal_test", + "repo_file": "v2.8.tar.gz" + } + } + test_info_file = tmp_path / "test_info" + test_info_file.write_text(json.dumps(test_info_data)) + + # Execute + processor = MinimalProcessor(str(tmp_path)) + result = processor.build_test_info() + + # Verify + assert isinstance(result, TestInfo) + assert result.name == "minimal_test" + assert result.version == "v2.8", "Should extract wrapper version from repo_file" + assert result.wrapper_version == "v2.8", "Should set wrapper_version to same value" + + +def test_build_test_info_returns_unknown_when_no_test_info_file(tmp_path): + """ + When test_info file is missing, both version fields should be "unknown". + """ + # Execute (no test_info file created) + processor = MinimalProcessor(str(tmp_path)) + result = processor.build_test_info() + + # Verify + assert result.version == "unknown" + assert result.wrapper_version == "unknown" + + +def test_build_test_info_returns_unknown_when_test_not_in_test_info(tmp_path): + """ + When test_info exists but doesn't contain the test, return "unknown". + """ + # Setup: Create test_info with different test + test_info_data = { + "other_test": { + "test_name": "other_test", + "repo_file": "v1.0.tar.gz" + } + } + test_info_file = tmp_path / "test_info" + test_info_file.write_text(json.dumps(test_info_data)) + + # Execute + processor = MinimalProcessor(str(tmp_path)) + result = processor.build_test_info() + + # Verify + assert result.version == "unknown" + assert result.wrapper_version == "unknown" + + +def test_build_test_info_handles_malformed_json(tmp_path): + """ + Malformed test_info file should log warning and return "unknown". + """ + # Setup: Create invalid JSON + test_info_file = tmp_path / "test_info" + test_info_file.write_text("{ invalid json }") + + # Execute + processor = MinimalProcessor(str(tmp_path)) + result = processor.build_test_info() + + # Verify + assert result.version == "unknown" + assert result.wrapper_version == "unknown" From f32197650c8bca0c06f4835289db700749b919e7 Mon Sep 17 00:00:00 2001 From: Agent VM Date: Mon, 15 Jun 2026 19:56:06 -0400 Subject: [PATCH 2/4] docs(base_processor): clarify version vs wrapper_version in build_test_info() Implements RPOPC-1316. Enhanced docstring and inline comments to explicitly document that: - Both test.version and test.wrapper_version are set to wrapper version by default - This is intentional base behavior but semantically incorrect for most benchmarks - Processors with independent benchmark versions (FIO, STREAMS, CoreMark, etc.) MUST override build_test_info() to extract benchmark-specific versions - Provides override pattern example and references VERSION_CONFLATION_IMPACT.md This establishes the pattern for processor-specific fixes (10/12 processors affected per impact analysis). No functional changes - purely documentation. --- src/chronicler/processors/base_processor.py | 43 +++++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/chronicler/processors/base_processor.py b/src/chronicler/processors/base_processor.py index f373b9c..1253386 100644 --- a/src/chronicler/processors/base_processor.py +++ b/src/chronicler/processors/base_processor.py @@ -196,10 +196,42 @@ def build_metadata(self) -> Metadata: ) def build_test_info(self) -> TestInfo: - """Build test information section""" + """ + Build test information section. + + By default, this method extracts the wrapper version from the test_info + file (the orchestrator's test repository version, e.g., "v2.8") and sets + BOTH test.version and test.wrapper_version to this same value. + + IMPORTANT: This is intentional base behavior, but often incorrect semantically. + For benchmarks with independent versioning (e.g., FIO 3.36, STREAMS 5.10), + processors MUST override this method to extract and return the benchmark's + own version in test.version while preserving wrapper_version. + + Examples of processors that should override: + - FIO: test.version = "fio-3.36", wrapper_version = "v2.1" + - STREAMS: test.version = "5.10", wrapper_version = "v2.8" + - CoreMark: test.version = "v1.01", wrapper_version = "v2.0" + + See VERSION_CONFLATION_IMPACT.md for full analysis of affected processors. + + Override pattern: + def build_test_info(self) -> TestInfo: + base_info = super().build_test_info() + benchmark_version = self._extract_benchmark_version() + return TestInfo( + name=self.get_test_name(), + version=benchmark_version or base_info.version, + wrapper_version=base_info.wrapper_version + ) + + Returns: + TestInfo with name, version (wrapper by default), and wrapper_version + """ test_name = self.get_test_name() - # Try to get version from test_info file + # Extract wrapper version from orchestrator's test_info file + # This contains the test wrapper repository version (e.g., "v2.8" from "v2.8.tar.gz") test_info_file = self.result_dir / "test_info" version = None @@ -211,6 +243,7 @@ def build_test_info(self) -> TestInfo: # Find test in test_info for key, test_data in test_info_data.items(): if test_data.get('test_name') == test_name: + # Extract wrapper version from repo_file (e.g., "v2.8.tar.gz" -> "v2.8") version = test_data.get('repo_file', '').replace('.tar.gz', '') break except (OSError, json.JSONDecodeError, KeyError, TypeError) as e: @@ -218,8 +251,10 @@ def build_test_info(self) -> TestInfo: return TestInfo( name=test_name, - version=version or "unknown", - wrapper_version=version or "unknown" + # NOTE: Both fields set to wrapper version by default + # Processors with independent benchmark versions MUST override to fix this + version=version or "unknown", # Should be benchmark version (override needed) + wrapper_version=version or "unknown" # Correct: wrapper repository version ) def build_system_under_test(self) -> SystemUnderTest: From abbb2956ea770dd4ee68a4299d44ab0126ce949f Mon Sep 17 00:00:00 2001 From: Agent VM Date: Mon, 15 Jun 2026 20:07:33 -0400 Subject: [PATCH 3/4] test: add edge case tests for build_test_info() Add tests for non-string repo_file values and non-dict test_info data. These tests verify graceful handling of malformed or unexpected data structures rather than raising uncaught AttributeError. Addresses review feedback on PR #48. --- tests/test_base_processor.py | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/test_base_processor.py b/tests/test_base_processor.py index 75a5c34..f8a1963 100644 --- a/tests/test_base_processor.py +++ b/tests/test_base_processor.py @@ -106,3 +106,51 @@ def test_build_test_info_handles_malformed_json(tmp_path): # Verify assert result.version == "unknown" assert result.wrapper_version == "unknown" + + +def test_build_test_info_handles_non_string_repo_file(tmp_path): + """ + Non-string repo_file values should be handled gracefully. + + If test_info contains valid JSON but repo_file is null, an integer, + or other non-string value, the processor should handle it gracefully + rather than raising AttributeError. + """ + # Setup: Create test_info with null repo_file + test_info_data = { + "minimal_test": { + "test_name": "minimal_test", + "repo_file": None + } + } + test_info_file = tmp_path / "test_info" + test_info_file.write_text(json.dumps(test_info_data)) + + # Execute + processor = MinimalProcessor(str(tmp_path)) + result = processor.build_test_info() + + # Verify - should fall back to "unknown" rather than crash + assert result.version == "unknown" + assert result.wrapper_version == "unknown" + + +def test_build_test_info_handles_non_dict_test_info(tmp_path): + """ + Non-dict test_info data should be handled gracefully. + + If test_info contains a list or other non-dict structure, + the processor should handle it gracefully rather than raising + AttributeError when calling .items(). + """ + # Setup: Create test_info with list instead of dict + test_info_file = tmp_path / "test_info" + test_info_file.write_text("[]") + + # Execute + processor = MinimalProcessor(str(tmp_path)) + result = processor.build_test_info() + + # Verify - should fall back to "unknown" rather than crash + assert result.version == "unknown" + assert result.wrapper_version == "unknown" From 3073ae484748e76f6265a4a0937a66929d829b76 Mon Sep 17 00:00:00 2001 From: Agent VM Date: Mon, 15 Jun 2026 20:07:47 -0400 Subject: [PATCH 4/4] fix: handle edge cases in build_test_info() version extraction Add AttributeError to exception handling to gracefully handle: - Non-dict test_info data (when calling .items()) - Non-string repo_file values (when calling .replace()) Expand archive extension support beyond just .tar.gz: - Now handles: .tar.gz, .tar.xz, .tar.bz2, .zip, .tgz - Add type guard to verify repo_file is a string before processing All edge cases now return "unknown" instead of raising exceptions. Addresses review feedback on PR #48. --- src/chronicler/processors/base_processor.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/chronicler/processors/base_processor.py b/src/chronicler/processors/base_processor.py index 1253386..233eb84 100644 --- a/src/chronicler/processors/base_processor.py +++ b/src/chronicler/processors/base_processor.py @@ -243,10 +243,20 @@ def build_test_info(self) -> TestInfo: # Find test in test_info for key, test_data in test_info_data.items(): if test_data.get('test_name') == test_name: - # Extract wrapper version from repo_file (e.g., "v2.8.tar.gz" -> "v2.8") - version = test_data.get('repo_file', '').replace('.tar.gz', '') + # Extract wrapper version from repo_file + # Handle various archive extensions (.tar.gz, .tar.xz, .zip, etc.) + repo_file = test_data.get('repo_file', '') + if isinstance(repo_file, str): + # Strip common archive extensions + for ext in ['.tar.gz', '.tar.xz', '.tar.bz2', '.zip', '.tgz']: + if repo_file.endswith(ext): + version = repo_file[:-len(ext)] + break + else: + # No known extension found, use as-is + version = repo_file if repo_file else None break - except (OSError, json.JSONDecodeError, KeyError, TypeError) as e: + except (OSError, json.JSONDecodeError, KeyError, TypeError, AttributeError) as e: logger.warning(f"Failed to parse test_info: {e}") return TestInfo(