From 201e8202e4270520c46784bb0c2edc682367c43c Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Fri, 19 Jul 2024 11:23:51 -0500 Subject: [PATCH 01/10] Added service config check and test for special characters. --- .../measurement/service.py | 5 +- .../example.Control.V1.serviceconfig | 19 ++++++ .../example.Control.V2.serviceconfig | 19 ++++++ .../check_char/example.Control.serviceconfig | 19 ++++++ .../example.ErrorAnnotations.serviceconfig | 19 ++++++ .../example.ErrorCollections.serviceconfig | 19 ++++++ .../example.ErrorDisplayName.serviceconfig | 19 ++++++ .../example.ErrorTags.serviceconfig | 19 ++++++ packages/service/tests/unit/test_service.py | 61 ++++++++++++++++++- 9 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 packages/service/tests/assets/check_char/example.Control.V1.serviceconfig create mode 100644 packages/service/tests/assets/check_char/example.Control.V2.serviceconfig create mode 100644 packages/service/tests/assets/check_char/example.Control.serviceconfig create mode 100644 packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig create mode 100644 packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig create mode 100644 packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig create mode 100644 packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig diff --git a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py index ab8718563..625faf454 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py +++ b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py @@ -210,7 +210,10 @@ def __init__( raise RuntimeError(f"File does not exist. {service_config_path}") with open(service_config_path) as service_config_file: - service_config = json.load(service_config_file) + try: + service_config = json.loads(service_config_file.read()) + except json.decoder.JSONDecodeError: + raise NameError(f"Invalid character(s) in {service_config_path}") if service_class is None: service = next(iter(service_config["services"]), None) diff --git a/packages/service/tests/assets/check_char/example.Control.V1.serviceconfig b/packages/service/tests/assets/check_char/example.Control.V1.serviceconfig new file mode 100644 index 000000000..305fd8840 --- /dev/null +++ b/packages/service/tests/assets/check_char/example.Control.V1.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "SampleMeasurement", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", + "ni/service.collection": "CurrentTests.Inrush", + "ni/service.tags": [ "powerup", "current" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.Control.V2.serviceconfig b/packages/service/tests/assets/check_char/example.Control.V2.serviceconfig new file mode 100644 index 000000000..305fd8840 --- /dev/null +++ b/packages/service/tests/assets/check_char/example.Control.V2.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "SampleMeasurement", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", + "ni/service.collection": "CurrentTests.Inrush", + "ni/service.tags": [ "powerup", "current" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.Control.serviceconfig b/packages/service/tests/assets/check_char/example.Control.serviceconfig new file mode 100644 index 000000000..305fd8840 --- /dev/null +++ b/packages/service/tests/assets/check_char/example.Control.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "SampleMeasurement", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", + "ni/service.collection": "CurrentTests.Inrush", + "ni/service.tags": [ "powerup", "current" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig new file mode 100644 index 000000000..5a070d54f --- /dev/null +++ b/packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "SampleMeasurement", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": """, + "ni/service.collection": "CurrentTests.Inrush", + "ni/service.tags": [ "powerup", "current" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig new file mode 100644 index 000000000..1dc3c2f05 --- /dev/null +++ b/packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "SampleMeasurement\", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", + "ni/service.collection": "\", + "ni/service.tags": [ "powerup", "current" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig new file mode 100644 index 000000000..7df8043f4 --- /dev/null +++ b/packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "\\SampleMeasurement\", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", + "ni/service.collection": "CurrentTests.Inrush", + "ni/service.tags": [ "powerup", "current" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig new file mode 100644 index 000000000..053b7430a --- /dev/null +++ b/packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig @@ -0,0 +1,19 @@ +{ + "services": [ + { + "displayName": "SampleMeasurement\", + "serviceClass": "SampleMeasurement_Python", + "descriptionUrl": "https://www.example.com/SampleMeasurement.html", + "providedInterfaces": [ + "ni.measurementlink.measurement.v1.MeasurementService", + "ni.measurementlink.measurement.v2.MeasurementService" + ], + "path": "start.bat", + "annotations": { + "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", + "ni/service.collection": "CurrentTests.Inrush", + "ni/service.tags": [ "powerup", "\"" ] + } + } + ] +} \ No newline at end of file diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index b62235f2f..edea0121c 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -10,8 +10,13 @@ from ni_measurement_plugin_sdk_service import _datatypeinfo from ni_measurement_plugin_sdk_service._annotations import TYPE_SPECIALIZATION_KEY -from ni_measurement_plugin_sdk_service._internal.stubs.ni.protobuf.types import xydata_pb2 -from ni_measurement_plugin_sdk_service.measurement.info import DataType, TypeSpecialization +from ni_measurement_plugin_sdk_service._internal.stubs.ni.protobuf.types import ( + xydata_pb2, +) +from ni_measurement_plugin_sdk_service.measurement.info import ( + DataType, + TypeSpecialization, +) from ni_measurement_plugin_sdk_service.measurement.service import MeasurementService @@ -421,6 +426,58 @@ def test___service_config___create_measurement_service___service_info_matches_se assert measurement_service.service_info.annotations == provided_annotations +@pytest.mark.parametrize( + "service_config,have_special_char", + [ + ( + "Control.serviceconfig", + False, + ), + ( + "Control.V1.serviceconfig", + False, + ), + ( + "Control.V2.serviceconfig", + False, + ), + ( + "ErrorDisplayName.serviceconfig", + True, + ), + ( + "ErrorAnnotations.serviceconfig", + True, + ), + ( + "ErrorTags.serviceconfig", + True, + ), + ( + "ErrorCollections.serviceconfig", + True, + ), + ], +) +def test___service_config___check_service_config___validiate_no_special_characters( + test_assets_directory: pathlib.Path, + service_config: str, + have_special_char: bool, +): + char_service_config = "check_char\\example." + service_config + try: + MeasurementService( + service_config_path=test_assets_directory / char_service_config, + version="1.0.0.0", + ui_file_paths=[], + ) + error_occurred = False + except NameError: + error_occurred = True + + assert error_occurred == have_special_char + + @pytest.mark.parametrize( "display_name,type,default_value,enum_type", [ From 922e3ddc4bea290f53027c852ebfcec0a66ebbaa Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Mon, 22 Jul 2024 12:09:24 -0500 Subject: [PATCH 02/10] Test only use error files and fixed config path --- .../example.Control.V1.serviceconfig | 19 --------- .../example.Control.V2.serviceconfig | 19 --------- .../check_char/example.Control.serviceconfig | 19 --------- packages/service/tests/unit/test_service.py | 42 ++++--------------- 4 files changed, 8 insertions(+), 91 deletions(-) delete mode 100644 packages/service/tests/assets/check_char/example.Control.V1.serviceconfig delete mode 100644 packages/service/tests/assets/check_char/example.Control.V2.serviceconfig delete mode 100644 packages/service/tests/assets/check_char/example.Control.serviceconfig diff --git a/packages/service/tests/assets/check_char/example.Control.V1.serviceconfig b/packages/service/tests/assets/check_char/example.Control.V1.serviceconfig deleted file mode 100644 index 305fd8840..000000000 --- a/packages/service/tests/assets/check_char/example.Control.V1.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "SampleMeasurement", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", - "ni/service.collection": "CurrentTests.Inrush", - "ni/service.tags": [ "powerup", "current" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.Control.V2.serviceconfig b/packages/service/tests/assets/check_char/example.Control.V2.serviceconfig deleted file mode 100644 index 305fd8840..000000000 --- a/packages/service/tests/assets/check_char/example.Control.V2.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "SampleMeasurement", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", - "ni/service.collection": "CurrentTests.Inrush", - "ni/service.tags": [ "powerup", "current" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.Control.serviceconfig b/packages/service/tests/assets/check_char/example.Control.serviceconfig deleted file mode 100644 index 305fd8840..000000000 --- a/packages/service/tests/assets/check_char/example.Control.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "SampleMeasurement", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", - "ni/service.collection": "CurrentTests.Inrush", - "ni/service.tags": [ "powerup", "current" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index edea0121c..bef7b862b 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -427,55 +427,29 @@ def test___service_config___create_measurement_service___service_info_matches_se @pytest.mark.parametrize( - "service_config,have_special_char", + "service_config", [ - ( - "Control.serviceconfig", - False, - ), - ( - "Control.V1.serviceconfig", - False, - ), - ( - "Control.V2.serviceconfig", - False, - ), - ( - "ErrorDisplayName.serviceconfig", - True, - ), - ( - "ErrorAnnotations.serviceconfig", - True, - ), - ( - "ErrorTags.serviceconfig", - True, - ), - ( - "ErrorCollections.serviceconfig", - True, - ), + "example.ErrorDisplayName.serviceconfig", + "example.ErrorAnnotations.serviceconfig", + "example.ErrorTags.serviceconfig", + "example.ErrorCollections.serviceconfig", ], ) -def test___service_config___check_service_config___validiate_no_special_characters( +def test___service_config___check_service_config___have_error( test_assets_directory: pathlib.Path, service_config: str, - have_special_char: bool, ): - char_service_config = "check_char\\example." + service_config try: MeasurementService( - service_config_path=test_assets_directory / char_service_config, + service_config_path=test_assets_directory / f"check_char/{service_config}", version="1.0.0.0", ui_file_paths=[], ) error_occurred = False except NameError: error_occurred = True + assert error_occurred - assert error_occurred == have_special_char @pytest.mark.parametrize( From c309a01981197d2f91263b3adba753fda0f98d16 Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Mon, 22 Jul 2024 12:13:28 -0500 Subject: [PATCH 03/10] Fixed styleguide --- packages/service/tests/unit/test_service.py | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index bef7b862b..689115504 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -451,7 +451,6 @@ def test___service_config___check_service_config___have_error( assert error_occurred - @pytest.mark.parametrize( "display_name,type,default_value,enum_type", [ From 3a67591c08787e56cd08f13e6d793805740a05e8 Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Mon, 22 Jul 2024 13:16:00 -0500 Subject: [PATCH 04/10] Changed test name --- packages/service/tests/unit/test_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index 689115504..06b07e39e 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -435,7 +435,7 @@ def test___service_config___create_measurement_service___service_info_matches_se "example.ErrorCollections.serviceconfig", ], ) -def test___service_config___check_service_config___have_error( +def test___service_config___check_service_config___name_error_exception( test_assets_directory: pathlib.Path, service_config: str, ): From c73bb40ee921d54768fed534de42f4820de6b06e Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Wed, 24 Jul 2024 12:37:01 -0500 Subject: [PATCH 05/10] Reverted all changes --- .../measurement/service.py | 5 +--- .../example.ErrorAnnotations.serviceconfig | 19 -------------- .../example.ErrorCollections.serviceconfig | 19 -------------- .../example.ErrorDisplayName.serviceconfig | 19 -------------- .../example.ErrorTags.serviceconfig | 19 -------------- packages/service/tests/unit/test_service.py | 25 ------------------- 6 files changed, 1 insertion(+), 105 deletions(-) delete mode 100644 packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig delete mode 100644 packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig delete mode 100644 packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig delete mode 100644 packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig diff --git a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py index 625faf454..ab8718563 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py +++ b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py @@ -210,10 +210,7 @@ def __init__( raise RuntimeError(f"File does not exist. {service_config_path}") with open(service_config_path) as service_config_file: - try: - service_config = json.loads(service_config_file.read()) - except json.decoder.JSONDecodeError: - raise NameError(f"Invalid character(s) in {service_config_path}") + service_config = json.load(service_config_file) if service_class is None: service = next(iter(service_config["services"]), None) diff --git a/packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig deleted file mode 100644 index 5a070d54f..000000000 --- a/packages/service/tests/assets/check_char/example.ErrorAnnotations.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "SampleMeasurement", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": """, - "ni/service.collection": "CurrentTests.Inrush", - "ni/service.tags": [ "powerup", "current" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig deleted file mode 100644 index 1dc3c2f05..000000000 --- a/packages/service/tests/assets/check_char/example.ErrorCollections.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "SampleMeasurement\", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", - "ni/service.collection": "\", - "ni/service.tags": [ "powerup", "current" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig deleted file mode 100644 index 7df8043f4..000000000 --- a/packages/service/tests/assets/check_char/example.ErrorDisplayName.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "\\SampleMeasurement\", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", - "ni/service.collection": "CurrentTests.Inrush", - "ni/service.tags": [ "powerup", "current" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig b/packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig deleted file mode 100644 index 053b7430a..000000000 --- a/packages/service/tests/assets/check_char/example.ErrorTags.serviceconfig +++ /dev/null @@ -1,19 +0,0 @@ -{ - "services": [ - { - "displayName": "SampleMeasurement\", - "serviceClass": "SampleMeasurement_Python", - "descriptionUrl": "https://www.example.com/SampleMeasurement.html", - "providedInterfaces": [ - "ni.measurementlink.measurement.v1.MeasurementService", - "ni.measurementlink.measurement.v2.MeasurementService" - ], - "path": "start.bat", - "annotations": { - "ni/service.description": "Measure inrush current with a shorted load and validate results against configured limits.", - "ni/service.collection": "CurrentTests.Inrush", - "ni/service.tags": [ "powerup", "\"" ] - } - } - ] -} \ No newline at end of file diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index 06b07e39e..46450ddd4 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -426,31 +426,6 @@ def test___service_config___create_measurement_service___service_info_matches_se assert measurement_service.service_info.annotations == provided_annotations -@pytest.mark.parametrize( - "service_config", - [ - "example.ErrorDisplayName.serviceconfig", - "example.ErrorAnnotations.serviceconfig", - "example.ErrorTags.serviceconfig", - "example.ErrorCollections.serviceconfig", - ], -) -def test___service_config___check_service_config___name_error_exception( - test_assets_directory: pathlib.Path, - service_config: str, -): - try: - MeasurementService( - service_config_path=test_assets_directory / f"check_char/{service_config}", - version="1.0.0.0", - ui_file_paths=[], - ) - error_occurred = False - except NameError: - error_occurred = True - assert error_occurred - - @pytest.mark.parametrize( "display_name,type,default_value,enum_type", [ From e9b6f6e44774e7e811dee6af4b970fce77e1ed2d Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Tue, 30 Jul 2024 09:40:56 -0500 Subject: [PATCH 06/10] Added display_name check and test --- .../_internal/parameter/metadata.py | 14 ++++++++ .../measurement/service.py | 2 ++ packages/service/tests/unit/test_decoder.py | 8 +++-- packages/service/tests/unit/test_metadata.py | 32 ++++++++++++++++++- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index 1abdadeaf..1afc3ae04 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -23,6 +23,8 @@ SupportedEnumType = Union[Type[Enum], _EnumTypeWrapper] +_VALID_CHARS = set(" ().,;:!?-_'+") + class ParameterMetadata(NamedTuple): """Class that represents the metadata of parameters.""" @@ -86,6 +88,18 @@ def initialize( ) +def validate_display_name(display_name: str) -> None: + """Validate and raise exception if 'display_name' has invalid characters. + + Raises: + ValueError: If display_name has invalid characters. + """ + if display_name == "" or not display_name[0].isalpha(): + raise ValueError("First letter in " + display_name + " must be a letter") + elif not all(char in _VALID_CHARS or char.isalnum() for char in display_name): + raise ValueError("Invalid characters in " + display_name) + + def validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: """Validate and raise exception if the default value does not match the type info. diff --git a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py index ff78e6b1f..dec2ef94b 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py +++ b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py @@ -411,6 +411,7 @@ def configuration( "DataType.PinArray1D is deprecated. Use DataType.IOResourceArray1D instead.", DeprecationWarning, ) + parameter_metadata.validate_display_name(display_name) data_type_info = _datatypeinfo.get_type_info(type) annotations = self._make_annotations_dict( data_type_info.type_specialization, instrument_type=instrument_type, enum_type=enum_type @@ -472,6 +473,7 @@ def output( "DataType.PinArray1D is deprecated. Use DataType.IOResourceArray1D instead.", DeprecationWarning, ) + parameter_metadata.validate_display_name(display_name) data_type_info = _datatypeinfo.get_type_info(type) annotations = self._make_annotations_dict( data_type_info.type_specialization, enum_type=enum_type diff --git a/packages/service/tests/unit/test_decoder.py b/packages/service/tests/unit/test_decoder.py index 9a69ae8e9..330cdda40 100644 --- a/packages/service/tests/unit/test_decoder.py +++ b/packages/service/tests/unit/test_decoder.py @@ -176,7 +176,7 @@ def _get_grpc_serialized_data(values): def _get_test_parameter_by_id(default_values): parameter_by_id = { 1: ParameterMetadata.initialize( - display_name="float_data!", + display_name="float_data", type=type_pb2.Field.TYPE_FLOAT, repeated=False, default_value=default_values[0], @@ -379,7 +379,7 @@ def _get_test_grpc_message(test_values): return parameter -def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata]: +def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata.initialize]: return { i + 1: ParameterMetadata.initialize( @@ -398,7 +398,9 @@ def _get_big_message(values: Sequence[float]) -> BigMessage: return BigMessage(**{f"field{i + 1}": value for (i, value) in enumerate(values)}) -def _test_create_file_descriptor(metadata: List[ParameterMetadata], file_name: str) -> str: +def _test_create_file_descriptor( + metadata: List[ParameterMetadata.initialize], file_name: str +) -> str: pool = descriptor_pool.Default() try: pool.FindMessageTypeByName(f"{file_name}.test") diff --git a/packages/service/tests/unit/test_metadata.py b/packages/service/tests/unit/test_metadata.py index a4f0c5398..17b45712b 100644 --- a/packages/service/tests/unit/test_metadata.py +++ b/packages/service/tests/unit/test_metadata.py @@ -10,7 +10,10 @@ TYPE_SPECIALIZATION_KEY, ) from ni_measurement_plugin_sdk_service._internal.parameter import metadata -from ni_measurement_plugin_sdk_service.measurement.info import DataType, TypeSpecialization +from ni_measurement_plugin_sdk_service.measurement.info import ( + DataType, + TypeSpecialization, +) class Color(Enum): @@ -148,3 +151,30 @@ def test___default_value_same_as_type___validate___raises_no_exception( ) metadata.validate_default_value_type(parameter_metadata) # implicitly assert does not throw + + +@pytest.mark.parametrize( + "display_name", + [ + " test_string", + "_____()!", + "test@string", + "", + ], +) +def test___display_name_invalid_characters___validate___raises_value_exception(display_name): + with pytest.raises(ValueError): + metadata.validate_display_name(display_name) + + +@pytest.mark.parametrize( + "display_name", + [ + "teststring()", + "tEsT StRIng?", + "test_string!", + "Test String: -10", + ], +) +def test___display_name_valid_characters___validate___raises_no_exception(display_name): + metadata.validate_display_name(display_name) From 1378ef25c5969e4247a7a67112b8478458a29e3f Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Tue, 30 Jul 2024 09:54:58 -0500 Subject: [PATCH 07/10] Fixed mypy check --- packages/service/tests/unit/test_decoder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/service/tests/unit/test_decoder.py b/packages/service/tests/unit/test_decoder.py index 330cdda40..8d7258ccb 100644 --- a/packages/service/tests/unit/test_decoder.py +++ b/packages/service/tests/unit/test_decoder.py @@ -379,7 +379,7 @@ def _get_test_grpc_message(test_values): return parameter -def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata.initialize]: +def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata]: return { i + 1: ParameterMetadata.initialize( @@ -399,7 +399,7 @@ def _get_big_message(values: Sequence[float]) -> BigMessage: def _test_create_file_descriptor( - metadata: List[ParameterMetadata.initialize], file_name: str + metadata: List[ParameterMetadata], file_name: str ) -> str: pool = descriptor_pool.Default() try: From c2636f2e01353f414368eb36fc237145f71fddbc Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Tue, 30 Jul 2024 10:17:55 -0500 Subject: [PATCH 08/10] Fixed formating --- packages/service/tests/unit/test_decoder.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/service/tests/unit/test_decoder.py b/packages/service/tests/unit/test_decoder.py index 8d7258ccb..6add901d7 100644 --- a/packages/service/tests/unit/test_decoder.py +++ b/packages/service/tests/unit/test_decoder.py @@ -398,9 +398,7 @@ def _get_big_message(values: Sequence[float]) -> BigMessage: return BigMessage(**{f"field{i + 1}": value for (i, value) in enumerate(values)}) -def _test_create_file_descriptor( - metadata: List[ParameterMetadata], file_name: str -) -> str: +def _test_create_file_descriptor(metadata: List[ParameterMetadata], file_name: str) -> str: pool = descriptor_pool.Default() try: pool.FindMessageTypeByName(f"{file_name}.test") From baeff8a9d84297535c32161e23e226e2913d4ba2 Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Thu, 1 Aug 2024 09:55:50 -0500 Subject: [PATCH 09/10] Initialzie call validate functions and fixed error messages --- .../_internal/parameter/metadata.py | 20 +++++++++++----- .../measurement/service.py | 5 ++-- packages/service/tests/unit/test_decoder.py | 23 +++++++++++++++++++ packages/service/tests/unit/test_metadata.py | 8 +++---- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index 1afc3ae04..4ee813646 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -60,6 +60,7 @@ class ParameterMetadata(NamedTuple): @staticmethod def initialize( + validate_type: bool, display_name: str, type: type_pb2.Field.Kind.ValueType, repeated: bool, @@ -69,14 +70,16 @@ def initialize( enum_type: Optional[SupportedEnumType] = None, ) -> "ParameterMetadata": """Initialize ParameterMetadata with field_name.""" + _validate_display_name(display_name) underscore_display_name = display_name.replace(" ", "_") + if all(char.isalnum() or char == "_" for char in underscore_display_name): field_name = underscore_display_name else: field_name = "".join( char for char in underscore_display_name if char.isalnum() or char == "_" ) - return ParameterMetadata( + parameter_metadata = ParameterMetadata( display_name, type, repeated, @@ -86,21 +89,26 @@ def initialize( field_name, enum_type, ) + if validate_type: + _validate_default_value_type(parameter_metadata) + return parameter_metadata -def validate_display_name(display_name: str) -> None: +def _validate_display_name(display_name: str) -> None: """Validate and raise exception if 'display_name' has invalid characters. Raises: ValueError: If display_name has invalid characters. """ - if display_name == "" or not display_name[0].isalpha(): - raise ValueError("First letter in " + display_name + " must be a letter") + if not display_name: + raise ValueError("The display name cannot be an empty string.") + elif not display_name[0].isalpha(): + raise ValueError(f"The first character in display name: '{display_name}' must be a letter.") elif not all(char in _VALID_CHARS or char.isalnum() for char in display_name): - raise ValueError("Invalid characters in " + display_name) + raise ValueError(f"There are Invalid characters in display name: '{display_name}'.") -def validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: +def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: """Validate and raise exception if the default value does not match the type info. Args: diff --git a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py index dec2ef94b..e1dc39b2d 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py +++ b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py @@ -411,12 +411,12 @@ def configuration( "DataType.PinArray1D is deprecated. Use DataType.IOResourceArray1D instead.", DeprecationWarning, ) - parameter_metadata.validate_display_name(display_name) data_type_info = _datatypeinfo.get_type_info(type) annotations = self._make_annotations_dict( data_type_info.type_specialization, instrument_type=instrument_type, enum_type=enum_type ) parameter = parameter_metadata.ParameterMetadata.initialize( + True, display_name, data_type_info.grpc_field_type, data_type_info.repeated, @@ -425,7 +425,6 @@ def configuration( data_type_info.message_type, enum_type, ) - parameter_metadata.validate_default_value_type(parameter) self._configuration_parameter_list.append(parameter) def _configuration(func: _F) -> _F: @@ -473,12 +472,12 @@ def output( "DataType.PinArray1D is deprecated. Use DataType.IOResourceArray1D instead.", DeprecationWarning, ) - parameter_metadata.validate_display_name(display_name) data_type_info = _datatypeinfo.get_type_info(type) annotations = self._make_annotations_dict( data_type_info.type_specialization, enum_type=enum_type ) parameter = parameter_metadata.ParameterMetadata.initialize( + False, display_name, data_type_info.grpc_field_type, data_type_info.repeated, diff --git a/packages/service/tests/unit/test_decoder.py b/packages/service/tests/unit/test_decoder.py index 6add901d7..3c902329f 100644 --- a/packages/service/tests/unit/test_decoder.py +++ b/packages/service/tests/unit/test_decoder.py @@ -176,6 +176,7 @@ def _get_grpc_serialized_data(values): def _get_test_parameter_by_id(default_values): parameter_by_id = { 1: ParameterMetadata.initialize( + validate_type=False, display_name="float_data", type=type_pb2.Field.TYPE_FLOAT, repeated=False, @@ -183,6 +184,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 2: ParameterMetadata.initialize( + validate_type=False, display_name="double_data", type=type_pb2.Field.TYPE_DOUBLE, repeated=False, @@ -190,6 +192,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 3: ParameterMetadata.initialize( + validate_type=False, display_name="int32_data", type=type_pb2.Field.TYPE_INT32, repeated=False, @@ -197,6 +200,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 4: ParameterMetadata.initialize( + validate_type=False, display_name="uint32_data", type=type_pb2.Field.TYPE_INT64, repeated=False, @@ -204,6 +208,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 5: ParameterMetadata.initialize( + validate_type=False, display_name="int64_data", type=type_pb2.Field.TYPE_UINT32, repeated=False, @@ -211,6 +216,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 6: ParameterMetadata.initialize( + validate_type=False, display_name="uint64_data", type=type_pb2.Field.TYPE_UINT64, repeated=False, @@ -218,6 +224,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 7: ParameterMetadata.initialize( + validate_type=False, display_name="bool_data", type=type_pb2.Field.TYPE_BOOL, repeated=False, @@ -225,6 +232,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 8: ParameterMetadata.initialize( + validate_type=False, display_name="string_data", type=type_pb2.Field.TYPE_STRING, repeated=False, @@ -232,6 +240,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 9: ParameterMetadata.initialize( + validate_type=False, display_name="double_array_data", type=type_pb2.Field.TYPE_DOUBLE, repeated=True, @@ -239,6 +248,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 10: ParameterMetadata.initialize( + validate_type=False, display_name="float_array_data", type=type_pb2.Field.TYPE_FLOAT, repeated=True, @@ -246,6 +256,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 11: ParameterMetadata.initialize( + validate_type=False, display_name="int32_array_data", type=type_pb2.Field.TYPE_INT32, repeated=True, @@ -253,6 +264,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 12: ParameterMetadata.initialize( + validate_type=False, display_name="uint32_array_data", type=type_pb2.Field.TYPE_UINT32, repeated=True, @@ -260,6 +272,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 13: ParameterMetadata.initialize( + validate_type=False, display_name="int64_array_data", type=type_pb2.Field.TYPE_INT64, repeated=True, @@ -267,6 +280,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 14: ParameterMetadata.initialize( + validate_type=False, display_name="uint64_array_data", type=type_pb2.Field.TYPE_UINT64, repeated=True, @@ -274,6 +288,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 15: ParameterMetadata.initialize( + validate_type=False, display_name="bool_array_data", type=type_pb2.Field.TYPE_BOOL, repeated=True, @@ -281,6 +296,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 16: ParameterMetadata.initialize( + validate_type=False, display_name="string_array_data", type=type_pb2.Field.TYPE_STRING, repeated=True, @@ -288,6 +304,7 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 17: ParameterMetadata.initialize( + validate_type=False, display_name="enum_data", type=type_pb2.Field.TYPE_ENUM, repeated=False, @@ -299,6 +316,7 @@ def _get_test_parameter_by_id(default_values): enum_type=DifferentColor, ), 18: ParameterMetadata.initialize( + validate_type=False, display_name="enum_array_data", type=type_pb2.Field.TYPE_ENUM, repeated=True, @@ -310,6 +328,7 @@ def _get_test_parameter_by_id(default_values): enum_type=DifferentColor, ), 19: ParameterMetadata.initialize( + validate_type=False, display_name="int_enum_data", type=type_pb2.Field.TYPE_ENUM, repeated=False, @@ -321,6 +340,7 @@ def _get_test_parameter_by_id(default_values): enum_type=Countries, ), 20: ParameterMetadata.initialize( + validate_type=False, display_name="int_enum_array_data", type=type_pb2.Field.TYPE_ENUM, repeated=True, @@ -332,6 +352,7 @@ def _get_test_parameter_by_id(default_values): enum_type=Countries, ), 21: ParameterMetadata.initialize( + validate_type=False, display_name="xy_data", type=type_pb2.Field.TYPE_MESSAGE, repeated=False, @@ -340,6 +361,7 @@ def _get_test_parameter_by_id(default_values): message_type=xydata_pb2.DoubleXYData.DESCRIPTOR.full_name, ), 22: ParameterMetadata.initialize( + validate_type=False, display_name="xy_data_array", type=type_pb2.Field.TYPE_MESSAGE, repeated=True, @@ -383,6 +405,7 @@ def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata]: return { i + 1: ParameterMetadata.initialize( + validate_type=False, display_name=f"field{i + 1}", type=type_pb2.Field.TYPE_DOUBLE, repeated=False, diff --git a/packages/service/tests/unit/test_metadata.py b/packages/service/tests/unit/test_metadata.py index 17b45712b..e5ca94dd0 100644 --- a/packages/service/tests/unit/test_metadata.py +++ b/packages/service/tests/unit/test_metadata.py @@ -93,7 +93,7 @@ def test___default_value_different_from_type___validate___raises_type_exception( ) with pytest.raises(TypeError): - metadata.validate_default_value_type(parameter_metadata) + metadata._validate_default_value_type(parameter_metadata) @pytest.mark.parametrize( @@ -150,7 +150,7 @@ def test___default_value_same_as_type___validate___raises_no_exception( annotations, ) - metadata.validate_default_value_type(parameter_metadata) # implicitly assert does not throw + metadata._validate_default_value_type(parameter_metadata) # implicitly assert does not throw @pytest.mark.parametrize( @@ -164,7 +164,7 @@ def test___default_value_same_as_type___validate___raises_no_exception( ) def test___display_name_invalid_characters___validate___raises_value_exception(display_name): with pytest.raises(ValueError): - metadata.validate_display_name(display_name) + metadata._validate_display_name(display_name) @pytest.mark.parametrize( @@ -177,4 +177,4 @@ def test___display_name_invalid_characters___validate___raises_value_exception(d ], ) def test___display_name_valid_characters___validate___raises_no_exception(display_name): - metadata.validate_display_name(display_name) + metadata._validate_display_name(display_name) From 3b8667d479aa97184e0d47d21dc4a1b3351ba244 Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Mon, 5 Aug 2024 08:15:05 -0500 Subject: [PATCH 10/10] Changed error messages --- .../_internal/parameter/metadata.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index 4ee813646..39ab664d5 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -103,9 +103,9 @@ def _validate_display_name(display_name: str) -> None: if not display_name: raise ValueError("The display name cannot be an empty string.") elif not display_name[0].isalpha(): - raise ValueError(f"The first character in display name: '{display_name}' must be a letter.") + raise ValueError(f"The first character in display name '{display_name}' must be a letter.") elif not all(char in _VALID_CHARS or char.isalnum() for char in display_name): - raise ValueError(f"There are Invalid characters in display name: '{display_name}'.") + raise ValueError(f"There are invalid characters in display name '{display_name}'.") def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: