From f6c9686d75c819ff280b6dbda6912a0277cf0d96 Mon Sep 17 00:00:00 2001
From: Ryan Williams <ryan.williams@tiledb.com>
Date: Thu, 15 Aug 2024 10:08:57 -0400
Subject: [PATCH] [python] Docstring typo, test-assertion msgs, `maybe_raises`
 improvement, `verify_logs` helper (#2892)

* docstring typo, test-assertion msgs, parametrize_cases nits

* `maybe_raises`: optionally accept message match string

* `tests._util`: `verify_logs` helper
---
 apis/python/src/tiledbsoma/io/outgest.py |  2 +-
 apis/python/tests/_util.py               | 58 +++++++++++++++++++-----
 apis/python/tests/parametrize_cases.py   |  2 +
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/apis/python/src/tiledbsoma/io/outgest.py b/apis/python/src/tiledbsoma/io/outgest.py
index 5685e25b88..6a649ceac4 100644
--- a/apis/python/src/tiledbsoma/io/outgest.py
+++ b/apis/python/src/tiledbsoma/io/outgest.py
@@ -237,7 +237,7 @@ def to_anndata(
 
     If ``uns_keys`` is provided, only the specified top-level ``uns`` keys
     are extracted.  The default is to extract them all.  Use ``uns_keys=[]``
-    to not ingest any ``uns`` keys.
+    to not outgest any ``uns`` keys.
 
     Lifecycle:
         Maturing.
diff --git a/apis/python/tests/_util.py b/apis/python/tests/_util.py
index e282508251..7734ef0c57 100644
--- a/apis/python/tests/_util.py
+++ b/apis/python/tests/_util.py
@@ -1,3 +1,4 @@
+import re
 from contextlib import contextmanager, nullcontext
 from pathlib import Path
 from typing import Any, List, Optional, Tuple, Type, Union
@@ -6,6 +7,7 @@
 import pandas as pd
 import pytest
 from _pytest._code import ExceptionInfo
+from _pytest.logging import LogCaptureFixture
 from _pytest.python_api import E, RaisesContext
 from anndata import AnnData
 from numpy import array_equal
@@ -15,7 +17,9 @@
 
 
 def assert_uns_equal(uns0, uns1):
-    assert uns0.keys() == uns1.keys()
+    assert (
+        uns0.keys() == uns1.keys()
+    ), f"extra keys: {uns0.keys() - uns1.keys()}, missing keys: {uns1.keys() - uns0.keys()}"
     for k, v0 in uns0.items():
         try:
             v1 = uns1[k]
@@ -34,7 +38,7 @@ def assert_uns_equal(uns0, uns1):
             elif isinstance(v0, np.ndarray):
                 assert array_equal(v0, v1)
             elif isinstance(v0, (int, float, str, bool)):
-                assert v0 == v1
+                assert v0 == v1, f"{v0} != {v1}"
             else:
                 raise ValueError(f"Unsupported type: {type(v0)}")
         except AssertionError:
@@ -115,20 +119,50 @@ def raises_no_typeguard(exc: Type[Exception], *args: Any, **kwargs: Any):
             yield
 
 
+# Alias for several types that can be used to specify expected exceptions in `maybe_raises`
+Err = Union[str, Type[E], Tuple[Type[E], str]]
+
+
 def maybe_raises(
-    expected_exception: Union[None, Type[E], Tuple[Type[E], ...]],
+    expected_exception: Optional[Err],
     *args: Any,
     **kwargs: Any,
-) -> Union[RaisesContext[E], ExceptionInfo[E]]:
+) -> Union[RaisesContext[E], ExceptionInfo[E], nullcontext]:
     """
-    Wrapper around ``pytest.raises`` that accepts None (signifying no exception should be raised).
-    This is only necessary since ``pytest.raises`` does not itself accept None, so we are
-    decorating.
+    Wrapper around ``pytest.raises`` that additionally accepts ``None`` (signifying no exception
+    should be raised), a string (signifying a message match) or a tuple of (exception, message).
 
     Useful in test cases that are parameterized to test both valid and invalid inputs.
     """
-    return (
-        nullcontext()
-        if expected_exception is None
-        else pytest.raises(expected_exception, *args, **kwargs)
-    )
+    if expected_exception is not None:
+        if isinstance(expected_exception, type):
+            exc = expected_exception
+        else:
+            if isinstance(expected_exception, str):
+                exc, match = Exception, expected_exception
+            else:
+                exc, match = expected_exception
+            if "match" in kwargs:
+                raise ValueError(
+                    "Cannot specify 'match' in both kwargs and `expected_exception`"
+                )
+            kwargs["match"] = match
+        return pytest.raises(exc, *args, **kwargs)
+    else:
+        return nullcontext()
+
+
+def verify_logs(caplog: LogCaptureFixture, expected_logs: Optional[List[str]]) -> None:
+    """Verify that expected log messages are present in a pytest "caplog" (captured logs) fixture."""
+    if not expected_logs:
+        return
+
+    for expected_log in expected_logs:
+        found = False
+        for record in caplog.records:
+            log_msg = record.getMessage()
+            if re.search(expected_log, log_msg):
+                found = True
+                break
+        if not found:
+            raise AssertionError(f"Expected log message not found: {expected_log}")
diff --git a/apis/python/tests/parametrize_cases.py b/apis/python/tests/parametrize_cases.py
index 81cd633d80..4ff43cb0dc 100644
--- a/apis/python/tests/parametrize_cases.py
+++ b/apis/python/tests/parametrize_cases.py
@@ -24,6 +24,7 @@ def wrapper(fn):
         """
         # Test-case IDs
         ids = [case.id for case in cases]
+
         # Convert each case to a "values" array; also filter and reorder to match kwargs expected
         # by the wrapped "test_*" function.
         spec = getfullargspec(fn)
@@ -33,6 +34,7 @@ def wrapper(fn):
             {name: rt_dict[name] for name in names}.values()
             for rt_dict in [asdict(case) for case in cases]
         ]
+
         # Delegate to PyTest `parametrize`
         return pytest.mark.parametrize(
             names,  # kwarg names