Skip to content

Commit

Permalink
fix(api): better error message for non-string variable names and min/…
Browse files Browse the repository at this point in the history
…max validation adjustment (#14938)
  • Loading branch information
jbleon95 authored and Carlos-fernandez committed May 20, 2024
1 parent 3bb5324 commit 41ea773
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
4 changes: 2 additions & 2 deletions api/src/opentrons/protocols/parameters/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def validate_variable_name_unique(
variable_name: str, other_variable_names: Set[str]
) -> None:
"""Validate that the given variable name is unique."""
if variable_name in other_variable_names:
if isinstance(variable_name, str) and variable_name in other_variable_names:
raise ParameterNameError(
f'"{variable_name}" is already defined as a variable name for another parameter.'
f" All variable names must be unique."
Expand Down Expand Up @@ -222,7 +222,7 @@ def _validate_min_and_max(
# These asserts are for the type checker and should never actually be asserted false
assert isinstance(minimum, (int, float))
assert isinstance(maximum, (int, float))
if maximum <= minimum:
if maximum < minimum:
raise ParameterDefinitionError(
"Maximum must be greater than the minimum"
)
Expand Down
5 changes: 4 additions & 1 deletion api/tests/opentrons/protocols/parameters/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@


def test_validate_variable_name_unique() -> None:
"""It should no-op if the name is unique and raise if it is not."""
"""It should no-op if the name is unique or if it's not a string, and raise if it is not."""
subject.validate_variable_name_unique("one of a kind", {"fee", "foo", "fum"})
subject.validate_variable_name_unique({}, {"fee", "foo", "fum"}) # type: ignore[arg-type]
with pytest.raises(ParameterNameError):
subject.validate_variable_name_unique("copy", {"paste", "copy", "cut"})

Expand Down Expand Up @@ -103,10 +104,12 @@ def test_ensure_variable_name_raises_keyword(variable_name: str) -> None:
def test_validate_options() -> None:
"""It should not raise when given valid constraints"""
subject.validate_options(123, 1, 100, None, int)
subject.validate_options(123, 100, 100, None, int)
subject.validate_options(
123, None, None, [{"display_name": "abc", "value": 456}], int
)
subject.validate_options(12.3, 1.1, 100.9, None, float)
subject.validate_options(12.3, 1.1, 1.1, None, float)
subject.validate_options(
12.3, None, None, [{"display_name": "abc", "value": 45.6}], float
)
Expand Down

0 comments on commit 41ea773

Please sign in to comment.