Improve quantity check

Give proper error if "definition" is not given and not any other
attributes either.
(In that case v is NoneType som you cannot check for "definition")

Signed-off-by: Erik Jaegervall <[email protected]>

tests/vspec/test_quantities.yaml

@@ -0,0 +1,9 @@
+# Default file for testing
+
+length:
+  definition: Linear extent in space between any two points (ISO 80000-3:2019)
+  remark: Length does not need to be measured along a straight line.
+          Length is one of the seven base quantities in the International System of Units (ISO 80000-1).
+temperature:
+  definition: Partial derivative of internal energy with respect to entropy at constant volume
+              and constant number of particles in the system (ISO 80000-3:2019)

tests/vspec/test_units.yaml

@@ -1,12 +1,12 @@
 # Unit file for test purposes only
 # Includes a subset of units existing in VSS catalog
-# For new test cases consider using
-units:
-  km:
-    label: kilometer
-    description: Distance measured in kilometers
-    domain: distance
-  celsius:
-    label: degree celsius
-    description: Temperature measured in degree celsius
-    domain: temperature
+km:
+  definition: Length measured in kilometers
+  unit: kilometer
+  quantity: length
+  allowed-datatypes: ['numeric']
+celsius:
+  definition: Temperature measured in degree celsius
+  unit: degree celsius
+  quantity: temperature
+  allowed-datatypes: ['numeric']

tests/vspec/test_units/quantities_no_def.yaml

@@ -0,0 +1,2 @@
+# Quantity but no definition which is mandatory
+volume:

tests/vspec/test_units/test_units.py

@@ -35,7 +35,7 @@ def run_unit(vspec_file, unit_argument, expected_file, quantity_argument="",
     assert os.WIFEXITED(result)
     assert os.WEXITSTATUS(result) == 0
 
-    # Verify expected quntity
+    # Verify expected quantity
 
     if grep_string is not None:
         test_str = 'grep \"' + grep_string + '\" out.txt > /dev/null'
@@ -49,9 +49,9 @@ def run_unit(vspec_file, unit_argument, expected_file, quantity_argument="",
     os.system("rm -f out.txt")
 
 
-def run_unit_error(vspec_file, unit_argument, grep_error):
+def run_unit_error(vspec_file, unit_argument, grep_error, quantity_argument=""):
     test_str = "../../../vspec2json.py --json-pretty " + \
-        vspec_file + " " + unit_argument + " out.json > out.txt 2>&1"
+        vspec_file + " " + unit_argument + " " + quantity_argument + " out.json > out.txt 2>&1"
     result = os.system(test_str)
     assert os.WIFEXITED(result)
     # failure expected
@@ -66,8 +66,8 @@ def run_unit_error(vspec_file, unit_argument, grep_error):
 
 # #################### Tests #############################
 
-# Short form
 
+# Short form
 
 def test_single_u(change_test_dir):
     run_unit("signals_with_special_units.vspec", "-u units_all.yaml", "expected_special.json")
@@ -165,3 +165,12 @@ def test_quantity_redefinition(change_test_dir):
         "--unit-file units_all.yaml",
         "expected_special.json",
         "-q quantity_volym.yaml -q quantity_volym.yaml", True, "Redefinition of quantity volym")
+
+
+def test_quantity_err_no_def(change_test_dir):
+    """
+    Test scenario when definition is not given
+    """
+    run_unit_error("signals_with_special_units.vspec", "-u units_all.yaml",
+                   "No definition found for quantity volume",
+                   "-q quantities_no_def.yaml")

tests/vspec/test_units/expected_default.json → tests/vspec/test_units_no_quantity/expected.json

@@ -1,11 +1,11 @@
 {
   "A": {
     "children": {
-      "SignalLiter": {
+      "Km": {
         "datatype": "float",
-        "description": "Volume in liters.",
+        "description": "Something in km.",
         "type": "sensor",
-        "unit": "l"
+        "unit": "km"
       }
     },
     "description": "Branch A.",

tests/vspec/test_units_no_quantity/test.vspec

@@ -0,0 +1,17 @@
+# Copyright (c) 2023 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: Branch A.
+
+A.Km:
+  datatype: float
+  type: sensor
+  unit: km
+  description: Something in km.

tests/vspec/test_units_no_quantity/test_quantities.yaml

@@ -0,0 +1,3 @@
+# Default file for testing
+volume:
+  definition: Extent of a three‑dimensional geometrical shape (ISO 80000-3:2019)

tests/vspec/test_units_no_quantity/test_units_no_quantity.py

@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+
+# Copyright (c) 2023 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
+from typing import Optional
+
+
+# #################### Helper methods #############################
+
+@pytest.fixture
+def change_test_dir(request, monkeypatch):
+    # To make sure we run from test directory
+    monkeypatch.chdir(request.fspath.dirname)
+
+
+def run_unit(vspec_file, unit_argument, expected_file, quantity_argument="",
+             grep_present: bool = True, grep_string: Optional[str] = None):
+    test_str = "../../../vspec2json.py --json-pretty " + \
+        vspec_file + " " + unit_argument + " " + quantity_argument + " out.json > out.txt 2>&1"
+    result = os.system(test_str)
+    assert os.WIFEXITED(result)
+    assert os.WEXITSTATUS(result) == 0
+
+    test_str = "diff out.json " + expected_file
+    result = os.system(test_str)
+    os.system("rm -f out.json")
+    assert os.WIFEXITED(result)
+    assert os.WEXITSTATUS(result) == 0
+
+    # Verify expected quntity
+
+    if grep_string is not None:
+        test_str = 'grep \"' + grep_string + '\" out.txt > /dev/null'
+        result = os.system(test_str)
+        assert os.WIFEXITED(result)
+        if grep_present:
+            assert os.WEXITSTATUS(result) == 0
+        else:
+            assert os.WEXITSTATUS(result) == 1
+
+    os.system("rm -f out.txt")
+
+
+# #################### Tests #############################
+
+def test_default_unit_no_quantity_warning(change_test_dir):
+    """
+    If no quantity file is found it shall only inform about that
+    """
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "", True,
+             "No quantities defined")
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "", False,
+             "Quantity length used by unit km has not been defined")
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "", False,
+             "Quantity temperature used by unit celsius has not been defined")
+
+
+def test_default_unit_quantities_missing(change_test_dir):
+    """
+    If quantity file found it shall inform about missing quantities (once for each)
+    """
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "-q test_quantities.yaml", False,
+             "No quantities defined")
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "-q test_quantities.yaml", True,
+             "Quantity length used by unit km has not been defined")
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "-q test_quantities.yaml", True,
+             "Quantity temperature used by unit celsius has not been defined")
+
+
+def test_default_unit_quantities_ok(change_test_dir):
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "-q ../test_quantities.yaml", False,
+             "No quantities defined")
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "-q ../test_quantities.yaml", False,
+             "Quantity length used by unit km has not been defined")
+    run_unit("test.vspec", "-u ../test_units.yaml", "expected.json", "-q ../test_quantities.yaml", False,
+             "Quantity temperature used by unit celsius has not been defined")

