Fix three bugs in the component tests

packages/python/openproblems/CHANGELOG.md

@@ -44,6 +44,18 @@
 * `render_file_format`: Render file formats of type `tabular`. `read_file_format` accepts them,
   but the renderer only knew about `csv`, `tsv` and `parquet`, so the Format and Data structure
   sections came out empty.
+
+* `check_config`: Check the required links even when a component defines no links at all.
+  `check_links` returned early on an empty `links`, so a method without any links passed,
+  while a method with only a `documentation` link was told that `.links.repository` is missing.
+
+* `check_config`: Give `check_url` a 30 second timeout, and report a request that fails outright
+  as an unreachable link rather than letting the exception escape.
+
+* `check_config`: Escape the dot in the DOI regex, which also matched a prefix like `10X1038`.
+
+* `run_and_check_output`: Skip the format validation of an output file argument that has no
+  value. An optional output without a default or example crashed with a `KeyError: 'value'`.
 
 # openproblems core Python v0.1.1
 

packages/python/openproblems/pyproject.toml

@@ -33,7 +33,9 @@ dependencies = [
 
 [project.optional-dependencies]
 test = [
-  "pytest>=8.0"
+  "pytest>=8.0",
+  # imported inside check_url(), so not a runtime dependency of the package
+  "requests"
 ]
 
 [project.urls]

packages/python/openproblems/src/openproblems/project/component_tests/check_config.py

@@ -9,12 +9,15 @@
 SUMMARY_MAXLEN = 400
 DESCRIPTION_MAXLEN = 5000
 
+# seconds to wait for a link or doi to respond
+URL_TIMEOUT = 30
+
 TIME_LABELS = ["lowtime", "midtime", "hightime", "veryhightime"]
 MEM_LABELS = ["lowmem", "midmem", "highmem", "veryhighmem"]
 CPU_LABELS = ["lowcpu", "midcpu", "highcpu", "veryhighcpu"]
 
 
-def check_url(url: str) -> bool:
+def check_url(url: str, timeout: int = URL_TIMEOUT) -> bool:
     import requests
     from urllib3.util.retry import Retry
     from requests.adapters import HTTPAdapter
@@ -25,13 +28,13 @@ def check_url(url: str) -> bool:
     session.mount("http://", adapter)
     session.mount("https://", adapter)
 
-    get = session.head(url)
-
-    if get.ok or get.status_code == 429:  # 429 rejected, too many requests
-        return True
-    else:
+    try:
+        get = session.head(url, timeout=timeout)
+    except requests.exceptions.RequestException:
         return False
 
+    return get.ok or get.status_code == 429  # 429 rejected, too many requests
+
 
 def check_references(references: Dict[str, Union[str, List[str]]]) -> None:
     doi = references.get("doi")
@@ -46,7 +49,7 @@ def check_references(references: Dict[str, Union[str, List[str]]]) -> None:
             doi = [doi]
         for d in doi:
             assert re.match(
-                r"^10.\d{4,9}/[-._;()/:A-Za-z0-9]+$", d
+                r"^10\.\d{4,9}/[-._;()/:A-Za-z0-9]+$", d
             ), f"Invalid DOI format: {doi}"
             assert check_url(f"https://doi.org/{d}"), f"DOI '{d}' is not reachable"
 
@@ -60,8 +63,7 @@ def check_references(references: Dict[str, Union[str, List[str]]]) -> None:
 def check_links(
     links: Dict[str, Union[str, List[str]]], required: List[str] = []
 ) -> None:
-    if not links:
-        return
+    links = links or {}
 
     for expected_link in required:
         assert expected_link in links, f"Link .links.{expected_link} is not defined"

packages/python/openproblems/src/openproblems/project/component_tests/run_and_check_output.py

@@ -47,7 +47,7 @@ def check_output_files(arguments: list) -> None:
 def check_format(arg: dict) -> None:
     """Read an output file and validate its contents against the format spec."""
     arg_info = arg.get("info") or {}
-    if arg["type"] == "file":
+    if arg["type"] == "file" and arg.get("value") is not None:
         arg_format = arg_info.get("format", {})
         file_type = arg_format.get("type") or arg_info.get("file_type")
 

packages/python/openproblems/tests/test_project_check_config.py

@@ -30,6 +30,51 @@ def test_check_config_accepts_resource_labels():
     check_config(_config())
 
 
+def test_check_url_passes_a_timeout_and_survives_a_failure():
+    import requests
+    from unittest import mock
+    from openproblems.project.component_tests.check_config import (
+        URL_TIMEOUT,
+        check_url,
+    )
+
+    with mock.patch.object(requests.Session, "head") as head:
+        head.return_value = mock.Mock(ok=True, status_code=200)
+        assert check_url("https://example.com")
+        assert head.call_args.kwargs["timeout"] == URL_TIMEOUT
+
+        head.side_effect = requests.exceptions.ConnectTimeout()
+        assert not check_url("https://example.com")
+
+
+def test_check_references_rejects_a_malformed_doi():
+    from openproblems.project.component_tests.check_config import check_references
+
+    with pytest.raises(AssertionError, match="Invalid DOI format"):
+        check_references({"doi": "10X1038/s41592-024-02189-7"})
+
+
+def test_check_references_accepts_a_bibtex_entry():
+    from openproblems.project.component_tests.check_config import check_references
+
+    check_references({"bibtex": "@article{key, title={A title}}"})
+
+
+def test_check_references_requires_a_doi_or_bibtex():
+    from openproblems.project.component_tests.check_config import check_references
+
+    with pytest.raises(AssertionError, match="should be defined"):
+        check_references({})
+
+
+def test_check_links_requires_the_expected_links():
+    from openproblems.project.component_tests.check_config import check_links
+
+    for links in [{}, None, {"documentation": "https://example.com"}]:
+        with pytest.raises(AssertionError, match="Link .links.repository"):
+            check_links(links, ["repository"])
+
+
 def test_check_config_requires_a_nextflow_runner():
     with pytest.raises(AssertionError, match="does not contain a nextflow runner"):
         check_config(_config(runners=[{"type": "executable"}]))

packages/python/openproblems/tests/test_project_run_and_check_output.py

@@ -0,0 +1,46 @@
+import pytest
+
+from openproblems.project.component_tests.run_and_check_output import (
+    check_output_files,
+    generate_cmd_args,
+    get_argument_sets,
+)
+
+
+def _arg(**kwargs):
+    arg = {
+        "name": "--output",
+        "clean_name": "output",
+        "type": "file",
+        "direction": "output",
+        "required": False,
+        "must_exist": True,
+        "multiple": False,
+        "multiple_sep": ";",
+    }
+    arg.update(kwargs)
+    return arg
+
+
+def test_check_output_files_skips_arguments_without_a_value():
+    # an optional output without a default or example never gets a value, so
+    # there is no file to read
+    arg = _arg(info={"format": {"type": "h5ad", "obs": [{"name": "label"}]}})
+    check_output_files([arg])
+
+
+def test_check_output_files_requires_required_outputs():
+    arg = _arg(required=True, info={})
+    with pytest.raises(AssertionError, match="is missing a value"):
+        check_output_files([arg])
+
+
+def test_get_argument_sets_leaves_valueless_arguments_alone():
+    config = {
+        "argument_groups": [{"name": "Arguments", "arguments": [_arg(info={})]}],
+        "all_arguments": [_arg(info={})],
+    }
+    argument_sets = get_argument_sets(config, "resources")
+
+    assert "value" not in argument_sets["run"][0]
+    assert generate_cmd_args(argument_sets["run"]) == []

packages/python/openproblems/tox.ini

@@ -8,6 +8,7 @@ description = run unit tests
 deps =
     pytest>=7
     pytest-sugar
+    requests
 commands =
     pytest {posargs:tests}