Skip to content

Commit cefb058

Browse files
committed
fix: address sixth round of PR review feedback
- Validate integration.id/name/version/description are strings - Catch TypeError in pkg_version.Version() for non-string versions - Swap validation order: check catalogs type before emptiness - Isolate TestActiveCatalogs from user ~/.specify/ via monkeypatch
1 parent 40df719 commit cefb058

2 files changed

Lines changed: 13 additions & 7 deletions

File tree

src/specify_cli/integrations/catalog.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,16 +111,16 @@ def _load_catalog_config(
111111
f"Invalid catalog config {config_path}: expected a YAML mapping at the root"
112112
)
113113
catalogs_data = data.get("catalogs", [])
114-
if not catalogs_data:
115-
raise IntegrationCatalogError(
116-
f"Catalog config {config_path} exists but contains no 'catalogs' entries. "
117-
f"Remove the file to use built-in defaults, or add valid catalog entries."
118-
)
119114
if not isinstance(catalogs_data, list):
120115
raise IntegrationCatalogError(
121116
f"Invalid catalog config: 'catalogs' must be a list, "
122117
f"got {type(catalogs_data).__name__}"
123118
)
119+
if not catalogs_data:
120+
raise IntegrationCatalogError(
121+
f"Catalog config {config_path} exists but contains no 'catalogs' entries. "
122+
f"Remove the file to use built-in defaults, or add valid catalog entries."
123+
)
124124
entries: List[IntegrationCatalogEntry] = []
125125
skipped: List[int] = []
126126
for idx, item in enumerate(catalogs_data):
@@ -475,6 +475,10 @@ def _validate(self) -> None:
475475
raise IntegrationDescriptorError(
476476
f"Missing integration.{field}"
477477
)
478+
if not isinstance(integ[field], str):
479+
raise IntegrationDescriptorError(
480+
f"integration.{field} must be a string, got {type(integ[field]).__name__}"
481+
)
478482

479483
if not re.match(r"^[a-z0-9-]+$", integ["id"]):
480484
raise IntegrationDescriptorError(
@@ -484,7 +488,7 @@ def _validate(self) -> None:
484488

485489
try:
486490
pkg_version.Version(integ["version"])
487-
except pkg_version.InvalidVersion:
491+
except (pkg_version.InvalidVersion, TypeError):
488492
raise IntegrationDescriptorError(
489493
f"Invalid version '{integ['version']}'"
490494
)

tests/integrations/test_integration_catalog.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ def test_missing_host_rejected(self):
7373

7474

7575
class TestActiveCatalogs:
76-
def test_defaults_when_no_config(self, tmp_path):
76+
def test_defaults_when_no_config(self, tmp_path, monkeypatch):
77+
monkeypatch.setenv("HOME", str(tmp_path))
78+
monkeypatch.setenv("USERPROFILE", str(tmp_path))
7779
(tmp_path / ".specify").mkdir()
7880
cat = IntegrationCatalog(tmp_path)
7981
active = cat.get_active_catalogs()

0 commit comments

Comments
 (0)