From ca9269551f0c4d7ffa43a22435c74522128e3b09 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sun, 26 Jul 2026 09:58:15 -0500 Subject: [PATCH 1/2] fix(analysis): raise ValueError instead of NameError when FIO command missing ## Summary When `bobber parse-results` processes a FIO log that does not contain at least two `/usr/bin/fio --rw...` command lines, the tool is supposed to raise a clear `ValueError`. Instead it crashes with `NameError: name 'log' is not defined`, hiding the real problem from the user. ## Root cause In `fio_command_details()` (`bobber/lib/analysis/common.py`), the error message references a variable `log` that does not exist in the function scope (the function only receives `log_contents`, `old_reads`, and `old_writes`): ```python commands = re.findall(r'/usr/bin/fio --rw.*', log_contents) if len(commands) < 2: raise ValueError(f'FIO command not found in {log} file!') ``` Evaluating the f-string raises `NameError` before the `ValueError` can be constructed, so callers see an unhelpful `NameError` traceback. ## Fix Drop the undefined filename reference from the message (the function has no access to the log filename): ```python raise ValueError('FIO command not found in the log file!') ``` ## Testing - Reproduced with `uv run --with pyyaml python`: - Before: `fio_command_details('no fio commands here', {}, {})` raised `NameError: name 'log' is not defined`. - After: raises `ValueError: FIO command not found in the log file!` as intended. - Sanity-checked the happy path: parsing a log containing read/write fio commands still returns the expected parameter dicts. - `pycodestyle bobber/lib/analysis/common.py` passes (matches the lint step in `.github/workflows/python-package.yml`). - The full CI suite (wheel build + `bobber build` Docker image build) was not run locally as it requires Docker and GPU test infrastructure. ## Why existing tests missed it The repository has no unit tests for the parsing library; CI only lints and builds the package/Docker image, so this error path was never exercised. --- bobber/lib/analysis/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bobber/lib/analysis/common.py b/bobber/lib/analysis/common.py index b1af96c..476221a 100644 --- a/bobber/lib/analysis/common.py +++ b/bobber/lib/analysis/common.py @@ -244,7 +244,7 @@ def fio_command_details(log_contents: str, old_reads: dict, """ commands = re.findall(r'/usr/bin/fio --rw.*', log_contents) if len(commands) < 2: - raise ValueError(f'FIO command not found in {log} file!') + raise ValueError('FIO command not found in the log file!') for command in commands: if '--rw=read' in command: From fd1c687fbccebf1b8fb95aca7a6058de161971a3 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sun, 26 Jul 2026 12:10:56 -0500 Subject: [PATCH 2/2] test(analysis): regression test for FIO command NameError Flags the case where `fio_command_details()` receives log contents with fewer than two `/usr/bin/fio --rw...` command lines and crashed with `NameError: name 'log' is not defined` instead of the intended `ValueError`. Also sanity-checks that a valid read/write FIO log still parses. Red-green verification: - With fix (HEAD): `uv run --with pytest pytest tests/test_fio_command_nameerror.py -q` -> 2 passed. - Without fix (`git show HEAD~1:bobber/lib/analysis/common.py > bobber/lib/analysis/common.py`): same command -> 1 failed (NameError: name 'log' is not defined), 1 passed. - Restored with `git checkout -- .`. --- tests/test_fio_command_nameerror.py | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 tests/test_fio_command_nameerror.py diff --git a/tests/test_fio_command_nameerror.py b/tests/test_fio_command_nameerror.py new file mode 100644 index 0000000..90b9809 --- /dev/null +++ b/tests/test_fio_command_nameerror.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: MIT +"""Regression test for the FIO command NameError bug. + +When a FIO log does not contain at least two ``/usr/bin/fio --rw...`` +command lines, ``fio_command_details()`` must raise a clear ``ValueError``. +Before the fix, the error message referenced an undefined variable ``log``, +so callers instead got ``NameError: name 'log' is not defined``. +""" +import os +import sys + +import pytest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from bobber.lib.analysis.common import fio_command_details # noqa: E402 + + +def test_missing_fio_commands_raises_valueerror_not_nameerror(): + with pytest.raises(ValueError, match='FIO command not found'): + fio_command_details('no fio commands here', {}, {}) + + +def test_valid_fio_log_still_parses(): + log_contents = ( + '/usr/bin/fio --rw=read --bs=1M --size=1G --name=read_test\n' + 'some output\n' + '/usr/bin/fio --rw=write --bs=1M --size=1G --name=write_test\n' + ) + reads, writes = fio_command_details(log_contents, {}, {}) + assert reads['rw'] == 'read' + assert writes['rw'] == 'write'