docs: improve test suite documentation

tests/conftest.py

@@ -1,3 +1,18 @@
+"""
+Shared pytest fixtures for the ToolServer test suite.
+
+Provides a FastAPI TestClient wired with fixed, pre-authorized fake
+delegated-execution identities (via require_workflow_execute /
+require_runs_read dependency overrides) and a monkeypatched tool runner,
+so most tests can exercise run/validate/results business logic without
+real Auth/IAM connectivity or real tool network calls. Authentication
+and authorization behavior itself is exercised separately, without this
+override, in tests/test_toolserver_delegated_auth.py.
+
+Developer:
+    Manish Kumar <manish@omnibioai.org>
+"""
+
 from __future__ import annotations
 
 import time
@@ -37,12 +52,14 @@
 
 
 def _authorize(app) -> None:
+    """Override the delegated-execution dependencies with fixed, already-authorized fake identities so callers skip real Auth/IAM."""
     app.dependency_overrides[require_workflow_execute] = lambda: FAKE_EXECUTE_IDENTITY
     app.dependency_overrides[require_runs_read] = lambda: FAKE_READ_IDENTITY
 
 
 @pytest.fixture()
 def client(monkeypatch, tmp_path):
+    """Build a TestClient with an isolated run-store directory, a network-free patched tool runner, and fake authorization already applied."""
     # Ensure RunStore writes into temp
     monkeypatch.setenv("TOOLSERVER_RUN_STORE_DIR", str(tmp_path / "runs"))
 

tests/test_david_annotation.py

@@ -1,3 +1,16 @@
+"""Tests for toolserver/tools/david_annotation.py, the DAVID functional
+annotation tool adapter.
+
+Covers input validation (_validate), the raw SOAP request/response
+mechanics against the DAVID web service (_soap), chart-record XML parsing
+with and without XML namespaces (_parse_chart_records), and the end-to-end
+tool run that authenticates, submits a gene list, and retrieves formatted
+enrichment results (_run).
+
+Developer:
+    Manish Kumar <manish@omnibioai.org>
+"""
+
 from __future__ import annotations
 
 from unittest.mock import MagicMock, patch
@@ -13,36 +26,45 @@
 # ═════════════════════════════════════════════════════════════════════════════
 
 class TestValidate:
+    """_validate's required-field checks for the DAVID annotation tool's inputs."""
+
     def test_valid_inputs_pass(self):
+        """A request with both email and gene_ids passes with no errors or warnings."""
         result = _validate({"email": "user@example.com", "gene_ids": "1,2,3"}, {})
         assert result["ok"] is True
         assert result["errors"] == []
         assert result["warnings"] == []
 
     def test_missing_email_fails(self):
+        """Omitting email fails validation and reports an error on the email field."""
         result = _validate({"gene_ids": "1,2,3"}, {})
         assert result["ok"] is False
         assert any(e["field"] == "email" for e in result["errors"])
 
     def test_empty_email_fails(self):
+        """An empty-string email fails validation."""
         result = _validate({"email": "", "gene_ids": "1,2,3"}, {})
         assert result["ok"] is False
 
     def test_missing_gene_ids_fails(self):
+        """Omitting gene_ids fails validation and reports an error on the gene_ids field."""
         result = _validate({"email": "user@example.com"}, {})
         assert result["ok"] is False
         assert any(e["field"] == "gene_ids" for e in result["errors"])
 
     def test_empty_gene_ids_fails(self):
+        """An empty gene_ids list fails validation."""
         result = _validate({"email": "user@example.com", "gene_ids": []}, {})
         assert result["ok"] is False
 
     def test_missing_both_gives_two_errors(self):
+        """Omitting both required fields reports exactly one error per field."""
         result = _validate({}, {})
         assert result["ok"] is False
         assert len(result["errors"]) == 2
 
     def test_error_message_references_field(self):
+        """Each validation error names the specific field it applies to."""
         result = _validate({}, {})
         fields = {e["field"] for e in result["errors"]}
         assert fields == {"email", "gene_ids"}
@@ -53,6 +75,8 @@ def test_error_message_references_field(self):
 # ═════════════════════════════════════════════════════════════════════════════
 
 class TestSoap:
+    """_soap's construction of DAVID SOAP requests and handling of responses."""
+
     def _make_client(self, response_text="<ok/>", raise_error=False):
         mock_client = MagicMock()
         mock_resp = MagicMock()
@@ -65,40 +89,47 @@ def _make_client(self, response_text="<ok/>", raise_error=False):
         return mock_client
 
     def test_posts_to_david_ws_url(self):
+        """The request is posted to the DAVID web service URL."""
         client = self._make_client()
         _soap(client, "authenticate", "<body/>")
         url = client.post.call_args[0][0]
         assert "DAVIDWebService" in url
 
     def test_sets_content_type_header(self):
+        """The request declares a text/xml Content-Type header."""
         client = self._make_client()
         _soap(client, "action", "<body/>")
         headers = client.post.call_args[1]["headers"]
         assert headers["Content-Type"] == "text/xml"
 
     def test_sets_soap_action_header(self):
