diff --git a/src/openeo_test_suite/tests/processes/processing/test_example.py b/src/openeo_test_suite/tests/processes/processing/test_example.py index 3865029..53044a7 100644 --- a/src/openeo_test_suite/tests/processes/processing/test_example.py +++ b/src/openeo_test_suite/tests/processes/processing/test_example.py @@ -1,19 +1,29 @@ +# TODO rename this module (`test_example.py` was originally just meant as an example) + +import logging import math -import warnings from pathlib import Path +from typing import List, Tuple, Union import json5 import pytest -import xarray as xr from deepdiff import DeepDiff +import openeo_test_suite +from openeo_test_suite.lib.process_runner.base import ProcessTestRunner from openeo_test_suite.lib.process_runner.util import isostr_to_datetime -# glob path to the test files -examples_path = "assets/processes/tests/*.json5" +_log = logging.getLogger(__name__) + + +DEFAULT_EXAMPLES_ROOT = ( + Path(openeo_test_suite.__file__).parents[2] / "assets/processes/tests" +) -def get_prop(prop, data, test, default=None): +def _get_prop(prop: str, data: dict, test: dict, *, default=None): + """Get property from example data, first trying test data, then full process data, then general fallback value.""" + # TODO make this function more generic (e.g. `multidict_get(key, *dicts, default=None)`) if prop in test: level = test[prop] elif prop in data: @@ -23,32 +33,37 @@ def get_prop(prop, data, test, default=None): return level -def get_examples(): +def get_examples( + root: Union[str, Path] = DEFAULT_EXAMPLES_ROOT +) -> List[Tuple[str, dict, Path, str, bool]]: + """Collect process examples/tests from examples root folder containing JSON5 files.""" + # TODO return a list of NamedTuples? examples = [] - package_root_folder = Path(__file__).parents[5] - files = package_root_folder.glob(examples_path) - for file in files: - id = file.stem + # TODO: it's not recommended use `file` (a built-in) as variable name. `path` would be better. + for file in root.glob("*.json5"): + process_id = file.stem try: with file.open() as f: data = json5.load(f) - for test in data["tests"]: - level = get_prop("level", data, test, "L4") - experimental = get_prop("experimental", data, test, False) - examples.append([id, test, file, level, experimental]) + for test in data["tests"]: + level = _get_prop("level", data, test, default="L4") + experimental = _get_prop("experimental", data, test, default=False) + examples.append((process_id, test, file, level, experimental)) except Exception as e: - warnings.warn("Failed to load {} due to {}".format(file, e)) + _log.warning(f"Failed to load {file}: {e}") return examples -@pytest.mark.parametrize("id,example,file,level, experimental", get_examples()) +@pytest.mark.parametrize( + ["process_id", "example", "file", "level", "experimental"], get_examples() +) def test_process( connection, skip_experimental, process_levels, processes, - id, + process_id, example, file, level, @@ -56,13 +71,13 @@ def test_process( skipper, ): skipper.skip_if_unmatching_process_level(level) - if len(processes) > 0 and id not in processes: + if len(processes) > 0 and process_id not in processes: pytest.skip( - "Skipping process {id!r} because it is not in the specified processes" + f"Skipping process {process_id!r} because it is not in the specified processes" ) # check whether the process is available - skipper.skip_if_unsupported_process([id]) + skipper.skip_if_unsupported_process([process_id]) if skip_experimental and experimental: pytest.skip("Skipping experimental process {}".format(id)) @@ -74,22 +89,29 @@ def test_process( # prepare the arguments from test JSON encoding to internal backend representations # or skip if not supported by the test runner try: - arguments = prepare_arguments(example["arguments"], id, connection, file) + arguments = _prepare_arguments( + arguments=example["arguments"], + process_id=process_id, + connection=connection, + file=file, + ) except Exception as e: - # TODO: skipping on a generic `Exception` is very liberal and might hide real issues + # TODO: this `except: pytest.skip()` is overly liberal, possibly hiding real issues. + # On what precise conditions should we skip? e.g. NotImplementedError? pytest.skip(str(e)) - throws = bool(example["throws"]) if "throws" in example else False + throws = bool(example.get("throws")) returns = "returns" in example # execute the process try: - result = connection.execute(id, arguments) + result = connection.execute(process_id, arguments) except Exception as e: result = e # check the process results / behavior if throws and returns: + # TODO what does it mean if test can both throw and return? if isinstance(result, Exception): check_exception(example, result) else: @@ -102,25 +124,31 @@ def test_process( # TODO: skipping at this point of test is a bit useless. # Instead: skip earlier, or just consider the test as passed? pytest.skip( - "Test for process {} doesn't provide an expected result for arguments: {}".format( - id, example["arguments"] - ) + f"Test for process {process_id} doesn't provide an expected result for arguments: {example['arguments']}" ) -def prepare_arguments(arguments, process_id, connection, file): - for name in arguments: - arguments[name] = prepare_argument( - arguments[name], process_id, name, connection, file +def _prepare_arguments( + arguments: dict, process_id: str, connection: ProcessTestRunner, file: Path +) -> dict: + return { + k: _prepare_argument( + arg=v, process_id=process_id, name=k, connection=connection, file=file ) + for k, v in arguments.items() + } - return arguments - -def prepare_argument(arg, process_id, name, connection, file): +def _prepare_argument( + arg: Union[dict, str, int, float], + process_id: str, + name: str, + connection: ProcessTestRunner, + file: Path, +): # handle external references to files if isinstance(arg, dict) and "$ref" in arg: - arg = load_ref(arg["$ref"], file) + arg = _load_ref(arg["$ref"], file) # handle custom types of data if isinstance(arg, dict): @@ -134,17 +162,36 @@ def prepare_argument(arg, process_id, name, connection, file): # nodata-values elif arg["type"] == "nodata": arg = connection.get_nodata_value() + else: + # TODO: raise NotImplementedError? + _log.warning(f"Unhandled argument type: {arg}") elif "process_graph" in arg: - arg = connection.encode_process_graph(arg, process_id, name) + arg = connection.encode_process_graph( + process=arg, parent_process_id=process_id, parent_parameter=name + ) else: - for key in arg: - arg[key] = prepare_argument( - arg[key], process_id, name, connection, file + arg = { + k: _prepare_argument( + arg=v, + process_id=process_id, + name=name, + connection=connection, + file=file, ) + for k, v in arg.items() + } elif isinstance(arg, list): - for i in range(len(arg)): - arg[i] = prepare_argument(arg[i], process_id, name, connection, file) + arg = [ + _prepare_argument( + arg=a, + process_id=process_id, + name=name, + connection=connection, + file=file, + ) + for a in arg + ] arg = connection.encode_data(arg) @@ -154,46 +201,58 @@ def prepare_argument(arg, process_id, name, connection, file): return arg -def prepare_results(connection, file, example, result=None): +def _prepare_results(connection: ProcessTestRunner, file: Path, example, result=None): # go through the example and result recursively and convert datetimes to iso strings # could be used for more conversions in the future... if isinstance(example, dict): # handle external references to files if isinstance(example, dict) and "$ref" in example: - example = load_ref(example["$ref"], file) + example = _load_ref(example["$ref"], file) if "type" in example: if example["type"] == "datetime": example = isostr_to_datetime(example["value"]) try: result = isostr_to_datetime(result) - except: + except Exception: pass elif example["type"] == "nodata": example = connection.get_nodata_value() else: + # TODO: avoid in-place dict mutation for key in example: if key not in result: - (example[key], _) = prepare_results(connection, file, example[key]) + (example[key], _) = _prepare_results( + connection=connection, file=file, example=example[key] + ) else: - (example[key], result[key]) = prepare_results( - connection, file, example[key], result[key] + (example[key], result[key]) = _prepare_results( + connection=connection, + file=file, + example=example[key], + result=result[key], ) elif isinstance(example, list): + # TODO: avoid in-place list mutation for i in range(len(example)): if i >= len(result): - (example[i], _) = prepare_results(connection, file, example[i]) + (example[i], _) = _prepare_results( + connection=connection, file=file, example=example[i] + ) else: - (example[i], result[i]) = prepare_results( - connection, file, example[i], result[i] + (example[i], result[i]) = _prepare_results( + connection=connection, + file=file, + example=example[i], + result=result[i], ) return (example, result) -def load_ref(ref, file): +def _load_ref(ref: str, file: Path): try: path = file.parent / ref if ref.endswith(".json") or ref.endswith(".json5") or ref.endswith(".geojson"): @@ -205,57 +264,52 @@ def load_ref(ref, file): else: raise NotImplementedError(f"Unhandled external reference {ref}.") except Exception as e: + # TODO: is this try-except actually useful? raise RuntimeError(f"Failed to load external reference {ref}") from e def check_non_json_values(value): + # TODO: shouldn't this check be an aspect of Http(ProcessTestRunner)? if isinstance(value, float): if math.isnan(value): raise ValueError("HTTP JSON APIs don't support NaN values") elif math.isinf(value): raise ValueError("HTTP JSON APIs don't support infinity values") elif isinstance(value, dict): - for key in value: - check_non_json_values(value[key]) + for v in value.values(): + check_non_json_values(v) elif isinstance(value, list): for item in value: check_non_json_values(item) def check_exception(example, result): - assert isinstance(result, Exception), "Excpected an exception, but got {}".format( - result - ) + assert isinstance(result, Exception), f"Expected an exception, but got {result}" if isinstance(example["throws"], str): if result.__class__.__name__ != example["throws"]: - warnings.warn( - "Expected exception {} but got {}".format( - example["throws"], result.__class__.__name__ - ) + # TODO: better way to report this warning? + _log.warning( + f"Expected exception {example['throws']} but got {result.__class__}" ) # todo: we should enable this end remove the two lines above, but right now tooling doesn't really implement this # assert result.__class__.__name__ == example["throws"] def check_return_value(example, result, connection, file): - assert not isinstance(result, Exception), "Unexpected exception: {} ".format( - str(result) - ) + assert not isinstance(result, Exception), f"Unexpected exception: {result}" # handle custom types of data result = connection.decode_data(result, example["returns"]) # decode special types (currently mostly datetimes and nodata) - (example["returns"], result) = prepare_results( - connection, file, example["returns"], result + (example["returns"], result) = _prepare_results( + connection=connection, file=file, example=example["returns"], result=result ) - delta = example["delta"] if "delta" in example else 0.0000000001 + delta = example.get("delta", 0.0000000001) if isinstance(example["returns"], dict): - assert isinstance(result, dict), "Expected a dict but got {}".format( - type(result) - ) + assert isinstance(result, dict), f"Expected a dict but got {type(result)}" exclude_regex_paths = [] exclude_paths = [] ignore_order_func = None @@ -280,11 +334,9 @@ def check_return_value(example, result, connection, file): exclude_regex_paths=exclude_regex_paths, ignore_order_func=ignore_order_func, ) - assert {} == diff, "Differences: {}".format(str(diff)) + assert {} == diff, f"Differences: {diff!s}" elif isinstance(example["returns"], list): - assert isinstance(result, list), "Expected a list but got {}".format( - type(result) - ) + assert isinstance(result, list), f"Expected a list but got {type(result)}" diff = DeepDiff( example["returns"], result, @@ -292,17 +344,15 @@ def check_return_value(example, result, connection, file): ignore_numeric_type_changes=True, ignore_nan_inequality=True, ) - assert {} == diff, "Differences: {}".format(str(diff)) + assert {} == diff, f"Differences: {diff!s}" elif isinstance(example["returns"], float) and math.isnan(example["returns"]): - assert math.isnan(result), "Got {} instead of NaN".format(result) + assert isinstance(result, float) and math.isnan(result), f"Got {result} instead of NaN" elif isinstance(example["returns"], float) or isinstance(example["returns"], int): - msg = "Expected a numerical result but got {} of type {}".format( - result, type(result) - ) + msg = f"Expected a numerical result but got {result} of type {type(result)}" assert isinstance(result, float) or isinstance(result, int), msg assert not math.isnan(result), "Got unexpected NaN as result" # handle numerical data with a delta - assert result == pytest.approx(example["returns"], delta) + assert result == pytest.approx(example["returns"], rel=delta) else: - msg = "Expected {} but got {}".format(example["returns"], result) + msg = f"Expected {example['returns']} but got {result}" assert result == example["returns"], msg