Allow extra properties & missing properties in testcases (#421)

* Allow extra properties & missing properties in testcases
* Add updated Documentation
* Delete formating workflow

Author: MaximilianSoerenPollak

.github/workflows/format.yml

@@ -25,10 +25,17 @@ TestLink will extract test name, file, line, result and verification lists
 (`PartiallyVerifies`, `FullyVerifies`) and create external needs from tests
 and `testlink` attributes on requirements that reference the test.
 
+.. hint::
+   It is possible to have 'additional' properties on tests. They will not show up in the
+   TestLink but also won't break the parsing process.
+
+
 
 Limitations
 -----------
 
 - Not compatible with Esbonio/Live_preview.
-- Tags and XML must match the expected format exactly for parsing to work.
+- To create a valid Testlink Tags and XML must match the expected format.
+- Partial properties will lead to no Testlink creation.
+  If you want a test to be linked, please ensure all requirement properties are provided.
 - Tests must be executed by Bazel first so `test.xml` files exist.

docs/how-to/test_to_doc_links.rst

@@ -79,28 +79,28 @@ def DataForTestLink_JSON_Decoder(d: dict[str, Any]) -> DataForTestLink | dict[st
 # We will have everything as string here as that mirrors the xml file
 @dataclass
 class DataOfTestCase:
-    name: str
-    file: str
-    line: str
-    result: str  # passed | falied | skipped | disabled
+    name: str | None = None
+    file: str | None = None
+    line: str | None = None
+    result: str | None = None  # passed | falied | skipped | disabled
     # Intentionally not snakecase to make dict parsing simple
-    TestType: str
-    DerivationTechnique: str
-    result_text: str = ""  # Can be None on anything but failed
+    TestType: str | None = None
+    DerivationTechnique: str | None = None
+    result_text: str | None = None  # Can be None on anything but failed
     # Either or HAVE to be filled.
     PartiallyVerifies: str | None = None
     FullyVerifies: str | None = None
 
     @classmethod
     def from_dict(cls, data: dict[str, Any]):  # type-ignore
         return cls(
-            name=data["name"],
-            file=data["file"],
-            line=data["line"],
-            result=data["result"],
-            TestType=data["TestType"],
-            DerivationTechnique=data["DerivationTechnique"],
-            result_text=data["result_text"],
+            name=data.get("name"),
+            file=data.get("file"),
+            line=data.get("line"),
+            result=data.get("result"),
+            TestType=data.get("TestType"),
+            DerivationTechnique=data.get("DerivationTechnique"),
+            result_text=data.get("result_text"),
             PartiallyVerifies=data.get("PartiallyVerifies"),
             FullyVerifies=data.get("FullyVerifies"),
         )
@@ -122,24 +122,63 @@ def __post_init__(self):
         # Cleaning text
         if self.result_text:
             self.result_text = self.clean_text(self.result_text)
-        # Self assertion to double check some mandatory options
         # For now this is disabled
 
-        # It's mandatory that the test either partially or fully verifies a requirement
-        # if self.PartiallyVerifies is None and self.FullyVerifies is None:
-        #     raise ValueError(
-        #         f"TestCase: {self.id} Error. Either 'PartiallyVerifies' or "
-        #         "'FullyVerifies' must be provided."
-        #     )
         # Skipped tests should always have a reason associated with them
         # if "skipped" in self.result.keys() and not list(self.result.values())[0]:
         #     raise ValueError(
         #         f"TestCase: {self.id} Error. Test was skipped without provided "
         #         "reason, reason is mandatory for skipped tests."
         #     )
 
+    # Self assertion to double check some mandatory options
+    def check_verifies_fields(self) -> bool:
+        if self.PartiallyVerifies is None and self.FullyVerifies is None:
+            # This might be a warning in the future, but for now we want be lenient.
+            LOGGER.info(
+                f"TestCase: {self.name} Error. Either 'PartiallyVerifies' or "
+                "'FullyVerifies' must be provided."
+                "This test case will be skipped and not linked.",
+                type="score_source_code_linker",
+            )
+            return False
+        # Either or is filled, this is fine
+        return True
+
+    def is_valid(self) -> bool:
+        if not self.check_verifies_fields():
+            return False
+
+        # if (
+        #     # Result Text can be None if result is not failed.
+        #         self.name is not None
+        #         and self.file is not None
+        #         and self.line is not None
+        #         and self.result is not None
+        #         and self.TestType is not None
+        #         and self.DerivationTechnique is not None
+        # ):
+        fields = [
+            x
+            for x in self.__dataclass_fields__
+            if x not in ["PartiallyVerifies", "FullyVerifies"]
+        ]
+        for field in fields:
+            if getattr(self, field) is None:
+                # This might be a warning in the future, but for now we want be lenient.
+                LOGGER.info(
+                    f"TestCase: {self.name} has a None value for the field: "
+                    f"{field}. This test case will be skipped and not linked.",
+                    type="score_source_code_linker",
+                )
+                return False
+        # All properties are filled
+        return True
+
     def get_test_links(self) -> list[DataForTestLink]:
         """Convert TestCaseNeed to list of TestLink objects."""
+        if not self.is_valid():
+            return []
 
         def parse_attributes(verify_field: str | None, verify_type: str):
             """Process a verification field and yield TestLink objects."""
@@ -151,11 +190,24 @@ def parse_attributes(verify_field: str | None, verify_type: str):
                 type="score_source_code_linker",
             )
 
+            # LSP can not figure out that 'is_valid' up top
+            # already gurantees non-None values here
+            # So we assert our worldview here to ensure type safety.
+            # Any of these being none should NOT happen at this point
+
+            assert self.name is not None
+            assert self.file is not None
+            assert self.line is not None
+            assert self.result is not None
+            assert self.result_text is not None
+            assert self.TestType is not None
+            assert self.DerivationTechnique is not None
+
             for need in verify_field.split(","):
                 yield DataForTestLink(
-                    name=self.name,
-                    file=Path(self.file),
-                    line=int(self.line),
+                    name=self.name,  # type-ignore
+                    file=Path(self.file),  # type-ignore
+                    line=int(self.line),  # type-ignore
                     need=need.strip(),
                     verify_type=verify_type,
                     result=self.result,

src/extensions/score_source_code_linker/testlink.py

@@ -68,12 +68,21 @@ def _write_test_xml(
 
 
 @pytest.fixture
-def tmp_xml_dirs(tmp_path: Path) -> Callable[..., tuple[Path, Path, Path]]:
-    def _tmp_xml_dirs(test_folder: str = "bazel-testlogs") -> tuple[Path, Path, Path]:
+def tmp_xml_dirs(
+    tmp_path: Path,
+) -> Callable[..., tuple[Path, Path, Path, Path, Path]]:
+    def _tmp_xml_dirs(
+        test_folder: str = "bazel-testlogs",
+    ) -> tuple[Path, Path, Path, Path, Path]:
         root = tmp_path / test_folder
-        dir1, dir2 = root / "with_props", root / "no_props"
+        dir1, dir2, dir3, dir4 = (
+            root / "with_props",
+            root / "no_props",
+            root / "with_extra_props",
+            root / "missing_props",
+        )
 
-        for d in (dir1, dir2):
+        for d in (dir1, dir2, dir3, dir4):
             d.mkdir(parents=True, exist_ok=True)
 
         # File with properties
@@ -95,7 +104,42 @@ def _tmp_xml_dirs(test_folder: str = "bazel-testlogs") -> tuple[Path, Path, Path
         # File without properties
         _write_test_xml(dir2 / "test.xml", name="tc_no_props", file="path2", line=20)
 
-        return root, dir1, dir2
+        # File with some properties that we don't care about
+        _write_test_xml(
+            dir3 / "test.xml",
+            name="tc_with_extra_props",
+            result="failed",
+            file="path1",
+            line=10,
+            props={
+                # Properties we do not parse should not throw an error
+                "PartiallyVerifies": "REQ1",
+                "FullyVerifies": "",
+                "TestType": "type",
+                "DerivationTechnique": "tech",
+                "Description": "desc",
+                "ASIL": "B",
+                "important": "yes",
+            },
+        )
+
+        # File with some properties missing
+        _write_test_xml(
+            dir4 / "test.xml",
+            name="tc_with_missing_props",
+            result="failed",
+            file="path1",
+            line=10,
+            props={
+                # derivation_technique and test_type are missing
+                # This should not make a 'valid' testlink
+                "PartiallyVerifies": "REQ1",
+                "FullyVerifies": "",
+                "Description": "desc",
+            },
+        )
+
+        return root, dir1, dir2, dir3, dir4
 
     return _tmp_xml_dirs
 
@@ -105,48 +149,62 @@ def _tmp_xml_dirs(test_folder: str = "bazel-testlogs") -> tuple[Path, Path, Path
     test_type="requirements-based",
     derivation_technique="requirements-analysis",
 )
-def test_find_xml_files(tmp_xml_dirs: Callable[..., tuple[Path, Path, Path]]):
+def test_find_xml_files(
+    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path, Path, Path]],
+):
     """Ensure xml files are found as expected if bazel-testlogs is used"""
     root: Path
     dir1: Path
     dir2: Path
-    root, dir1, dir2 = tmp_xml_dirs()
+    root, dir1, dir2, dir3, dir4 = tmp_xml_dirs()
     found = xml_parser.find_xml_files(root)
-    expected: set[Path] = {dir1 / "test.xml", dir2 / "test.xml"}
+    expected: set[Path] = {
+        dir1 / "test.xml",
+        dir2 / "test.xml",
+        dir3 / "test.xml",
+        dir4 / "test.xml",
+    }
     assert set(found) == expected
 
 
-def test_find_xml_folder(tmp_xml_dirs: Callable[..., tuple[Path, Path, Path]]):
+def test_find_xml_folder(
+    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path, Path, Path]],
+):
     """Ensure xml files are found as expected if bazel-testlogs is used"""
     root: Path
