From bc61abb51469a687e071384d2a2ed614f6ddf238 Mon Sep 17 00:00:00 2001 From: MaximilianSoerenPollak Date: Wed, 25 Feb 2026 21:15:10 +0100 Subject: [PATCH 1/3] Allow extra properties & missing properties in testcases --- .../score_source_code_linker/testlink.py | 100 +++++++++++---- .../tests/test_xml_parser.py | 117 +++++++++++++++--- .../score_source_code_linker/xml_parser.py | 38 ++++-- 3 files changed, 200 insertions(+), 55 deletions(-) diff --git a/src/extensions/score_source_code_linker/testlink.py b/src/extensions/score_source_code_linker/testlink.py index 51311f3e..ee83c7f9 100644 --- a/src/extensions/score_source_code_linker/testlink.py +++ b/src/extensions/score_source_code_linker/testlink.py @@ -79,14 +79,14 @@ 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 @@ -94,13 +94,13 @@ class DataOfTestCase: @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,15 +122,8 @@ 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( @@ -138,8 +131,54 @@ def __post_init__(self): # "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, diff --git a/src/extensions/score_source_code_linker/tests/test_xml_parser.py b/src/extensions/score_source_code_linker/tests/test_xml_parser.py index adc0d6b9..95b445dd 100644 --- a/src/extensions/score_source_code_linker/tests/test_xml_parser.py +++ b/src/extensions/score_source_code_linker/tests/test_xml_parser.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,31 +149,40 @@ 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 @@ -137,16 +190,21 @@ def test_find_xml_folder_test_reports( 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( diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index 93456971..d91358bd 100644 --- a/src/extensions/score_source_code_linker/xml_parser.py +++ b/src/extensions/score_source_code_linker/xml_parser.py @@ -87,7 +87,7 @@ def parse_properties(case_properties: dict[str, Any], properties: Element): return case_properties -def read_test_xml_file(file: Path) -> tuple[list[DataOfTestCase], list[str]]: +def read_test_xml_file(file: Path) -> tuple[list[DataOfTestCase], list[str], list[str]]: """ Reading & parsing the test.xml files into TestCaseNeeds @@ -98,6 +98,7 @@ def read_test_xml_file(file: Path) -> tuple[list[DataOfTestCase], list[str]]: """ test_case_needs: list[DataOfTestCase] = [] non_prop_tests: list[str] = [] + missing_prop_tests: list[str] = [] tree = ET.parse(file) root = tree.getroot() @@ -155,9 +156,17 @@ def read_test_xml_file(file: Path) -> tuple[list[DataOfTestCase], list[str]]: # "and either 'PartiallyVerifies' or 'FullyVerifies' are mandatory." # ) + # TODO: There is a better way here to check this i think. + # I think it should be possible to save the 'from_dict' operation + # If the is_valid method would return 'False' anyway. + # I just can't think of it right now, leaving this for future me case_properties = parse_properties(case_properties, properties_element) - test_case_needs.append(DataOfTestCase.from_dict(case_properties)) - return test_case_needs, non_prop_tests + test_case = DataOfTestCase.from_dict(case_properties) + if not test_case.is_valid(): + missing_prop_tests.append(testname) + continue + test_case_needs.append(test_case) + return test_case_needs, non_prop_tests, missing_prop_tests def find_xml_files(dir: Path) -> list[Path]: @@ -213,7 +222,7 @@ def run_xml_parser(app: Sphinx, env: BuildEnvironment): def build_test_needs_from_files( - app: Sphinx, env: BuildEnvironment, xml_paths: list[Path] + app: Sphinx, enw_: BuildEnvironment, xml_paths: list[Path] ) -> list[DataOfTestCase]: """ Reading in all test.xml files, and building 'testcase' external need objects out of @@ -223,14 +232,15 @@ def build_test_needs_from_files( - list[TestCaseNeed] """ tcns: list[DataOfTestCase] = [] - for f in xml_paths: - b, z = read_test_xml_file(f) - non_prop_tests = ", ".join(n for n in z) + for file in xml_paths: + # Last value can be ignored. The 'is_valid' function already prints infos + test_cases, tests_missing_all_props,_ = read_test_xml_file(file) + non_prop_tests = ", ".join(n for n in tests_missing_all_props) if non_prop_tests: - logger.info(f"Tests missing properties: {non_prop_tests}") - tcns.extend(b) - for c in b: - construct_and_add_need(app, c) + logger.info(f"Tests missing all properties: {non_prop_tests}") + tcns.extend(test_cases) + for tc in test_cases: + construct_and_add_need(app, tc) return tcns @@ -246,6 +256,12 @@ def short_hash(input_str: str, length: int = 5) -> str: def construct_and_add_need(app: Sphinx, tn: DataOfTestCase): + # Asserting worldview to a peace Language Server + # And ensure non crashing due to non string concatenation + # Everything but 'result_text', + # and either 'Fully' or 'PartiallyVerifies' should not be None here + assert tn.file is not None + assert tn.name is not None # IDK if this is ideal or not with contextlib.suppress(BaseException): _ = add_external_need( From 976e9b2cca6ccfef9d65975e7a910acaf7353150 Mon Sep 17 00:00:00 2001 From: MaximilianSoerenPollak Date: Wed, 25 Feb 2026 21:23:18 +0100 Subject: [PATCH 2/3] Add updated Documentation --- docs/how-to/test_to_doc_links.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/how-to/test_to_doc_links.rst b/docs/how-to/test_to_doc_links.rst index 82dd7904..4453d07f 100644 --- a/docs/how-to/test_to_doc_links.rst +++ b/docs/how-to/test_to_doc_links.rst @@ -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. From 25c49dd77706c32b9efcf1cc27e10b9f02968ccd Mon Sep 17 00:00:00 2001 From: MaximilianSoerenPollak Date: Fri, 27 Feb 2026 09:49:09 +0100 Subject: [PATCH 3/3] Delete formating workflow The pre-commit already takes care of everything the formating workflow has tested. Therefore this can be deleted. --- .github/workflows/format.yml | 41 ------------------------------------ 1 file changed, 41 deletions(-) delete mode 100644 .github/workflows/format.yml diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml deleted file mode 100644 index 05664bac..00000000 --- a/.github/workflows/format.yml +++ /dev/null @@ -1,41 +0,0 @@ -# ******************************************************************************* -# Copyright (c) 2025 Contributors to the Eclipse Foundation -# -# See the NOTICE file(s) distributed with this work for additional -# information regarding copyright ownership. -# -# This program and the accompanying materials are made available under the -# terms of the Apache License Version 2.0 which is available at -# https://www.apache.org/licenses/LICENSE-2.0 -# -# SPDX-License-Identifier: Apache-2.0 -# ******************************************************************************* - -name: Formatting checks -on: - pull_request: - types: [opened, reopened, synchronize] - merge_group: - types: [checks_requested] -jobs: - formatting-check: - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v4.2.2 - - name: Cache Bazel - uses: actions/cache@v4 - with: - path: ~/.cache/bazel - key: ${{ runner.os }}-format-${{ hashFiles('**/*.bazel', '**/BUILD', '**/*.bzl') }} - - - name: Setup Bazel with cache - uses: bazel-contrib/setup-bazel@0.15.0 - with: - disk-cache: true - repository-cache: true - bazelisk-cache: true - - name: Run formatting checks - run: | - bazel run //:ide_support - bazel test //src:format.check