vspec/model/constants.py

@@ -20,6 +20,7 @@
 from typing import (
     Sequence, Type, TypeVar, Optional, Dict, TextIO
 )
+from collections import abc
 
 import yaml
 
@@ -196,8 +197,11 @@ def load_config_file(cls, config_file: str) -> int:
                     logging.error("No quantity (domain) found for unit %s", k)
                     sys.exit(-1)
 
-                if VSSQuantityCollection.get_quantity(quantity) is None:
+                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
                     logging.info("Quantity %s used by unit %s has not been defined", quantity, k)
+                    VSSQuantityCollection.add_quantity(quantity)
 
                 unit_node = VSSUnit(k, unit, definition, quantity)
                 if k in cls.units:
@@ -228,7 +232,7 @@ def load_config_file(cls, config_file: str) -> int:
             my_quantities = yaml.safe_load(my_yaml_file)
             added_quantities = len(my_quantities)
             for k, v in my_quantities.items():
-                if "definition" in v:
+                if isinstance(v, abc.Mapping) and "definition" in v:
                     definition = v["definition"]
                 else:
                     logging.error("No definition found for quantity %s", k)
@@ -253,6 +257,16 @@ def get_quantity(cls, id: str) -> Optional[VSSQuantity]:
         else:
             return None
 
+    @classmethod
+    def nbr_quantities(cls) -> int:
+        return len(cls.quantities)
+
+    @classmethod
+    def add_quantity(cls, id: str) -> None:
+        if id not in cls.quantities:
+            quantity_node = VSSQuantity(id, "Automatically generated quantity")
+            cls.quantities[id] = quantity_node
+
 
 class VSSTreeType(Enum, metaclass=EnumMetaWithReverseLookup):
     SIGNAL_TREE = "signal_tree"