From 839b0d087ecb6419372476fb99a7d8c160c7d13b Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 21 Feb 2024 16:07:55 +0100 Subject: [PATCH] Update strict-handling Aligning messages to what is stated in documentation after https://github.com/COVESA/vehicle_signal_specification/pull/716 Signed-off-by: Erik Jaegervall --- docs/vspec2x.md | 2 +- tests/model/test_contants.py | 28 +------ tests/model/test_vsstree.py | 53 +++++++++++++ tests/vspec/test_allowed/test_allowed.py | 2 +- tests/vspec/test_strict/correct.vspec | 17 ++++ tests/vspec/test_strict/correct_boolean.vspec | 17 ++++ tests/vspec/test_strict/faulty_case.vspec | 17 ++++ .../test_strict/faulty_name_boolean.vspec | 17 ++++ tests/vspec/test_strict/test_strict.py | 77 +++++++++++++++++++ vspec/model/constants.py | 22 +----- vspec/model/vsstree.py | 19 +++-- vspec/vspec2x.py | 4 +- 12 files changed, 215 insertions(+), 60 deletions(-) create mode 100644 tests/vspec/test_strict/correct.vspec create mode 100644 tests/vspec/test_strict/correct_boolean.vspec create mode 100644 tests/vspec/test_strict/faulty_case.vspec create mode 100644 tests/vspec/test_strict/faulty_name_boolean.vspec create mode 100644 tests/vspec/test_strict/test_strict.py diff --git a/docs/vspec2x.md b/docs/vspec2x.md index 65191241..94867060 100644 --- a/docs/vspec2x.md +++ b/docs/vspec2x.md @@ -52,7 +52,7 @@ Terminates parsing when an unknown attribute is encountered, that is an attribut *Note*: Here an *attribute* refers to VSS signal metadata such as "datatype", "min", "max", ... and not to the VSS signal type attribute ### --abort-on-name-style -Terminates parsing, when the name of a signal does not follow [VSS recomendations](https://covesa.github.io/vehicle_signal_specification/rule_set/basics/#naming-conventions). +Terminates parsing, when the name of a signal does not follow [VSS Naming Conventions](https://covesa.github.io/vehicle_signal_specification/rule_set/basics/#naming-conventions) for the VSS standard catalog. ### --strict Equivalent to setting `--abort-on-unknown-attribute` and `--abort-on-name-style` diff --git a/tests/model/test_contants.py b/tests/model/test_contants.py index ebe10ce4..22b35044 100644 --- a/tests/model/test_contants.py +++ b/tests/model/test_contants.py @@ -9,36 +9,10 @@ import pytest import os -from vspec.model.constants import VSSType, VSSDataType, VSSUnitCollection, StringStyle, VSSTreeType +from vspec.model.constants import VSSType, VSSDataType, VSSUnitCollection, VSSTreeType from vspec.model.constants import VSSUnit, VSSQuantity -@pytest.mark.parametrize("style_enum, style_str", - [(StringStyle.NONE, "none"), - (StringStyle.CAMEL_CASE, "camelCase"), - (StringStyle.CAMEL_BACK, "camelBack"), - (StringStyle.CAPITAL_CASE, "capitalcase"), - (StringStyle.CONST_CASE, "constcase"), - (StringStyle.LOWER_CASE, "lowercase"), - (StringStyle.PASCAL_CASE, "pascalcase"), - (StringStyle.SENTENCE_CASE, "sentencecase"), - (StringStyle.SNAKE_CASE, "snakecase"), - (StringStyle.TITLE_CASE, "titlecase"), - (StringStyle.TRIM_CASE, "trimcase"), - (StringStyle.UPPER_CASE, "uppercase"), - (StringStyle.ALPHANUM_CASE, "alphanumcase")]) -def test_string_styles(style_enum, style_str): - """ - Test correct parsing of string styles - """ - assert style_enum == StringStyle.from_str(style_str) - - -def test_invalid_string_styles(): - with pytest.raises(Exception): - StringStyle.from_str("not_a_valid_case") - - @pytest.mark.parametrize("unit_file", ['explicit_units.yaml', 'explicit_units_old_syntax.yaml']) diff --git a/tests/model/test_vsstree.py b/tests/model/test_vsstree.py index c59cc574..c84276ba 100644 --- a/tests/model/test_vsstree.py +++ b/tests/model/test_vsstree.py @@ -12,6 +12,7 @@ from vspec.model.constants import VSSType, VSSDataType, VSSUnitCollection, VSSTreeType from vspec.model.vsstree import VSSNode +from vspec.model.exceptions import NameStyleValidationException class TestVSSNode(unittest.TestCase): @@ -266,6 +267,58 @@ def test_orphan_detection(self): self.assertTrue(node_drivetrain.is_orphan()) + def test_name_style_string(self): + + source = { + "description": "Some element.", + "type": "sensor", + "datatype": "string", + "$file_name$": "testfile"} + + node = VSSNode( + "this_name_is_not_allowed_in_standard_catalog", + source, + VSSTreeType.SIGNAL_TREE.available_types()) + with pytest.raises(NameStyleValidationException): + node.validate_name_style("dummyfile") + + node = VSSNode( + "ButThisNameIs", + source, + VSSTreeType.SIGNAL_TREE.available_types()) + # No exception expected + node.validate_name_style("dummyfile") + + def test_name_style_boolean(self): + + source = { + "description": "Some element.", + "type": "sensor", + "datatype": "boolean", + "$file_name$": "testfile"} + + node = VSSNode( + "this_name_is_not_allowed_in_standard_catalog", + source, + VSSTreeType.SIGNAL_TREE.available_types()) + with pytest.raises(NameStyleValidationException): + node.validate_name_style("dummyfile") + + # Boolean ones must start with "Is" + node = VSSNode( + "ThisNameNeither", + source, + VSSTreeType.SIGNAL_TREE.available_types()) + with pytest.raises(NameStyleValidationException): + node.validate_name_style("dummyfile") + + node = VSSNode( + "IsThisAllowedYesItIs", + source, + VSSTreeType.SIGNAL_TREE.available_types()) + # No exception expected + node.validate_name_style("dummyfile") + if __name__ == '__main__': unittest.main() diff --git a/tests/vspec/test_allowed/test_allowed.py b/tests/vspec/test_allowed/test_allowed.py index a19e13ac..e5d0bb94 100644 --- a/tests/vspec/test_allowed/test_allowed.py +++ b/tests/vspec/test_allowed/test_allowed.py @@ -43,7 +43,7 @@ def run_exporter(exporter, argument): os.system("rm -f out." + exporter + " out.txt") -def test_uuid(change_test_dir): +def test_allowed(change_test_dir): # Run all "supported" exporters, i.e. not those in contrib # Exception is "binary", as it is assumed output may vary depending on target diff --git a/tests/vspec/test_strict/correct.vspec b/tests/vspec/test_strict/correct.vspec new file mode 100644 index 00000000..2dda6220 --- /dev/null +++ b/tests/vspec/test_strict/correct.vspec @@ -0,0 +1,17 @@ +# Copyright (c) 2024 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +A: + type: branch + description: This description is mandatory! + +A.UInt8: + datatype: uint8 + type: sensor + unit: km + description: Name OK! diff --git a/tests/vspec/test_strict/correct_boolean.vspec b/tests/vspec/test_strict/correct_boolean.vspec new file mode 100644 index 00000000..1e915836 --- /dev/null +++ b/tests/vspec/test_strict/correct_boolean.vspec @@ -0,0 +1,17 @@ +# Copyright (c) 2024 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +A: + type: branch + description: This description is mandatory! + +A.IsSomething: + datatype: boolean + type: sensor + unit: km + description: OK name for a boolean! diff --git a/tests/vspec/test_strict/faulty_case.vspec b/tests/vspec/test_strict/faulty_case.vspec new file mode 100644 index 00000000..9496d80c --- /dev/null +++ b/tests/vspec/test_strict/faulty_case.vspec @@ -0,0 +1,17 @@ +# Copyright (c) 2024 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +A: + type: branch + description: This description is mandatory! + +A.uInt8: + datatype: uint8 + type: sensor + unit: km + description: First letter must be capital! diff --git a/tests/vspec/test_strict/faulty_name_boolean.vspec b/tests/vspec/test_strict/faulty_name_boolean.vspec new file mode 100644 index 00000000..04abd90f --- /dev/null +++ b/tests/vspec/test_strict/faulty_name_boolean.vspec @@ -0,0 +1,17 @@ +# Copyright (c) 2024 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +A: + type: branch + description: This description is mandatory! + +A.NotStartingWithIs: + datatype: boolean + type: sensor + unit: km + description: Faulty name for boolean (if for VSS catalog)! diff --git a/tests/vspec/test_strict/test_strict.py b/tests/vspec/test_strict/test_strict.py new file mode 100644 index 00000000..f84692ae --- /dev/null +++ b/tests/vspec/test_strict/test_strict.py @@ -0,0 +1,77 @@ +#!/usr/bin/env python3 + +# Copyright (c) 2024 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +import pytest +import os + + +@pytest.fixture +def change_test_dir(request, monkeypatch): + # To make sure we run from test directory + monkeypatch.chdir(request.fspath.dirname) + + +@pytest.mark.parametrize("vspec_file", [ + "correct.vspec", + "correct_boolean.vspec", + "faulty_case.vspec", + "faulty_name_boolean.vspec" + ]) +def test_not_strict(vspec_file: str, change_test_dir): + test_str = "../../../vspec2json.py --json-pretty -u ../test_units.yaml " + vspec_file + \ + " out.json > out.txt 2>&1" + result = os.system(test_str) + assert os.WIFEXITED(result) + assert os.WEXITSTATUS(result) == 0 + + test_str = 'grep \"You asked for strict checking. Terminating\" out.txt > /dev/null' + result = os.system(test_str) + os.system("cat out.txt") + os.system("rm -f out.json out.txt") + assert os.WIFEXITED(result) + assert os.WEXITSTATUS(result) != 0 + + +@pytest.mark.parametrize("vspec_file", [ + "correct.vspec", + "correct_boolean.vspec" + ]) +def test_strict_ok(vspec_file: str, change_test_dir): + test_str = "../../../vspec2json.py --json-pretty -s -u ../test_units.yaml " + vspec_file + \ + " out.json > out.txt 2>&1" + result = os.system(test_str) + assert os.WIFEXITED(result) + assert os.WEXITSTATUS(result) == 0 + + test_str = 'grep \"You asked for strict checking. Terminating\" out.txt > /dev/null' + result = os.system(test_str) + os.system("cat out.txt") + os.system("rm -f out.json out.txt") + assert os.WIFEXITED(result) + assert os.WEXITSTATUS(result) != 0 + + +@pytest.mark.parametrize("vspec_file", [ + "faulty_case.vspec", + "faulty_name_boolean.vspec" + ]) +def test_strict_error(vspec_file: str, change_test_dir): + test_str = "../../../vspec2json.py --json-pretty -s -u ../test_units.yaml " + vspec_file + \ + " out.json > out.txt 2>&1" + result = os.system(test_str) + assert os.WIFEXITED(result) + assert os.WEXITSTATUS(result) != 0 + + test_str = 'grep \"You asked for strict checking. Terminating.\" out.txt > /dev/null' + result = os.system(test_str) + os.system("cat out.txt") + os.system("rm -f out.json out.txt") + assert os.WIFEXITED(result) + assert os.WEXITSTATUS(result) == 0 diff --git a/vspec/model/constants.py b/vspec/model/constants.py index 5a7197a8..2826826c 100644 --- a/vspec/model/constants.py +++ b/vspec/model/constants.py @@ -13,7 +13,6 @@ # # noinspection PyPackageRequirements from __future__ import annotations -import re import logging import sys from enum import Enum, EnumMeta @@ -25,8 +24,6 @@ import yaml -NON_ALPHANUMERIC_WORD = re.compile('[^A-Za-z0-9]+') - T = TypeVar("T") @@ -98,23 +95,6 @@ def values(cls: Type[T]) -> Sequence[str]: return cls.__values__ # type: ignore[attr-defined] -class StringStyle(Enum, metaclass=EnumMetaWithReverseLookup): - NONE = "none" - CAMEL_CASE = "camelCase" - CAMEL_BACK = "camelBack" - CAPITAL_CASE = "capitalcase" - CONST_CASE = "constcase" - LOWER_CASE = "lowercase" - PASCAL_CASE = "pascalcase" - SENTENCE_CASE = "sentencecase" - SNAKE_CASE = "snakecase" - SPINAL_CASE = "spinalcase" - TITLE_CASE = "titlecase" - TRIM_CASE = "trimcase" - UPPER_CASE = "uppercase" - ALPHANUM_CASE = "alphanumcase" - - class VSSType(Enum, metaclass=EnumMetaWithReverseLookup): BRANCH = "branch" ATTRIBUTE = "attribute" @@ -211,7 +191,7 @@ def load_config_file(cls, config_file: str) -> int: if ((VSSQuantityCollection.nbr_quantities() > 0) and (VSSQuantityCollection.get_quantity(quantity) is None)): - # Only give info on first occurance and only if quantities exist at all + # Only give info on first occurrence and only if quantities exist at all logging.info("Quantity %s used by unit %s has not been defined", quantity, k) VSSQuantityCollection.add_quantity(quantity) diff --git a/vspec/model/vsstree.py b/vspec/model/vsstree.py index 5e3a383e..9d27fb06 100644 --- a/vspec/model/vsstree.py +++ b/vspec/model/vsstree.py @@ -79,7 +79,8 @@ def __init__(self, name, source_dict: dict, available_types: Set[str], parent=No parent: Optional parent of this node instance. children: Optional children instances of this node. break_on_unknown_attribute: Throw if the node contains attributes not in core VSS specification - break_on_name_style_vioation: Throw if this node's name is not follwing th VSS recommended style + break_on_name_style_violation: Throw if this node's name is not following + the VSS standard catalog naming conventions Returns: VSSNode object according to the Vehicle Signal Specification. @@ -104,10 +105,13 @@ def __init__(self, name, source_dict: dict, available_types: Set[str], parent=No try: self.validate_name_style(self.source_dict["$file_name$"]) except NameStyleValidationException as e: - logging.warning(f"Exception: {e}") + info_string = str(e) if break_on_name_style_violation: + logging.warning(info_string) logging.error("You asked for strict checking. Terminating.") sys.exit(-1) + else: + logging.info(info_string) def unpack_source_dict(self): self.extended_attributes = self.source_dict.copy() @@ -175,18 +179,17 @@ def validate_name_style(self, sourcefile): this conventions can still be a valid model. """ - camel_regexp = re.compile('[A-Z][A-Za-z0-9]*$') if self.is_signal() and self.datatype == VSSDataType.BOOLEAN and not self.name.startswith("Is"): raise NameStyleValidationException( - (f'Boolean node "{self.name}" found in file "{sourcefile}" is not following naming conventions. ', - 'It is recommended that boolean nodes start with "Is".')) + (f'Boolean node "{self.name}" found in file "{sourcefile}" ' + 'is not following VSS standard catalog naming conventions.')) + camel_regexp = re.compile('[A-Z][A-Za-z0-9]*$') # relax camel case requirement for struct properties if not self.is_property() and not camel_regexp.match(self.name): raise NameStyleValidationException( - (f'Node "{self.name}" found in file "{sourcefile}" is not following naming conventions. ', - 'It is recommended that node names use camel case, starting with a capital letter, ', - 'only using letters A-z and numbers 0-9.')) + (f'Node "{self.name}" found in file "{sourcefile}" ' + 'is not following VSS standard catalog naming conventions.')) def base_data_type_str(self) -> str: """ diff --git a/vspec/vspec2x.py b/vspec/vspec2x.py index 2979634b..d0b849a4 100755 --- a/vspec/vspec2x.py +++ b/vspec/vspec2x.py @@ -47,9 +47,9 @@ def main(self, arguments): help='Use strict checking: Terminate when anything not covered or not recommended ' 'by VSS language or extensions is found.') parser.add_argument('--abort-on-unknown-attribute', action='store_true', - help=" Terminate when an unknown attribute is found.") + help="Terminate when an unknown attribute is found.") parser.add_argument('--abort-on-name-style', action='store_true', - help=" Terminate naming style not follows recommendations.") + help="Terminate if name style does not follow VSS standard catalog naming convention.") if self.vspec2vss_config.uuid_supported: parser.add_argument('--uuid', action='store_true', help='Include uuid in generated files. (Deprecated, will be removed in VSS-tools 6.0)')