diff --git a/api/src/opentrons/protocol_api/_parameter_context.py b/api/src/opentrons/protocol_api/_parameter_context.py index 7773653a9c5..8c9debd882c 100644 --- a/api/src/opentrons/protocol_api/_parameter_context.py +++ b/api/src/opentrons/protocol_api/_parameter_context.py @@ -52,6 +52,7 @@ def add_int( description: A description of the parameter as it will show up on the frontend. unit: An optional unit to be appended to the end of the integer as it shown on the frontend. """ + validation.validate_variable_name_unique(variable_name, set(self._parameters)) parameter = parameter_definition.create_int_parameter( display_name=display_name, variable_name=variable_name, @@ -88,6 +89,7 @@ def add_float( description: A description of the parameter as it will show up on the frontend. unit: An optional unit to be appended to the end of the float as it shown on the frontend. """ + validation.validate_variable_name_unique(variable_name, set(self._parameters)) parameter = parameter_definition.create_float_parameter( display_name=display_name, variable_name=variable_name, @@ -115,6 +117,7 @@ def add_bool( default: The default value the boolean parameter will be set to. This will be used in initial analysis. description: A description of the parameter as it will show up on the frontend. """ + validation.validate_variable_name_unique(variable_name, set(self._parameters)) parameter = parameter_definition.create_bool_parameter( display_name=display_name, variable_name=variable_name, @@ -145,6 +148,7 @@ def add_str( Mutually exclusive with minimum and maximum. description: A description of the parameter as it will show up on the frontend. """ + validation.validate_variable_name_unique(variable_name, set(self._parameters)) parameter = parameter_definition.create_str_parameter( display_name=display_name, variable_name=variable_name, diff --git a/api/src/opentrons/protocols/parameters/validation.py b/api/src/opentrons/protocols/parameters/validation.py index 166055df504..9410db294ed 100644 --- a/api/src/opentrons/protocols/parameters/validation.py +++ b/api/src/opentrons/protocols/parameters/validation.py @@ -1,5 +1,5 @@ import keyword -from typing import List, Optional, Union, Literal +from typing import List, Set, Optional, Union, Literal from .types import ( AllowedTypes, @@ -16,6 +16,17 @@ DESCRIPTION_MAX_LEN = 100 +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: + raise ParameterNameError( + f'"{variable_name}" is already defined as a variable name for another parameter.' + f" All variable names must be unique." + ) + + def ensure_display_name(display_name: str) -> str: """Validate display name is within the character limit.""" if len(display_name) > DISPLAY_NAME_MAX_LEN: diff --git a/api/tests/opentrons/protocol_api/test_parameter_context.py b/api/tests/opentrons/protocol_api/test_parameter_context.py index 4d839d72667..7dcc246f216 100644 --- a/api/tests/opentrons/protocol_api/test_parameter_context.py +++ b/api/tests/opentrons/protocol_api/test_parameter_context.py @@ -46,6 +46,9 @@ def subject(api_version: APIVersion) -> ParameterContext: def test_add_int(decoy: Decoy, subject: ParameterContext) -> None: """It should create and add an int parameter definition.""" + subject._parameters["other_param"] = decoy.mock( + cls=mock_parameter_definition.ParameterDefinition + ) param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition) decoy.when(param_def.variable_name).then_return("my cool variable") decoy.when( @@ -60,6 +63,7 @@ def test_add_int(decoy: Decoy, subject: ParameterContext) -> None: unit="foot candles", ) ).then_return(param_def) + subject.add_int( display_name="abc", variable_name="xyz", @@ -70,11 +74,16 @@ def test_add_int(decoy: Decoy, subject: ParameterContext) -> None: description="blah blah blah", unit="foot candles", ) + assert param_def is subject._parameters["my cool variable"] + decoy.verify(mock_validation.validate_variable_name_unique("xyz", {"other_param"})) def test_add_float(decoy: Decoy, subject: ParameterContext) -> None: """It should create and add a float parameter definition.""" + subject._parameters["other_param"] = decoy.mock( + cls=mock_parameter_definition.ParameterDefinition + ) param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition) decoy.when(param_def.variable_name).then_return("my cooler variable") decoy.when(mock_validation.ensure_float_value(12.3)).then_return(3.21) @@ -83,7 +92,6 @@ def test_add_float(decoy: Decoy, subject: ParameterContext) -> None: decoy.when( mock_validation.ensure_float_choices([{"display_name": "foo", "value": 4.2}]) ).then_return([{"display_name": "bar", "value": 2.4}]) - decoy.when( mock_parameter_definition.create_float_parameter( display_name="abc", @@ -96,6 +104,7 @@ def test_add_float(decoy: Decoy, subject: ParameterContext) -> None: unit="lux", ) ).then_return(param_def) + subject.add_float( display_name="abc", variable_name="xyz", @@ -106,11 +115,16 @@ def test_add_float(decoy: Decoy, subject: ParameterContext) -> None: description="blah blah blah", unit="lux", ) + assert param_def is subject._parameters["my cooler variable"] + decoy.verify(mock_validation.validate_variable_name_unique("xyz", {"other_param"})) def test_add_bool(decoy: Decoy, subject: ParameterContext) -> None: """It should create and add a boolean parameter definition.""" + subject._parameters["other_param"] = decoy.mock( + cls=mock_parameter_definition.ParameterDefinition + ) param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition) decoy.when(param_def.variable_name).then_return("my coolest variable") decoy.when( @@ -125,17 +139,23 @@ def test_add_bool(decoy: Decoy, subject: ParameterContext) -> None: description="lorem ipsum", ) ).then_return(param_def) + subject.add_bool( display_name="cba", variable_name="zxy", default=False, description="lorem ipsum", ) + assert param_def is subject._parameters["my coolest variable"] + decoy.verify(mock_validation.validate_variable_name_unique("zxy", {"other_param"})) def test_add_string(decoy: Decoy, subject: ParameterContext) -> None: """It should create and add a string parameter definition.""" + subject._parameters["other_param"] = decoy.mock( + cls=mock_parameter_definition.ParameterDefinition + ) param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition) decoy.when(param_def.variable_name).then_return("my slightly less cool variable") decoy.when( @@ -147,6 +167,7 @@ def test_add_string(decoy: Decoy, subject: ParameterContext) -> None: description="fee foo fum", ) ).then_return(param_def) + subject.add_str( display_name="jkl", variable_name="qwerty", @@ -154,7 +175,11 @@ def test_add_string(decoy: Decoy, subject: ParameterContext) -> None: choices=[{"display_name": "bar", "value": "aaa"}], description="fee foo fum", ) + assert param_def is subject._parameters["my slightly less cool variable"] + decoy.verify( + mock_validation.validate_variable_name_unique("qwerty", {"other_param"}) + ) def test_set_parameters(decoy: Decoy, subject: ParameterContext) -> None: @@ -200,5 +225,5 @@ def test_export_parameters_for_protocol( subject._parameters = {"foo": param_def_1, "bar": param_def_2} result = subject.export_parameters_for_protocol() - assert result.x == "a" # type: ignore [attr-defined] - assert result.y == 1.23 # type: ignore [attr-defined] + assert result.x == "a" # type: ignore[attr-defined] + assert result.y == 1.23 # type: ignore[attr-defined] diff --git a/api/tests/opentrons/protocols/parameters/test_validation.py b/api/tests/opentrons/protocols/parameters/test_validation.py index 1f092a51c46..4d3b2fc83b5 100644 --- a/api/tests/opentrons/protocols/parameters/test_validation.py +++ b/api/tests/opentrons/protocols/parameters/test_validation.py @@ -12,6 +12,13 @@ from opentrons.protocols.parameters import validation as subject +def test_validate_variable_name_unique() -> None: + """It should no-op if the name is unique and raise if it is not.""" + subject.validate_variable_name_unique("one of a kind", {"fee", "foo", "fum"}) + with pytest.raises(ParameterNameError): + subject.validate_variable_name_unique("copy", {"paste", "copy", "cut"}) + + def test_ensure_display_name() -> None: """It should ensure the display name is within the character limit.""" result = subject.ensure_display_name("abc")