Feature: WP3 Process metadata validation

.gitignore

@@ -169,3 +169,7 @@ poetry.toml
 
 # LSP config files
 pyrightconfig.json
+
+# vscode
+.vscode/
+pytest.ini

README.md

@@ -40,7 +40,22 @@ focussing on a specific API aspect to test or verify
     ```
 - **WP3 Validation of process metadata** (lead implementation partner: EODC)
   - Main location: [`src/openeo_test_suite/tests/processes/metadata`](./src/openeo_test_suite/tests/processes/metadata)
-  - TODO: [Open-EO/openeo-test-suite#19](https://github.com/Open-EO/openeo-test-suite/issues/19)
+  - Defines tests to validate openEO process metadata against specs
+    defined in the [openeo-processes](https://github.com/Open-EO/openeo-processes) project
+  - Functional tests concern actual values and behavior of processes (like parameters and return values),
+    failures in these tests should be looked into and fixed.
+  - Non-functional tests concern descriptions and other metadata of processes that have no impact on the actual behavior of the process,
+    failures in these tests should be taken as warnings, but don't necessarily need to be fixed.
+  - Usage example for running these tests against a desired openEO backend URL:
+    ```bash
+    pytest src/openeo_test_suite/tests/processes/metadata \
+      --html=reports/process-metadata.html \
+      --openeo-backend-url=openeo.example \
+      --tb=no \
+      -vv
+    ```
+    It is recommended to run these tests with the `--tb=no` option to avoid excessive output of no substance.
+    and the `-vv` option to get more detailed output.
 - **WP4 General openEO API compliance validation** (lead implementation partner: EODC)
   - TODO: [Open-EO/openeo-test-suite#20](https://github.com/Open-EO/openeo-test-suite/issues/20)
 - **WP5 Individual process testing** (lead implementation partner: M. Mohr)

src/openeo_test_suite/lib/internal-tests/test_process_registry.py

@@ -19,7 +19,7 @@ def test_get_process_match(self, process_registry):
         assert process_data.process_id == "add"
         assert process_data.level == "L1"
         assert process_data.experimental is False
-        assert process_data.path.name == "add.json5"
+        assert process_data.path.name + "5" == "add.json5"
         assert len(process_data.tests)
 
     def test_get_process_no_match(self, process_registry):
@@ -37,7 +37,7 @@ def test_get_all_processes_add(self, process_registry):
 
         assert add.level == "L1"
         assert add.experimental is False
-        assert add.path.name == "add.json5"
+        assert add.path.name + "5" == "add.json5"
         assert len(add.tests)
 
         add00 = {"arguments": {"x": 0, "y": 0}, "returns": 0}
@@ -50,7 +50,7 @@ def test_get_all_processes_divide(self, process_registry):
 
         assert divide.level == "L1"
         assert divide.experimental is False
-        assert divide.path.name == "divide.json5"
+        assert divide.path.name + "5" == "divide.json5"
         assert len(divide.tests)
 
         divide0 = {

src/openeo_test_suite/lib/process_registry.py

@@ -1,3 +1,5 @@
+import itertools
+import json
 import logging
 import os
 from dataclasses import dataclass
@@ -16,6 +18,7 @@ class ProcessData:
     """Process data, including profile level and list of tests"""
 
     process_id: str
+    metadata: dict
     level: str
     tests: List[dict]  # TODO: also make dataclass for each test?
     experimental: bool
@@ -32,22 +35,23 @@ def __init__(self, root: Optional[Path] = None):
         """
         :param root: Root directory of the tests folder in  openeo-processes project
         """
-        self._root = Path(
-            root
-            # TODO: eliminate need for this env var?
+
+        self._root = Path(root or self._guess_root())
+        self._root_json5 = Path(
+            self._root.joinpath("tests")
             or os.environ.get("OPENEO_TEST_SUITE_PROCESSES_TEST_ROOT")
-            or self._guess_root()
         )
+
         # Lazy load cache
         self._processes: Union[None, List[ProcessData]] = None
 
     def _guess_root(self):
         # TODO: avoid need for guessing and properly include assets in (installed) package
         project_root = Path(openeo_test_suite.__file__).parents[2]
         candidates = [
-            project_root / "assets/processes/tests",
-            Path("./assets/processes/tests"),
-            Path("./openeo-test-suite/assets/processes/tests"),
+            project_root / "assets/processes",
+            Path("./assets/processes"),
+            Path("./openeo-test-suite/assets/processes"),
         ]
         for candidate in candidates:
             if candidate.exists() and candidate.is_dir():
@@ -62,18 +66,31 @@ def _load(self) -> Iterator[ProcessData]:
         if not self._root.is_dir():
             raise ValueError(f"Invalid process test root directory: {self._root}")
         _log.info(f"Loading process definitions from {self._root}")
-        for path in self._root.glob("*.json5"):
+
+        process_paths = itertools.chain(
+            self._root.glob("*.json"), self._root.glob("proposals/*.json")
+        )
+        for path in process_paths:
+            test_metadata_path = self._root_json5 / f"{path.stem}.json5"
             try:
                 with path.open() as f:
-                    data = json5.load(f)
+                    data = json.load(f)
                 if data["id"] != path.stem:
                     raise ValueError(
                         f"Process id mismatch between id {data['id']!r} and filename {path.name!r}"
                     )
+                # Metadata is stored in sibling json file
+                if test_metadata_path.exists():
+                    with test_metadata_path.open() as f:
+                        metadata = json5.load(f)
+                else:
+                    metadata = {}
+
                 yield ProcessData(
                     process_id=data["id"],
-                    level=data.get("level"),
-                    tests=data.get("tests", []),
+                    metadata=data,
+                    level=metadata.get("level", "L4"),
+                    tests=metadata.get("tests", []),
                     experimental=data.get("experimental", False),
                     path=path,
                 )

src/openeo_test_suite/tests/processes/metadata/test_process_metadata.py

@@ -0,0 +1,163 @@
+import json
+import pytest
+import requests
+from openeo_test_suite.lib.process_registry import ProcessData
+from openeo_test_suite.lib.process_selection import get_selected_processes
+from openeo_test_suite.lib.backend_under_test import (
+    get_backend_under_test,
+)
+import warnings
+
+
+@pytest.fixture(scope="module")
+def api_processes():
+    endpoint_path = "processes"
+    base_url = get_backend_under_test().connection.root_url
+    if not base_url:
+        raise ValueError("No backend URL configured")
+    if base_url.endswith("/"):
+        base_url = base_url[:-1]
+    full_endpoint_url = f"{base_url}/{endpoint_path}"
+    response = requests.get(full_endpoint_url)
+    if response.status_code != 200:
+        raise ValueError(
+            f"Failed to get processes from {full_endpoint_url}: {response.content}"
+        )
+    return json.loads(response.content)["processes"]
+
+
+def _get_test_id(val):
+    if isinstance(val, ProcessData):
+        return val.process_id
+
+
+@pytest.mark.parametrize(
+    "expected_process",
+    [p for p in get_selected_processes() if p.metadata],
+    ids=_get_test_id,
+)
+def test_process_metadata_functional(api_processes, expected_process, skipper):
+    """
+    Tests if the metadata of processes are correct, first tests if the process exists,
+    then tests if the parameters of processes are correct and finally tests if the return type of processes is correct.
+
+    Any process that has no metadata is skipped.
+
+    These are the functional parts of the process metadata e.g.: the parameters and return type.
+    """
+
+    skipper.skip_if_unsupported_process(expected_process.process_id)
+
+    actual_process = [
+        process
+        for process in api_processes
+        if process["id"] == expected_process.process_id
+    ][0]
+    # Tests if the parameters of processes are correct
+
+    expected_parameters = expected_process.metadata.get("parameters", [])
+    actual_parameters = actual_process["parameters"]
+
+    if len(expected_parameters) > len(actual_parameters):
+        warnings.warn(
+            f"Process {expected_process.process_id} has {len(expected_parameters)} expected parameters, but only {len(actual_parameters)} actual parameters"
+        )
+        for expected_parameter in expected_parameters[len(actual_parameters) :]:
+            assert expected_parameter.get("default", None) != None
+
+    if len(expected_parameters) < len(actual_parameters):
+        warnings.warn(
+            f"Process {expected_process.process_id} has {len(expected_parameters)} expected parameters, but {len(actual_parameters)} actual parameters"
+        )
+        for actual_parameter in actual_parameters[len(expected_parameters) :]:
+            assert actual_parameter.get("default", None) != None
+
+    # Check if the expected parameters are in the actual parameters
+    # throw a warning if there are added parameters with default values
+
+    for expected_parameter, actual_parameter in zip(
+        expected_parameters, actual_parameters
+    ):
+        # Tests if parameter names are equivalent
+        assert expected_parameter["name"] == actual_parameter["name"]
+        # Tests if optionality of parameters is equivalent
+        assert expected_parameter.get("optional", False) == actual_parameter.get(
+            "optional", False
+        )
+        # Tests if the type of parameters is equivalent
+        assert expected_parameter["schema"] == actual_parameter["schema"]
+
+    # Tests if the return type of processes is correct
+    expected_return_type = expected_process.metadata.get("returns", {})
+
+    actual_return_type = actual_process["returns"]
+    assert expected_return_type["schema"] == actual_return_type["schema"]
+
+    # Tests the deprecated and experimental flags (Should be true if expected process is true as well, but not the other way around)
+    if expected_process.metadata.get("experimental", False):
+        assert actual_process.get("experimental", False)
+
+    if expected_process.metadata.get("deprecated", False):
+        assert actual_process.get("deprecated", False)
+
+
+@pytest.mark.parametrize(
+    "expected_process",
+    [p for p in get_selected_processes() if p.metadata],
+    ids=_get_test_id,
+)
+def test_process_metadata_non_functional(api_processes, expected_process, skipper):
+    """
+    Tests if the non-functional metadata of processes are correct (descriptions and categories), first tests if the process exists,
+    then tests if the categories of processes are correct.
+    """
+
+    skipper.skip_if_unsupported_process(expected_process.process_id)
+
+    actual_process = [
+        process
+        for process in api_processes
+        if process["id"] == expected_process.process_id
+    ][0]
+
+    # Tests if the categories of processes is equivalent
+    assert (
+        expected_process.metadata.get("categories", []) == actual_process["categories"]
+    )
+
+    # Tests if the description of processes is equivalent
+    assert (
+        expected_process.metadata.get("description", "")
+        == actual_process["description"]
+    )
+
+    # Tests if the summary of processes is equivalent
+    assert expected_process.metadata.get("summary", "") == actual_process["summary"]
+
+    # Tests if the description of parameters is equivalent
+    expected_parameters = expected_process.metadata.get("parameters", [])
+    actual_parameters = actual_process["parameters"]
+
+    assert len(expected_parameters) == len(actual_parameters)
+
+    for expected_parameter, actual_parameter in zip(
+        expected_parameters, actual_parameters
+    ):
+        assert expected_parameter.get("description", "") == actual_parameter.get(
+            "description", ""
+        )
+
+    # Tests if the description of returns is equivalent
+    expected_return_type = expected_process.metadata.get("returns", {})
+    actual_return_type = actual_process["returns"]
+    assert expected_return_type.get("description", "") == actual_return_type.get(
+        "description", ""
+    )
+
+    # Tests if the links of processes are equivalent
+    expected_links = expected_process.metadata.get("links", [])
+    actual_links = actual_process["links"]
+    for expected_link, actual_link in zip(expected_links, actual_links):
+        assert expected_link.get("href", "") == actual_link.get("href", "")
+        assert expected_link.get("rel", "") == actual_link.get("rel", "")
+        assert expected_link.get("title", "") == actual_link.get("title", "")