-    root, _, _ = tmp_xml_dirs()
+    root, _, _, _, _ = tmp_xml_dirs()
     found = xml_parser.find_test_folder(base_path=root.parent)
     assert found is not None
     assert found == root
 
 
 def test_find_xml_folder_test_reports(
-    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path]],
+    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path, Path, Path]],
 ):
     # root is the 'tests-report' folder inside tmp_path
-    root, _, _ = tmp_xml_dirs(test_folder="tests-report")
+    root, _, _, _, _ = tmp_xml_dirs(test_folder="tests-report")
     # We pass the PARENT of 'tests-report' as the workspace root
     found = xml_parser.find_test_folder(base_path=root.parent)
     assert found is not None
     assert found == root
 
 
 def test_find_xml_files_test_reports(
-    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path]],
+    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path, Path, Path]],
 ):
     """Ensure xml files are found as expected if tests-report is used"""
     root: Path
     dir1: Path
     dir2: Path
-    root, dir1, dir2 = tmp_xml_dirs(test_folder="tests-report")
+    root, dir1, dir2, dir3, dir4 = tmp_xml_dirs(test_folder="tests-report")
     found = xml_parser.find_xml_files(dir=root)
     assert found is not None
-    expected: set[Path] = {root / dir1 / "test.xml", root / dir2 / "test.xml"}
+    expected: set[Path] = {
+        root / dir1 / "test.xml",
+        root / dir2 / "test.xml",
+        root / dir3 / "test.xml",
+        root / dir4 / "test.xml",
+    }
     assert set(found) == expected
 
 
