Skip to content

Commit

Permalink
Improve code, add tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
felixfontein committed Nov 8, 2024
1 parent d0658b2 commit 445cf98
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 24 deletions.
26 changes: 3 additions & 23 deletions src/antsibull_core/collection_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,23 +137,6 @@ def _validate_removal_updates(
)
last_version = version

if update.reason:
if not (update.deprecated_version or update.redeprecated_version):
self.errors.append(
f"{prefix}[{index}] -> reason: Reason can only be provided"
f" if 'deprecated_version' or 'redeprecated_version' is used"
)
if update.reason != "other" and update.reason_text is not None:
self.errors.append(
f"{prefix}[{index}] -> reason_text: Reason text"
f" must not be provided if reason is not 'other'"
)
if update.reason == "other" and update.reason_text is None:
self.errors.append(
f"{prefix}[{index}] -> reason_text: Reason text"
f" must be provided if reason is 'other'"
)

def _validate_removal_base(
self, collection: str, removal: BaseRemovalInformation, prefix: str
) -> None:
Expand All @@ -167,12 +150,9 @@ def _validate_removal(
removal.major_version != "TBD"
and removal.major_version <= self.major_release # pyre-ignore[58]
):
is_ok = False
if removal.major_version == self.major_release:
for update in removal.updates:
if update.removed_version:
is_ok = True
break
is_ok = removal.major_version == self.major_release and any(
update.removed_version for update in removal.updates
)
if not is_ok:
self.errors.append(
f"{prefix} major_version: Removal major version {removal.major_version} must"
Expand Down
27 changes: 26 additions & 1 deletion src/antsibull_core/schemas/collection_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class RemovalUpdate(p.BaseModel):

model_config = p.ConfigDict(arbitrary_types_allowed=True)

# Exactly one of the following must be provided
cancelled_version: t.Optional[PydanticPypiVersion] = None
deprecated_version: t.Optional[PydanticPypiVersion] = None
redeprecated_version: t.Optional[PydanticPypiVersion] = None
Expand All @@ -73,7 +74,7 @@ class RemovalUpdate(p.BaseModel):
]
] = None

# If reason is not provided, or if reason is other, an optional extra text appended
# If reason is not provided, or if reason is 'other', an optional extra text appended
# to the message.
reason_text: t.Optional[str] = None

Expand All @@ -100,6 +101,30 @@ def _exactly_one_required(self) -> Self:
raise ValueError(f"Exactly one of {', '.join(fields)} must be specified")
return self

@p.model_validator(mode="after") # pyre-ignore[56]
def _check_reason(self) -> Self:
if self.reason and not (self.deprecated_version or self.redeprecated_version):
raise ValueError(
"Reason can only be provided if 'deprecated_version'"
" or 'redeprecated_version' is used"
)
return self

@p.model_validator(mode="after") # pyre-ignore[56]
def _check_reason_text(self) -> Self:
reasons_with_text = ("other", "guidelines-violation")
if self.reason in reasons_with_text:
if self.reason_text is None:
raise ValueError(
f"Reason text must be provided if reason is '{self.reason}'"
)
elif self.reason:
if self.reason_text is not None:
raise ValueError(
f"Reason text must not be provided if reason is '{self.reason}'"
)
return self


class BaseRemovalInformation(p.BaseModel):
"""
Expand Down
21 changes: 21 additions & 0 deletions tests/functional/test_collection_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@
updates:
- removed_version: 9.0.0a1
- readded_version: 9.1.0
correct.foo11:
repository: https://github.com/ansible-collections/collection_template
removal:
major_version: 9
reason: considered-unmaintained
discussion: https://forum.ansible.com/...
announce_version: 9.3.0
correct.foo2:
repository: https://github.com/ansible-collections/collection_template
removal:
Expand Down Expand Up @@ -182,6 +189,7 @@
"foo.bar",
"correct.foo1",
"correct.foo10",
"correct.foo11",
"correct.foo2",
"correct.foo3",
"correct.foo4",
Expand Down Expand Up @@ -210,6 +218,7 @@
"collections -> baz.bam -> repository: Required field not provided",
"collections -> baz.bam: Collection not in ansible.in",
"collections -> correct.foo10 -> removal -> -> updates[0] -> removed_version: Unexpected update after deprecated_version",
"collections -> correct.foo11 -> removal -> major_version: Removal major version 9 must be larger than current major version 9",
"collections -> correct.foo3 -> removal -> -> updates[0] -> readded_version: Unexpected first update",
"collections -> correct.foo3 -> removal -> -> updates[0] -> readded_version: Version's major version 10 must be the current major version 9",
"collections -> foo.bar -> repository: Required field not provided",
Expand Down Expand Up @@ -343,6 +352,14 @@
redeprecated_version: 1.2.3
removed_version: 1.2.3
readded_version: 1.2.3
- reason_text: foo
- readded_version: 1.2.3
reason: deprecated
- deprecated_version: 1.2.3
reason: deprecated
reason_text: bla
- deprecated_version: 1.2.3
reason: other
removed_collections:
bad.foo1:
repository: https://github.com/ansible-collections/collection_template
Expand Down Expand Up @@ -383,6 +400,10 @@
"collections -> bad.foo15 -> removal -> updates -> 2 -> extra: Extra inputs are not permitted",
"collections -> bad.foo15 -> removal -> updates -> 3 -> cancelled_version: Value error, must be a string or PypiVer object, got []",
"collections -> bad.foo15 -> removal -> updates -> 4: Value error, Exactly one of cancelled_version, deprecated_version, redeprecated_version, removed_version, readded_version must be specified",
"collections -> bad.foo15 -> removal -> updates -> 5: Value error, Exactly one of cancelled_version, deprecated_version, redeprecated_version, removed_version, readded_version must be specified",
"collections -> bad.foo15 -> removal -> updates -> 6: Value error, Reason can only be provided if 'deprecated_version' or 'redeprecated_version' is used",
"collections -> bad.foo15 -> removal -> updates -> 7: Value error, Reason text must not be provided if reason is 'deprecated'",
"collections -> bad.foo15 -> removal -> updates -> 8: Value error, Reason text must be provided if reason is 'other'",
"collections -> bad.foo2 -> removal -> announce_version: Value error, must be a version with three release numbers (e.g. 1.2.3, 2.3.4a1), got '9.3'",
"collections -> bad.foo3 -> removal -> announce_version: Value error, must be a non-trivial string, got ''",
"collections -> bad.foo4 -> removal: Value error, major_version must not be TBD if reason is not 'renamed'",
Expand Down

0 comments on commit 445cf98

Please sign in to comment.