From 8cf8f412de025b3bc6974360337722dbf79cd150 Mon Sep 17 00:00:00 2001
From: BJ Fulton <bjfultn@gmail.com>
Date: Tue, 6 Oct 2026 13:10:19 -0600
Subject: [PATCH] Declare runtime dependencies in setup.py

sqlparse, astropy, beautifulsoup4, lxml and xmltodict are needed at run
time but were listed only in requirements-test.txt, so CI (which installs
with --no-deps) passed while a real pip install produced a service whose
async jobs all failed: BeautifulSoup asks for the lxml parser by name.
Database drivers stay undeclared; each deployment installs its own.

tests/test_install_requires.py fails when TAP imports a package that
setup.py does not declare.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 requirements-test.txt          |  8 ++---
 setup.py                       |  6 +++-
 tests/test_install_requires.py | 59 ++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 5 deletions(-)
 create mode 100644 tests/test_install_requires.py

diff --git a/requirements-test.txt b/requirements-test.txt
index 22dfd2b..f8c7ea5 100644
--- a/requirements-test.txt
+++ b/requirements-test.txt
@@ -5,10 +5,10 @@ pytest>=7
 ruff==0.15.2
 requests
 
-# Runtime deps. These are not declared in setup.py's install_requires (a
-# separate cleanup) but the package imports them. The CI install uses
-# --no-deps for nexsciTAP itself (to avoid the spatial_index PyPI hole),
-# so we list everything actually-needed here.
+# Runtime deps, repeated from setup.py's install_requires: the CI install
+# uses --no-deps for nexsciTAP itself (to avoid the spatial_index PyPI
+# hole), so they are listed here too.  tests/test_install_requires.py
+# keeps setup.py complete.
 configobj
 sqlparse
 astropy
diff --git a/setup.py b/setup.py
index b660788..c31d093 100644
--- a/setup.py
+++ b/setup.py
@@ -13,6 +13,10 @@
     description='NExScI VO Table Access Protocol (TAP) web service',
     long_description=open('README.md').read(),
     ext_modules=[Extension('TAP/writerecs', ['TAP/writerecsmodule.c'])],
-    install_requires=['ADQL', 'spatial_index', 'configobj'],
+    # Database drivers (cx_Oracle, psycopg2, mysql-connector) are not listed:
+    # each deployment installs the one for its DBMS.  lxml is not imported,
+    # but BeautifulSoup is asked for it by name to read UWS job documents.
+    install_requires=['ADQL', 'spatial_index', 'configobj', 'sqlparse',
+                      'astropy', 'beautifulsoup4', 'lxml', 'xmltodict'],
     packages=['TAP']
 )
diff --git a/tests/test_install_requires.py b/tests/test_install_requires.py
new file mode 100644
index 0000000..2b22fa2
--- /dev/null
+++ b/tests/test_install_requires.py
@@ -0,0 +1,59 @@
+"""setup.py declares every package the TAP code needs at run time.
+
+CI installs nexsciTAP with --no-deps and gets its runtime packages from
+requirements-test.txt, so a package missing from install_requires passes
+CI and only fails on a real `pip install`.  Missing lxml did exactly that:
+sync queries worked and every async job returned 500.
+"""
+from __future__ import annotations
+
+import ast
+import re
+import sys
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+
+# Import name -> distribution name, where they differ.
+DIST_NAME = {'bs4': 'beautifulsoup4'}
+
+# Database drivers are imported only for the DBMS a deployment configures.
+OPTIONAL = {'cx_Oracle', 'psycopg2', 'mysql'}
+
+# Not imported, but named as a parser: BeautifulSoup(data, 'lxml').
+NAMED_ONLY = {'lxml'}
+
+
+def _install_requires() -> set[str]:
+    tree = ast.parse((REPO_ROOT / 'setup.py').read_text())
+    for node in ast.walk(tree):
+        if isinstance(node, ast.keyword) and node.arg == 'install_requires':
+            reqs = ast.literal_eval(node.value)
+            return {re.split(r'[\s;\[<>=!~]', r, maxsplit=1)[0] for r in reqs}
+    raise AssertionError('setup.py has no install_requires')
+
+
+def _third_party_imports() -> set[str]:
+    found = set()
+    for path in (REPO_ROOT / 'TAP').glob('*.py'):
+        for node in ast.walk(ast.parse(path.read_text())):
+            if isinstance(node, ast.Import):
+                names = [a.name for a in node.names]
+            elif isinstance(node, ast.ImportFrom) and node.level == 0:
+                names = [node.module or '']
+            else:
+                continue
+            for name in names:
+                top = name.split('.')[0]
+                if top != 'TAP' and top not in sys.stdlib_module_names:
+                    found.add(top)
+    return found
+
+
+@pytest.mark.skipif(sys.version_info < (3, 10), reason='needs sys.stdlib_module_names')
+def test_every_runtime_import_is_declared():
+    needed = {DIST_NAME.get(m, m) for m in _third_party_imports() - OPTIONAL} | NAMED_ONLY
+    missing = needed - _install_requires()
+    assert not missing, f'add to install_requires in setup.py: {sorted(missing)}'