@@ -204,23 +262,42 @@ def test_parse_properties():
     test_type="requirements-based",
     derivation_technique="requirements-analysis",
 )
-def test_read_test_xml_file(tmp_xml_dirs: Callable[..., tuple[Path, Path, Path]]):
+def test_read_test_xml_file(
+    tmp_xml_dirs: Callable[..., tuple[Path, Path, Path, Path, Path]],
+):
     """Ensure a whole pre-defined xml file is parsed correctly"""
     _: Path
     dir1: Path
     dir2: Path
-    _, dir1, dir2 = tmp_xml_dirs()
-
-    needs1, no_props1 = xml_parser.read_test_xml_file(dir1 / "test.xml")
+    _, dir1, dir2, dir3, dir4 = tmp_xml_dirs()
+    needs1, no_props1, missing_props1 = xml_parser.read_test_xml_file(dir1 / "test.xml")
+    # Should parse the properties and create a 'valid' testlink
     assert isinstance(needs1, list) and len(needs1) == 1
     tcneed = needs1[0]
     assert isinstance(tcneed, DataOfTestCase)
     assert tcneed.result == "failed"
     assert no_props1 == []
+    assert missing_props1 == []
 
-    needs2, no_props2 = xml_parser.read_test_xml_file(dir2 / "test.xml")
+    # No properties at all => Should not be a 'valid' testlink
+    needs2, no_props2, missing_props2 = xml_parser.read_test_xml_file(dir2 / "test.xml")
     assert needs2 == []
     assert no_props2 == ["tc_no_props"]
+    assert missing_props2 == []
+
+    # Extra Properties => Should not cause an error
+    needs3, no_props3, missing_props3 = xml_parser.read_test_xml_file(dir3 / "test.xml")
+    assert isinstance(needs1, list) and len(needs1) == 1
+    tcneed3 = needs3[0]
+    assert isinstance(tcneed3, DataOfTestCase)
+    assert no_props3 == []
+    assert missing_props3 == []
+
+    # Missing some properties => Should not be a 'valid' testlink
+    needs4, no_props4, missing_props4 = xml_parser.read_test_xml_file(dir4 / "test.xml")
+    assert needs4 == []
+    assert no_props4 == []
+    assert missing_props4 == ["tc_with_missing_props"]
 
 
 @add_test_properties(