+        """The SOAPAction header carries the requested action name."""
         client = self._make_client()
         _soap(client, "myAction", "<body/>")
         headers = client.post.call_args[1]["headers"]
         assert "myAction" in headers["SOAPAction"]
 
     def test_includes_body_in_envelope(self):
+        """The caller-supplied SOAP body is embedded in the request envelope."""
         client = self._make_client()
         _soap(client, "action", "<custom>data</custom>")
         content = client.post.call_args[1]["content"]
         assert "<custom>data</custom>" in content
 
     def test_returns_response_text(self):
+        """_soap returns the raw response text unchanged."""
         client = self._make_client("expected_response_text")
         result = _soap(client, "action", "<body/>")
         assert result == "expected_response_text"
 
     def test_raise_for_status_called(self):
+        """_soap checks the HTTP response status before returning."""
         client = self._make_client()
         _soap(client, "action", "<body/>")
         client.post.return_value.raise_for_status.assert_called_once()
 
     def test_propagates_http_error(self):
+        """An HTTP error status from DAVID propagates out of _soap unhandled."""
         client = self._make_client(raise_error=True)
         with pytest.raises(httpx.HTTPStatusError):
             _soap(client, "action", "<body/>")
@@ -109,13 +140,17 @@ def test_propagates_http_error(self):
 # ═════════════════════════════════════════════════════════════════════════════
 
 class TestParseChartRecords:
+    """_parse_chart_records's extraction of <return> records from DAVID chart XML."""
+
     def test_bare_return_element(self):
+        """A <return> element with no XML namespace is parsed into a record."""
         xml = "<root><return><termName>GO:0001</termName></return></root>"
         records = _parse_chart_records(xml)
         assert len(records) == 1
         assert records[0]["termName"] == "GO:0001"
 
     def test_namespaced_return_element(self):
