Skip to content

Commit 1200ac6

Browse files
committed
fix: address review feedback for Debian naming
- Fix no-op tests for legacy packages field and malformed JSON - Remove unreachable KeyError from except clause in catalog.py Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 0ee6c44 commit 1200ac6

2 files changed

Lines changed: 17 additions & 46 deletions

File tree

cli_audit/catalog.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ def resolve_apt_package_name(tool_name: str) -> str:
331331
packages = data.get("packages", {})
332332
if "apt" in packages:
333333
return packages["apt"]
334-
except (json.JSONDecodeError, KeyError):
334+
except json.JSONDecodeError:
335335
pass
336336
return tool_name
337337

tests/test_upgrade.py

Lines changed: 16 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -816,34 +816,18 @@ def test_resolve_apt_package_name_ripgrep(self):
816816
assert resolve_apt_package_name("ripgrep") == "ripgrep"
817817

818818
def test_resolve_apt_package_name_with_legacy_packages_field(self):
819-
"""Tools with legacy 'packages' field should still resolve."""
820-
from cli_audit.catalog import resolve_apt_package_name
821-
import tempfile
819+
"""Tools using legacy 'packages' field should resolve correctly."""
822820
import json
823-
import os
824-
825-
# Create a temporary catalog file with legacy packages field
826-
with tempfile.TemporaryDirectory() as tmpdir:
827-
catalog_file = os.path.join(tmpdir, "legacy_tool.json")
828-
data = {
829-
"name": "legacy_tool",
830-
"install_method": "package_manager",
831-
"packages": {"apt": "legacy-tool-pkg"},
832-
}
833-
with open(catalog_file, "w") as f:
834-
json.dump(data, f)
835-
836-
# Patch the catalog path resolution
837-
with patch(
838-
"cli_audit.catalog.Path.__truediv__",
839-
) as mock_div:
840-
# We need a different approach - patch at function level
841-
pass
842-
843-
# Test via direct file - use the php.json which has packages.apt
844-
result = resolve_apt_package_name("php")
845-
# php.json has packages.apt field
846-
assert result != "php" or result == "php" # Just verify it doesn't crash
821+
from cli_audit.catalog import resolve_apt_package_name
822+
823+
fake_json = json.dumps({
824+
"name": "legacy_tool",
825+
"packages": {"apt": "legacy-apt-pkg"},
826+
})
827+
with patch("builtins.open", mock_open(read_data=fake_json)):
828+
with patch("pathlib.Path.exists", return_value=True):
829+
result = resolve_apt_package_name("legacy_tool")
830+
assert result == "legacy-apt-pkg"
847831

848832
def test_resolve_apt_package_name_tool_without_apt_method(self):
849833
"""Tools with available_methods but no apt method should fall back."""
@@ -856,24 +840,11 @@ def test_resolve_apt_package_name_tool_without_apt_method(self):
856840
def test_resolve_apt_package_name_with_malformed_json(self):
857841
"""Malformed catalog JSON should fall back to tool name."""
858842
from cli_audit.catalog import resolve_apt_package_name
859-
import tempfile
860-
import os
861-
862-
with tempfile.TemporaryDirectory() as tmpdir:
863-
catalog_file = os.path.join(tmpdir, "broken.json")
864-
with open(catalog_file, "w") as f:
865-
f.write("not valid json{{{")
866-
867-
with patch("cli_audit.catalog.Path") as MockPath:
868-
mock_path = MagicMock()
869-
mock_path.exists.return_value = True
870-
mock_path.__truediv__ = MagicMock(return_value=mock_path)
871-
# Make open() read the malformed file
872-
MockPath.return_value.__truediv__.return_value = mock_path
873-
874-
# The function should handle this gracefully
875-
# Since we can't easily mock the path, test with nonexistent tool
876-
assert resolve_apt_package_name("nonexistent_xyz_abc") == "nonexistent_xyz_abc"
843+
844+
with patch("builtins.open", mock_open(read_data="not valid json{{{")):
845+
with patch("pathlib.Path.exists", return_value=True):
846+
result = resolve_apt_package_name("broken_tool")
847+
assert result == "broken_tool"
877848

878849

879850
class TestCheckUpgradeAvailableAptResolved:

0 commit comments

Comments
 (0)