Skip to content

Commit

Permalink
Update strict-handling
Browse files Browse the repository at this point in the history
Aligning messages to what is stated in documentation after
COVESA/vehicle_signal_specification#716

Signed-off-by: Erik Jaegervall <[email protected]>
  • Loading branch information
erikbosch committed Feb 21, 2024
1 parent 469bfa3 commit bff4ea6
Show file tree
Hide file tree
Showing 12 changed files with 215 additions and 60 deletions.
2 changes: 1 addition & 1 deletion docs/vspec2x.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
28 changes: 1 addition & 27 deletions tests/model/test_contants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
53 changes: 53 additions & 0 deletions tests/model/test_vsstree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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()
2 changes: 1 addition & 1 deletion tests/vspec/test_allowed/test_allowed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions tests/vspec/test_strict/correct.vspec
Original file line number Diff line number Diff line change
@@ -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!
17 changes: 17 additions & 0 deletions tests/vspec/test_strict/correct_boolean.vspec
Original file line number Diff line number Diff line change
@@ -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!
17 changes: 17 additions & 0 deletions tests/vspec/test_strict/faulty_case.vspec
Original file line number Diff line number Diff line change
@@ -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!
17 changes: 17 additions & 0 deletions tests/vspec/test_strict/faulty_name_boolean.vspec
Original file line number Diff line number Diff line change
@@ -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)!
77 changes: 77 additions & 0 deletions tests/vspec/test_strict/test_strict.py
Original file line number Diff line number Diff line change
@@ -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
22 changes: 1 addition & 21 deletions vspec/model/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#
# noinspection PyPackageRequirements
from __future__ import annotations
import re
import logging
import sys
from enum import Enum, EnumMeta
Expand All @@ -25,8 +24,6 @@
import yaml


NON_ALPHANUMERIC_WORD = re.compile('[^A-Za-z0-9]+')

T = TypeVar("T")


Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down
19 changes: 11 additions & 8 deletions vspec/model/vsstree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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:
"""
Expand Down
4 changes: 2 additions & 2 deletions vspec/vspec2x.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand Down

0 comments on commit bff4ea6

Please sign in to comment.