Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/specify_cli/presets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,37 @@ def _validate(self):

# Validate provides section
provides = self.data["provides"]
if "templates" not in provides or not provides["templates"]:
if "templates" not in provides:
raise PresetValidationError(
"Preset must provide at least one template"
)

# Validate templates
for tmpl in provides["templates"]:
# Validate templates. Guard the container and each entry's shape so a
# malformed third-party preset.yml (e.g. ``templates: 5`` or
# ``templates: [null]``) raises a clean PresetValidationError the
# install handler already catches, instead of a raw TypeError
# ('int'/'NoneType' object is not iterable) that escapes to an
# unhandled traceback. Mirrors the sibling ExtensionManifest guards.
#
# Order matters: the container's TYPE is checked before its emptiness,
# so a FALSY non-list (``templates: 0``/``false``/``null``/``''``/``{}``)
# reports the accurate type error rather than the misleading "must
# provide at least one template". An empty list still reports the
# latter, since that genuinely is a list with no templates.
templates = provides["templates"]
if not isinstance(templates, list):
Comment thread
jawwad-ali marked this conversation as resolved.
raise PresetValidationError(
"Invalid provides.templates: expected a list"
)
if not templates:
raise PresetValidationError(
"Preset must provide at least one template"
)
for tmpl in templates:
if not isinstance(tmpl, dict):
raise PresetValidationError(
"Each template entry in 'provides.templates' must be a mapping"
)
if "type" not in tmpl or "name" not in tmpl or "file" not in tmpl:
raise PresetValidationError(
"Template missing 'type', 'name', or 'file'"
Expand Down
39 changes: 39 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,45 @@ def test_non_mapping_yaml_raises_validation_error(self, temp_dir):
with pytest.raises(PresetValidationError, match="YAML mapping"):
PresetManifest(manifest_path)

@pytest.mark.parametrize(
"bad",
[
5, "oops", {"a": 1}, # truthy non-lists
0, False, None, "", {}, # FALSY non-lists: must not fall through to
# the misleading "at least one template"
],
)
def test_non_list_templates_raises_validation_error(
self, temp_dir, valid_pack_data, bad
):
"""A non-list provides.templates raises the accurate type error, not a raw
'int object is not iterable' TypeError and not the misleading "must provide
at least one template" (which a falsy non-list hit while the type check
sat behind the emptiness check) — mirrors ExtensionManifest."""
valid_pack_data["provides"]["templates"] = bad
manifest_path = temp_dir / "preset.yml"
manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8")
with pytest.raises(PresetValidationError, match="templates.*expected a list"):
PresetManifest(manifest_path)

# NOTE: the empty-list case (a well-typed container with no templates, which
# must keep the "must provide at least one template" message after the
# type-before-emptiness reordering) is already covered by
# test_no_templates_provided below.

@pytest.mark.parametrize("bad_entry", [None, 5, "oops", ["nested"]])
def test_non_mapping_template_entry_raises_validation_error(
self, temp_dir, valid_pack_data, bad_entry
):
"""A non-mapping template entry (null/scalar/list) raises PresetValidationError,
not a raw 'argument of type ... is not iterable' TypeError from the
`"type" not in tmpl` membership test — mirrors ExtensionManifest."""
valid_pack_data["provides"]["templates"] = [bad_entry]
manifest_path = temp_dir / "preset.yml"
manifest_path.write_text(yaml.dump(valid_pack_data), encoding="utf-8")
with pytest.raises(PresetValidationError, match="must be a mapping"):
PresetManifest(manifest_path)

def test_missing_schema_version(self, temp_dir, valid_pack_data):
"""Test missing schema_version field."""
del valid_pack_data["schema_version"]
Expand Down