Skip to content

Commit 483bf2d

Browse files
grdumasgrdumas
authored
Merge pull request #50 from redhat-performance/fix/RPOPC-1318-fio-version-to-test-version
RPOPC-1318: Move FIO benchmark version to test.version
2 parents 1614097 + fcdab5c commit 483bf2d

2 files changed

Lines changed: 330 additions & 0 deletions

File tree

‎src/chronicler/processors/fio_processor.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,31 @@ class FioProcessor(BaseProcessor):
7474
Aggregates results across all jobs while preserving per-job details.
7575
"""
7676

77+
def __init__(self, *args, **kwargs):
78+
super().__init__(*args, **kwargs)
79+
self._fio_version = None # Stores benchmark version extracted from fio-results.json
80+
7781
def get_test_name(self) -> str:
7882
return "fio"
7983

84+
def build_test_info(self):
85+
"""Override to use benchmark version from fio-results.json instead of wrapper version.
86+
87+
Extracts FIO benchmark version from fio-results.json top-level field
88+
(e.g., "fio version": "fio-3.36") and uses it for test.version,
89+
while preserving wrapper_version from base implementation.
90+
"""
91+
from ..schema import TestInfo
92+
93+
base_info = super().build_test_info()
94+
95+
# Use benchmark version if extracted, otherwise fall back to wrapper version
96+
return TestInfo(
97+
name=self.get_test_name(),
98+
version=self._fio_version or base_info.version,
99+
wrapper_version=base_info.wrapper_version
100+
)
101+
80102
def _extract_primary_metrics(
81103
self, runs: Dict[str, Any],
82104
overall_stats: Optional[StatisticalSummary]
@@ -169,6 +191,9 @@ def parse_runs(self, extracted_result: Dict[str, Any]) -> Dict[str, Any]:
169191
...
170192
}
171193
"""
194+
# Reset state to prevent leakage between parse calls
195+
self._fio_version = None
196+
172197
files = extracted_result.get("files") or {}
173198
if files.get("fio_results_json"):
174199
# Direct file path (demo / tmp/coremark-style layout)
@@ -360,6 +385,11 @@ def _build_run_object(
360385
) -> Run:
361386
"""Build a Run object for one FIO workload test."""
362387

388+
# Extract FIO benchmark version from JSON (e.g., "fio-3.36")
389+
# Store in instance variable for use in build_test_info()
390+
if not self._fio_version and "fio version" in fio_data:
391+
self._fio_version = fio_data["fio version"]
392+
363393
# Parse workload info from directory name
364394
workload_info = self._parse_workload_dir_name(workload_dir.name)
365395

‎tests/test_fio_version_extraction.py‎