+        """A namespaced <ns:return> element is parsed the same as a bare one."""
         xml = (
             '<root xmlns:ns="http://service.session.sample">'
             "<ns:return><ns:termName>GO:0002</ns:termName></ns:return>"
@@ -126,6 +161,7 @@ def test_namespaced_return_element(self):
         assert records[0]["termName"] == "GO:0002"
 
     def test_mixed_namespaced_and_bare_children(self):
+        """A <return> element's namespaced and bare child tags are both captured."""
         xml = (
             '<root xmlns:ns="http://example.com">'
             "<ns:return><ns:cat>BP</ns:cat><term>GO:003</term></ns:return>"
@@ -136,6 +172,7 @@ def test_mixed_namespaced_and_bare_children(self):
         assert records[0]["term"] == "GO:003"
 
     def test_multiple_records(self):
+        """Multiple <return> elements each produce their own record, in document order."""
         xml = (
             "<root>"
             "<return><t>A</t></return>"
@@ -148,20 +185,24 @@ def test_multiple_records(self):
         assert records[1]["t"] == "B"
 
     def test_invalid_xml_returns_empty(self):
+        """Malformed XML yields an empty record list rather than raising."""
         records = _parse_chart_records("<<<invalid xml>>>")
         assert records == []
 
     def test_empty_string_returns_empty(self):
+        """An empty input string yields an empty record list rather than raising."""
         records = _parse_chart_records("")
         assert records == []
 
     def test_empty_return_element_skipped(self):
+        """A <return> element with no child fields is skipped, not returned as an empty record."""
         xml = "<root><return></return><return><t>valid</t></return></root>"
         records = _parse_chart_records(xml)
         assert len(records) == 1
         assert records[0]["t"] == "valid"
 
     def test_no_return_elements_returns_empty(self):
+        """XML with no <return> elements at all yields an empty record list."""
         xml = (
             "<soapenv:Envelope "
             "xmlns:soapenv='http://schemas.xmlsoap.org/soap/envelope/'>"
@@ -172,6 +213,7 @@ def test_no_return_elements_returns_empty(self):
         assert records == []
 
     def test_all_fields_extracted(self):
+        """Every child field of a <return> element is extracted into the record."""
         xml = (
             "<root><return>"
             "<categoryName>GOTERM_BP</categoryName>"
@@ -195,11 +237,14 @@ def test_all_fields_extracted(self):
 # ═════════════════════════════════════════════════════════════════════════════
 
 class TestRun:
+    """_run's end-to-end DAVID annotation flow: authenticate, submit, fetch chart, format."""
+
     def _soap_iter(self, responses):
         it = iter(responses)
         return lambda *args, **kwargs: next(it)
 
     def test_successful_run_returns_ok(self):
+        """A full successful run (auth, addList, setCategories, getChartReport) reports ok."""
         responses = ["true", "<ok/>", "<ok/>", "<ok/>"]
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             result = _run({"email": "test@lab.com", "gene_ids": "1,2,3"}, {}, MagicMock())
@@ -208,6 +253,7 @@ def test_successful_run_returns_ok(self):
         assert result["results"] == []
 
     def test_run_meta_reflects_inputs(self):
+        """The run's meta block reflects the caller-supplied email and id_type."""
         responses = ["true", "<ok/>", "<ok/>", "<ok/>"]
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             result = _run(
@@ -218,6 +264,7 @@ def test_run_meta_reflects_inputs(self):
         assert result["meta"]["id_type"] == "GENE_SYMBOL"
 
     def test_custom_params_reflected_in_meta(self):
+        """Custom threshold, count, and categories parameters are reflected in the run's meta block."""
         responses = ["true", "<ok/>", "<ok/>", "<ok/>"]
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             result = _run(
@@ -238,18 +285,21 @@ def test_custom_params_reflected_in_meta(self):
         assert result["meta"]["categories"] == "KEGG_PATHWAY"
 
     def test_auth_failure_raises_runtime_error(self):
+        """A DAVID authentication failure raises RuntimeError instead of proceeding."""
         responses = ["<auth>FALSE</auth>"]
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             with pytest.raises(RuntimeError, match="authentication failed"):
                 _run({"email": "bad@test.com", "gene_ids": "1"}, {}, MagicMock())
 
     def test_addlist_fault_raises_runtime_error(self):
+        """A SOAP Fault from the addList step raises RuntimeError instead of proceeding."""
         responses = ["true", "<Fault>bad gene list</Fault>"]
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             with pytest.raises(RuntimeError, match="addList failed"):
                 _run({"email": "test@test.com", "gene_ids": "bad"}, {}, MagicMock())
 
     def test_chart_records_are_parsed_and_formatted(self):
+        """A single chart record from DAVID is parsed and reformatted into the run's result fields."""
         chart_xml = (
             "<root><return>"
             "<categoryName>GOTERM_BP_DIRECT</categoryName>"
@@ -279,6 +329,7 @@ def test_chart_records_are_parsed_and_formatted(self):
         assert r["bonferroni"] == "0.003"
 
     def test_multiple_chart_records(self):
+        """Multiple chart records from DAVID are each parsed into their own result entry, in order."""
         chart_xml = (
             "<root>"
             "<return><categoryName>GO_BP</categoryName><termName>T1</termName></return>"
@@ -293,13 +344,15 @@ def test_multiple_chart_records(self):
         assert result["results"][1]["category"] == "KEGG"
 
     def test_log_called_during_run(self):
+        """The caller-supplied log callback is invoked at each step of the run."""
         responses = ["true", "<ok/>", "<ok/>", "<ok/>"]
         log = MagicMock()
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             _run({"email": "test@test.com", "gene_ids": "1"}, {}, log)
         assert log.call_count >= 4
 
     def test_default_parameters_applied(self):
+        """Omitted optional parameters fall back to DAVID's documented defaults."""
         responses = ["true", "<ok/>", "<ok/>", "<ok/>"]
         with patch("toolserver.tools.david_annotation._soap", side_effect=self._soap_iter(responses)):
             result = _run({"email": "test@test.com", "gene_ids": "1,2"}, {}, MagicMock())

tests/test_endpoints.py

@@ -1,4 +1,14 @@
+"""
+Tests for ToolServer's core HTTP endpoints: health, capabilities,
+validation, and the create-run-to-completion happy path.
+
+Developer:
+    Manish Kumar <manish@omnibioai.org>
+"""
+
+
 def test_health(client):
+    """Return 200 with ok=True and a service identifier."""
     r = client.get("/health")
     assert r.status_code == 200
     body = r.json()
@@ -7,6 +17,7 @@ def test_health(client):
 
 
 def test_capabilities(client):
+    """List registered engines and tools, including enrichr_pathway."""
     r = client.get("/capabilities")
     assert r.status_code == 200
     caps = r.json()
@@ -17,6 +28,7 @@ def test_capabilities(client):
 
 
 def test_validate_unknown_tool(client):
+    """Report UNKNOWN_TOOL as a validation error, not an HTTP failure, for an unregistered tool_id."""
     r = client.post("/validate", json={"tool_id": "nope", "inputs": {}, "resources": {}})
     assert r.status_code == 200
     body = r.json()
@@ -25,6 +37,7 @@ def test_validate_unknown_tool(client):
 
 
 def test_validate_enrichr_pathway_requires_genes(client):
+    """Flag the missing required "genes" field when validating enrichr_pathway inputs."""
     r = client.post("/validate", json={"tool_id": "enrichr_pathway", "inputs": {}, "resources": {}})
     assert r.status_code == 200
     body = r.json()
@@ -33,6 +46,7 @@ def test_validate_enrichr_pathway_requires_genes(client):
 
 
 def test_create_run_validation_fails(client):
+    """Reject run creation with 400 VALIDATION_FAILED when required inputs are missing."""
     r = client.post("/runs", json={"tool_id": "enrichr_pathway", "inputs": {}, "resources": {}})
     assert r.status_code == 400
     body = r.json()
@@ -41,6 +55,7 @@ def test_create_run_validation_fails(client):
 
 
 def test_create_run_and_poll_to_complete(client):
+    """Drive a valid run from creation through COMPLETED state, with logs and results populated along the way."""
     req = {
         "tool_id": "enrichr_pathway",
         "inputs": {"genes": ["TP53", "BRCA1"], "top_n": 10},