From 2f588af2306a4b72821290f4480f6993916c87d0 Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Mon, 6 Apr 2026 00:11:01 -0400 Subject: [PATCH] fix(build_environment): re-raise exception in get_distributions() on JSON parse failure Previously, if json.loads() failed, the except block logged the error but did not re-raise, causing an UnboundLocalError on the next line where `mapping` was undefined. Now the original exception is re-raised. Closes: #1023 Co-Authored-By: Claude Signed-off-by: Lalatendu Mohanty --- src/fromager/build_environment.py | 1 + tests/test_build_environment.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/fromager/build_environment.py b/src/fromager/build_environment.py index 3a9ee296..a5480be5 100644 --- a/src/fromager/build_environment.py +++ b/src/fromager/build_environment.py @@ -241,6 +241,7 @@ def get_distributions(self) -> typing.Mapping[str, Version]: mapping = json.loads(result.strip()) except Exception: logger.exception("failed to de-serialize JSON: %s", result) + raise return {name: Version(version) for name, version in sorted(mapping.items())} diff --git a/tests/test_build_environment.py b/tests/test_build_environment.py index 7b263389..28c504af 100644 --- a/tests/test_build_environment.py +++ b/tests/test_build_environment.py @@ -1,6 +1,8 @@ +import json import textwrap from unittest.mock import Mock, patch +import pytest from packaging.requirements import Requirement from packaging.version import Version @@ -81,3 +83,24 @@ def test_missing_dependency_pattern_resolution_impossible() -> None: """) match = build_environment._uv_missing_dependency_pattern.search(msg) assert match is not None + + +def test_get_distributions_valid_json() -> None: + """get_distributions returns a mapping of package names to versions.""" + env = Mock(spec=build_environment.BuildEnvironment) + env.python = "/fake/python3" + env.run.return_value = json.dumps({"setuptools": "69.5.1", "pip": "24.0"}) + + result = build_environment.BuildEnvironment.get_distributions(env) + + assert result == {"pip": Version("24.0"), "setuptools": Version("69.5.1")} + + +def test_get_distributions_invalid_json_raises() -> None: + """get_distributions raises on invalid JSON instead of UnboundLocalError.""" + env = Mock(spec=build_environment.BuildEnvironment) + env.python = "/fake/python3" + env.run.return_value = "not valid json" + + with pytest.raises(json.JSONDecodeError): + build_environment.BuildEnvironment.get_distributions(env)