Lines changed: 300 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,300 @@
1+
"""
2+
FIO processor: benchmark version extraction.
3+
4+
Tests that FIO extracts benchmark version (e.g., "fio-3.36") from fio-results.json
5+
and stores it in test.version instead of wrapper version.
6+
7+
Validates RPOPC-1318.
8+
"""
9+
10+
import json
11+
import pytest
12+
from pathlib import Path
13+
from unittest.mock import patch
14+
15+
from chronicler.processors.fio_processor import FioProcessor
16+
17+
pytestmark = pytest.mark.unit
18+
19+
20+
@pytest.fixture
21+
def result_dir_with_fio_version(tmp_path):
22+
"""Create FIO result directory with version in fio-results.json and wrapper version in test_info."""
23+
# Create fio-results.json with FIO version
24+
fio_data = {
25+
"fio version": "fio-3.36",
26+
"timestamp": 1776775499,
27+
"jobs": [
28+
{
29+
"jobname": "test-job",
30+
"groupid": 0,
31+
"elapsed": 120,
32+
"read": {
33+
"io_bytes": 1000000,
34+
"bw": 8333,
35+
"iops": 2083,
36+
"runtime": 120000,
37+
"total_ios": 250000,
38+
"lat_ns": {"mean": 480000, "min": 100000, "max": 1000000},
39+
"clat_ns": {"mean": 470000, "min": 90000, "max": 990000},
40+
"slat_ns": {"mean": 10000, "min": 1000, "max": 50000}
41+
}
42+
}
43+
]
44+
}
45+
46+
fio_json_path = tmp_path / "fio-results.json"
47+
with open(fio_json_path, "w") as f:
48+
json.dump(fio_data, f)
49+
50+
# Create test_info with wrapper version
51+
test_info = {
52+
"fio": {
53+
"test_name": "fio",
54+
"repo_file": "v2.1.tar.gz"
55+
}
56+
}
57+
58+
test_info_path = tmp_path / "test_info"
59+
with open(test_info_path, "w") as f:
60+
json.dump(test_info, f)
61+
62+
return tmp_path
63+
64+
65+
@pytest.fixture
66+
def result_dir_without_fio_version(tmp_path):
67+
"""Create FIO result directory without version in fio-results.json."""
68+
# Create fio-results.json WITHOUT FIO version
69+
fio_data = {
70+
"timestamp": 1776775499,
71+
"jobs": [
72+
{
73+
"jobname": "test-job",
74+
"groupid": 0,
75+
"elapsed": 120,
76+
"read": {
77+
"io_bytes": 1000000,
78+
"bw": 8333,
79+
"iops": 2083,
80+
"runtime": 120000,
81+
"total_ios": 250000,
82+
"lat_ns": {"mean": 480000, "min": 100000, "max": 1000000},
83+
"clat_ns": {"mean": 470000, "min": 90000, "max": 990000},
84+
"slat_ns": {"mean": 10000, "min": 1000, "max": 50000}
85+
}
86+
}
87+
]
88+
}
89+
90+
fio_json_path = tmp_path / "fio-results.json"
91+
with open(fio_json_path, "w") as f:
92+
json.dump(fio_data, f)
93+
94+
# Create test_info with wrapper version
95+
test_info = {
96+
"fio": {
97+
"test_name": "fio",
98+
"repo_file": "v2.1.tar.gz"
99+
}
100+
}
101+
102+
test_info_path = tmp_path / "test_info"
103+
with open(test_info_path, "w") as f:
104+
json.dump(test_info, f)
105+
106+
return tmp_path
107+
108+
109+
def test_fio_version_in_test_version_field(result_dir_with_fio_version):
110+
"""Test that FIO benchmark version is stored in test.version."""
111+
processor = FioProcessor(str(result_dir_with_fio_version))
112+
extracted_result = {"files": {"fio_results_json": str(result_dir_with_fio_version / "fio-results.json")},
113+
"extracted_path": str(result_dir_with_fio_version)}
114+
115+
# Mock archive extraction and call build_test_info
116+
with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
117+
mock_extract.return_value = extracted_result
118+
119+
# Parse runs to extract FIO version (this should populate _fio_version)
120+
processor.parse_runs(extracted_result)
121+
122+
# Now build_test_info should use the extracted FIO version
123+
test_info = processor.build_test_info()
124+
125+
# Verify test.version contains benchmark version, not wrapper version
126+
assert test_info.version == "fio-3.36", \
127+
f"Expected test.version='fio-3.36', got '{test_info.version}'"
128+
129+
# Verify wrapper_version contains wrapper version
130+
assert test_info.wrapper_version == "v2.1", \
131+
f"Expected wrapper_version='v2.1', got '{test_info.wrapper_version}'"
132+
133+
134+
def test_fio_version_fallback_to_wrapper_version(result_dir_without_fio_version):
135+
"""Test that wrapper version is used when FIO version is missing."""
136+
processor = FioProcessor(str(result_dir_without_fio_version))
137+
extracted_result = {"files": {"fio_results_json": str(result_dir_without_fio_version / "fio-results.json")},
138+
"extracted_path": str(result_dir_without_fio_version)}
139+
140+
# Mock archive extraction and call build_test_info
141+
with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
142+
mock_extract.return_value = extracted_result
143+
144+
# Parse runs (no FIO version to extract)
145+
processor.parse_runs(extracted_result)
146+
147+
# build_test_info should fall back to wrapper version
148+
test_info = processor.build_test_info()
149+
150+
# When FIO version is missing, should fall back to wrapper version
151+
assert test_info.version == "v2.1", \
152+
f"Expected test.version='v2.1' (fallback), got '{test_info.version}'"
153+
154+
assert test_info.wrapper_version == "v2.1", \
155+
f"Expected wrapper_version='v2.1', got '{test_info.wrapper_version}'"
156+
157+
158+
def test_fio_version_no_wrapper_version(tmp_path):
159+
"""Test FIO version extraction when no wrapper version exists."""
160+
# Create fio-results.json with FIO version but NO test_info file
161+
fio_data = {
162+
"fio version": "fio-3.36",
163+
"timestamp": 1776775499,
164+
"jobs": [
165+
{
166+
"jobname": "test-job",
167+
"groupid": 0,
168+
"elapsed": 120,
169+
"read": {
170+
"io_bytes": 1000000,
171+
"bw": 8333,
172+
"iops": 2083,
173+
"runtime": 120000,
174+
"total_ios": 250000,
175+
"lat_ns": {"mean": 480000, "min": 100000, "max": 1000000},
176+
"clat_ns": {"mean": 470000, "min": 90000, "max": 990000},
177+
"slat_ns": {"mean": 10000, "min": 1000, "max": 50000}
178+
}
179+
}
180+
]
181+
}
182+
183+
fio_json_path = tmp_path / "fio-results.json"
184+
with open(fio_json_path, "w") as f:
185+
json.dump(fio_data, f)
186+
187+
processor = FioProcessor(str(tmp_path))
188+
extracted_result = {"files": {"fio_results_json": str(fio_json_path)},
189+
"extracted_path": str(tmp_path)}
190+
191+
# Mock archive extraction
192+
with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
193+
mock_extract.return_value = extracted_result
194+
195+
# Parse runs to extract FIO version
196+
processor.parse_runs(extracted_result)
197+
198+
# build_test_info should use extracted FIO version
199+
test_info = processor.build_test_info()
200+
201+
# Should still extract FIO version
202+
assert test_info.version == "fio-3.36", \
203+
f"Expected test.version='fio-3.36', got '{test_info.version}'"
204+
205+
# Wrapper version should be "unknown" when test_info is missing
206+
assert test_info.wrapper_version == "unknown", \
207+
f"Expected wrapper_version='unknown', got '{test_info.wrapper_version}'"
208+
209+
210+
def test_fio_version_resets_between_parses(tmp_path):
211+
"""Processor reuse: version state should not leak between parse_runs() calls."""
212+
# First parse: fio-results.json WITH benchmark version
213+
fio_data_with_version = {
214+
"fio version": "fio-3.36",
215+
"timestamp": 1776775499,
216+
"jobs": [
217+
{
218+
"jobname": "test-job-1",
219+
"groupid": 0,
220+
"elapsed": 120,
221+
"read": {
222+
"io_bytes": 1000000,
223+
"bw": 8333,
224+
"iops": 2083,
225+
"runtime": 120000,
226+
"total_ios": 250000,
227+
"lat_ns": {"mean": 480000, "min": 100000, "max": 1000000},
228+
"clat_ns": {"mean": 470000, "min": 90000, "max": 990000},
229+
"slat_ns": {"mean": 10000, "min": 1000, "max": 50000}
230+
}
231+
}
232+
]
233+
}
234+
235+
fio_json_path1 = tmp_path / "fio-results-1.json"
236+
with open(fio_json_path1, "w") as f:
237+
json.dump(fio_data_with_version, f)
238+
239+
# Create test_info file with wrapper version for fallback
240+
test_info = {
241+
"fio": {
242+
"test_name": "fio",
243+
"repo_file": "v2.1.tar.gz"
244+
}
245+
}
246+
test_info_path = tmp_path / "test_info"
247+
with open(test_info_path, "w") as f:
248+
json.dump(test_info, f)
249+
250+
processor = FioProcessor(str(tmp_path))
251+
extracted_result1 = {"files": {"fio_results_json": str(fio_json_path1)},
252+
"extracted_path": str(tmp_path)}
253+
254+
# First parse
255+
with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
256+
mock_extract.return_value = extracted_result1
257+
processor.parse_runs(extracted_result1)
258+
test_info1 = processor.build_test_info()
259+
260+
assert test_info1.version == "fio-3.36", "First parse should extract benchmark version"
261+
262+
# Second parse: fio-results.json WITHOUT benchmark version (reusing same processor instance)
263+
fio_data_without_version = {
264+
"timestamp": 1776775500,
265+
"jobs": [
266+
{
267+
"jobname": "test-job-2",
268+
"groupid": 0,
269+
"elapsed": 120,
270+
"read": {
271+
"io_bytes": 2000000,
272+
"bw": 16666,
273+
"iops": 4166,
274+
"runtime": 120000,
275+
"total_ios": 500000,
276+
"lat_ns": {"mean": 240000, "min": 50000, "max": 500000},
277+
"clat_ns": {"mean": 235000, "min": 45000, "max": 495000},
278+
"slat_ns": {"mean": 5000, "min": 500, "max": 25000}
279+
}
280+
}
281+
]
282+
}
283+
284+
fio_json_path2 = tmp_path / "fio-results-2.json"
285+
with open(fio_json_path2, "w") as f:
286+
json.dump(fio_data_without_version, f)
287+
288+
extracted_result2 = {"files": {"fio_results_json": str(fio_json_path2)},
289+
"extracted_path": str(tmp_path)}
290+
291+
# Second parse with same processor instance
292+
with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
293+
mock_extract.return_value = extracted_result2
294+
processor.parse_runs(extracted_result2)
295+
test_info2 = processor.build_test_info()
296+
297+
# Should fall back to wrapper version, NOT retain stale "fio-3.36"
298+
assert test_info2.version == "v2.1", \
299+
f"Second parse should fall back to wrapper version, not retain stale 'fio-3.36'. Got '{test_info2.version}'"
300+
assert test_info2.wrapper_version == "v2.1"

0 commit comments

Comments
 (0)