From 704b97b1abd3bdba0f609f000effbbae8a5d4648 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 21:16:15 +1100 Subject: [PATCH 1/2] fix(ci): make the integration job's Tier E and API tests query the right place (#45) Two independent causes, both reproduced locally against PostgreSQL 16 with the job's exact steps: 1. Tier E bound the schema name with `psql --set ... -c`. psql never interpolates :'var' inside -c, so every query was a syntax error, which _column_signature turned into an empty result: "0 tables". The query now goes on stdin, and a failed query is reported as a query error instead of an empty schema. Three negative controls; two of them fail against the old code. 2. The job sets PGDATABASE=postgres, and api/config.py reads PGDATABASE, so the API looked for te_dev in the wrong database. The full-suite step now sets PGDATABASE=te_mgmt_dev. With Tier E working it found one real difference: dev has test_run_results, created by the Tier S SQL test harness. It is listed in the Tier E expectation file as ignored, with the reason, rather than filtered silently. Local replay: evals P/I/S/E 26/26, test_report --strict 239/239, 0 skipped; make lint clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AYM3RR8Fmw2pwqFfnJhAo6 --- .github/workflows/quality-gate.yml | 4 ++ .../01_envs_have_identical_structure.json | 2 + evals/runner.py | 31 ++++++++--- tests/test_evals_tier_e_x.py | 55 +++++++++++++++++++ 4 files changed, 85 insertions(+), 7 deletions(-) diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index 9a8cf79..821ae9a 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -158,7 +158,11 @@ jobs: # Prints a final block accounting for every test: PASSED / FAILED / # ERROR / SKIPPED (with reasons) / NOT RUN (deselected). --strict fails # the build on any skip. + # The API reads PGDATABASE (api/config.py). The schema lives in + # te_mgmt_dev, not the job-level default "postgres" (issue #45). - name: Full test suite — final result with skip accounting + env: + PGDATABASE: te_mgmt_dev run: python3 scripts/test_report.py --strict - name: Upload eval reports diff --git a/evals/expected/tier_e/01_envs_have_identical_structure.json b/evals/expected/tier_e/01_envs_have_identical_structure.json index adb8904..0d9bc45 100644 --- a/evals/expected/tier_e/01_envs_have_identical_structure.json +++ b/evals/expected/tier_e/01_envs_have_identical_structure.json @@ -3,6 +3,8 @@ "description": "Deploy all four environments and assert their deployed schemas are structurally identical; only the database/schema namespace may differ. Closes VCRM BR-01.", "envs_under_test": ["dev", "test", "staging", "prod"], "reference_env": "dev", + "ignore_tables": ["test_run_results"], + "ignore_tables_reason": "Created in te_dev by tests/framework/test_framework.sql when Tier S runs; test-harness state, not deployed schema.", "expected": { "all_envs_deploy_exit_zero": true, "column_signature_identical_across_envs": true, diff --git a/evals/runner.py b/evals/runner.py index c191a49..cf82dc3 100644 --- a/evals/runner.py +++ b/evals/runner.py @@ -596,8 +596,9 @@ def _column_signature(db: str, schema: str) -> Dict[str, List[str]]: different namespaces compare equal when their structure matches. """ # The schema name is bound by psql via --set + :'sch' rather than pasted into - # the SQL text, so there is no string interpolation to reason about even - # though the value comes from the ENV_TARGETS constant. + # the SQL text. psql only interpolates variables in SQL it reads from a file + # or stdin, never in -c, so the query goes on stdin (issue #45: under -c it + # was a syntax error that this function used to report as "0 tables"). query = ( "SELECT table_name, column_name, data_type, is_nullable " "FROM information_schema.columns " @@ -605,11 +606,12 @@ def _column_signature(db: str, schema: str) -> Dict[str, List[str]]: "ORDER BY table_name, ordinal_position" ) r = subprocess.run( - ["psql", "-tA", "-d", db, "--set=sch=" + schema, "-c", query], - env=_pg_env(), capture_output=True, text=True, timeout=30, + ["psql", "-tA", "-v", "ON_ERROR_STOP=1", "-d", db, "--set=sch=" + schema], + input=query, env=_pg_env(), capture_output=True, text=True, timeout=30, ) if r.returncode != 0: - return {} + raise RuntimeError("schema query on " + db + "." + schema + " failed: " + + r.stderr.strip()[-300:]) sig: Dict[str, List[str]] = {} for line in r.stdout.splitlines(): parts = line.split("|") @@ -689,16 +691,31 @@ def _run_envs_have_identical_structure( deploys = [_deploy_env(env_name) for env_name, _, _ in targets] signatures: Dict[str, Dict[str, List[str]]] = {} + query_errors: List[str] = [] for env_name, db, schema in targets: - signatures[env_name] = _column_signature(db, schema) + try: + signatures[env_name] = _column_signature(db, schema) + except RuntimeError as exc: + signatures[env_name] = {} + query_errors.append("env " + env_name + ": " + str(exc)) + + # Tables the SQL test harness creates in the schema it tests (Tier S runs + # tests/framework/test_framework.sql in te_dev). Not deployed schema, so + # not drift. Listed in the expectation file, never silently. + ignored = set(expected.get("ignore_tables", [])) + for sig in signatures.values(): + for table in ignored: + sig.pop(table, None) table_counts = {e: len(sig) for e, sig in signatures.items()} result.actual = { "deploys": deploys, "table_counts": table_counts, + "ignored_tables": sorted(ignored), } - errors: List[str] = [] + # A failed query is not an empty schema; report it as what it is. + errors: List[str] = list(query_errors) if exp.get("all_envs_deploy_exit_zero") is True: for d in deploys: diff --git a/tests/test_evals_tier_e_x.py b/tests/test_evals_tier_e_x.py index 1088f84..b74d2f1 100644 --- a/tests/test_evals_tier_e_x.py +++ b/tests/test_evals_tier_e_x.py @@ -9,8 +9,10 @@ from __future__ import annotations +import subprocess import sys import unittest +from unittest.mock import patch from pathlib import Path import pytest @@ -83,6 +85,59 @@ def test_env_targets_cover_all_four_environments(self): self.assertEqual(len({e[2] for e in runner.ENV_TARGETS}), 4) +@pytest.mark.unit +class TierEColumnSignatureQuery(unittest.TestCase): + """Issue #45: the schema query itself, not just the comparison. + + psql never interpolates :'var' inside -c, so the query must go on stdin, and + a failed query must not be reported as an empty schema. + """ + + def _scenario(self, completed): + expected = {"expected": {"min_tables_per_env": 12}, + "envs_under_test": ["dev"]} + result = runner.ScenarioResult(tier="e", name="t") + with patch.object(runner, "_deploy_env", + return_value={"env": "dev", "exit_code": 0}), \ + patch.object(runner.subprocess, "run", return_value=completed) as run: + runner._run_envs_have_identical_structure(result, expected) + return result, run + + def test_query_is_sent_on_stdin_not_via_dash_c(self): + ok = subprocess.CompletedProcess([], 0, stdout="t|c|text|NO\n", stderr="") + _, run = self._scenario(ok) + args, kwargs = run.call_args + self.assertNotIn("-c", args[0]) + self.assertIn(":'sch'", kwargs["input"]) + + def test_query_failure_is_reported_as_a_query_error(self): + bad = subprocess.CompletedProcess([], 1, stdout="", + stderr='ERROR: syntax error at or near ":"') + result, _ = self._scenario(bad) + self.assertFalse(result.passed) + self.assertTrue(any("schema query" in e and "syntax error" in e + for e in result.errors), result.errors) + + def test_only_listed_harness_tables_are_ignored(self): + two = subprocess.CompletedProcess( + [], 0, stdout="test_run_results|id|bigint|NO\nstray|id|bigint|NO\n", stderr="") + expected = {"expected": {"min_tables_per_env": 1}, + "envs_under_test": ["dev"], "ignore_tables": ["test_run_results"]} + result = runner.ScenarioResult(tier="e", name="t") + with patch.object(runner, "_deploy_env", + return_value={"env": "dev", "exit_code": 0}), \ + patch.object(runner.subprocess, "run", return_value=two): + runner._run_envs_have_identical_structure(result, expected) + self.assertEqual(result.actual["table_counts"], {"dev": 1}) + + def test_genuinely_empty_schema_still_fails(self): + empty = subprocess.CompletedProcess([], 0, stdout="", stderr="") + result, _ = self._scenario(empty) + self.assertFalse(result.passed) + self.assertIn("env dev has 0 tables in its schema, expected >= 12 " + "(deploy may have partially failed)", result.errors) + + @pytest.mark.unit class TierXSqliteStructure(unittest.TestCase): """O-08: SQLite DDL execution and structural readback.""" From bc6ff27edc685d8bf252f68e19febd264080b296 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 21:42:05 +1100 Subject: [PATCH 2/2] fix(tests): keep SQL harness objects out of the schema under test (#45) Replaces the ignore_tables exception from the previous commit, per review: Tier E should need no exceptions. run_all_tests.sql now creates schema te_test_harness and puts it first on search_path, so test_framework.sql's unqualified CREATEs (test_run_results, assert_*, report_*) land there instead of in te_dev. T&E tables still resolve from the env schema, and the suites already address it through te.schema_name. The four report calls are qualified with te_test_harness. Tier S caught the first attempt, which left the report calls qualified with the env schema. Local replay on PostgreSQL 16: te_dev holds exactly 12 tables; evals P/I/S/E 26/26, 0 skipped; test_report --strict 238/238, 0 skipped; make lint clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AYM3RR8Fmw2pwqFfnJhAo6 --- .../01_envs_have_identical_structure.json | 2 -- evals/runner.py | 9 --------- tests/run_all_tests.sql | 18 ++++++++++++------ tests/test_evals_tier_e_x.py | 12 ------------ 4 files changed, 12 insertions(+), 29 deletions(-) diff --git a/evals/expected/tier_e/01_envs_have_identical_structure.json b/evals/expected/tier_e/01_envs_have_identical_structure.json index 0d9bc45..adb8904 100644 --- a/evals/expected/tier_e/01_envs_have_identical_structure.json +++ b/evals/expected/tier_e/01_envs_have_identical_structure.json @@ -3,8 +3,6 @@ "description": "Deploy all four environments and assert their deployed schemas are structurally identical; only the database/schema namespace may differ. Closes VCRM BR-01.", "envs_under_test": ["dev", "test", "staging", "prod"], "reference_env": "dev", - "ignore_tables": ["test_run_results"], - "ignore_tables_reason": "Created in te_dev by tests/framework/test_framework.sql when Tier S runs; test-harness state, not deployed schema.", "expected": { "all_envs_deploy_exit_zero": true, "column_signature_identical_across_envs": true, diff --git a/evals/runner.py b/evals/runner.py index cf82dc3..5a9c0a0 100644 --- a/evals/runner.py +++ b/evals/runner.py @@ -699,19 +699,10 @@ def _run_envs_have_identical_structure( signatures[env_name] = {} query_errors.append("env " + env_name + ": " + str(exc)) - # Tables the SQL test harness creates in the schema it tests (Tier S runs - # tests/framework/test_framework.sql in te_dev). Not deployed schema, so - # not drift. Listed in the expectation file, never silently. - ignored = set(expected.get("ignore_tables", [])) - for sig in signatures.values(): - for table in ignored: - sig.pop(table, None) - table_counts = {e: len(sig) for e, sig in signatures.items()} result.actual = { "deploys": deploys, "table_counts": table_counts, - "ignored_tables": sorted(ignored), } # A failed query is not an empty schema; report it as what it is. diff --git a/tests/run_all_tests.sql b/tests/run_all_tests.sql index 8dd4034..abec3d9 100644 --- a/tests/run_all_tests.sql +++ b/tests/run_all_tests.sql @@ -32,11 +32,17 @@ -- ── Step 1: Load the test framework (results table + assertion functions) ───── --- Set search_path so framework objects and assertions resolve without schema prefix. +-- The harness keeps its own objects (test_run_results, assert_*, report_*) in +-- te_test_harness so they never land in the schema under test, where Tier E +-- would see them as drift (issue #45). It comes first on search_path, so the +-- framework's unqualified CREATEs go there; T&E tables still resolve unqualified +-- from the env schema, and the suites address it via te.schema_name anyway. +CREATE SCHEMA IF NOT EXISTS te_test_harness; + -- Expose per-env vars via current_setting() for PL/pgSQL DO blocks (:'var' doesn't -- substitute inside $dollar-quoted$ strings in psql 18). SELECT - set_config('search_path', :'schema_name' || ',public', false), + set_config('search_path', 'te_test_harness,' || :'schema_name' || ',public', false), set_config('te.schema_name', :'schema_name', false), set_config('te.app_user', :'app_user', false), set_config('te.conn_limit', :'conn_limit', false); @@ -70,7 +76,7 @@ SELECT skipped, pass_rate, suite_status -FROM :"schema_name".report_suite_summary(); +FROM te_test_harness.report_suite_summary(); \echo '' \echo '============================================================' @@ -83,7 +89,7 @@ SELECT expected, actual, message -FROM :"schema_name".report_failures(); +FROM te_test_harness.report_failures(); \echo '' \echo '============================================================' @@ -97,7 +103,7 @@ SELECT expected, actual, message -FROM :"schema_name".report_detail(); +FROM te_test_harness.report_detail(); \echo '' \echo '============================================================' @@ -111,7 +117,7 @@ SELECT skipped, pass_rate, overall -FROM :"schema_name".report_totals(); +FROM te_test_harness.report_totals(); \echo '' \echo '============================================================' diff --git a/tests/test_evals_tier_e_x.py b/tests/test_evals_tier_e_x.py index b74d2f1..51efa99 100644 --- a/tests/test_evals_tier_e_x.py +++ b/tests/test_evals_tier_e_x.py @@ -118,18 +118,6 @@ def test_query_failure_is_reported_as_a_query_error(self): self.assertTrue(any("schema query" in e and "syntax error" in e for e in result.errors), result.errors) - def test_only_listed_harness_tables_are_ignored(self): - two = subprocess.CompletedProcess( - [], 0, stdout="test_run_results|id|bigint|NO\nstray|id|bigint|NO\n", stderr="") - expected = {"expected": {"min_tables_per_env": 1}, - "envs_under_test": ["dev"], "ignore_tables": ["test_run_results"]} - result = runner.ScenarioResult(tier="e", name="t") - with patch.object(runner, "_deploy_env", - return_value={"env": "dev", "exit_code": 0}), \ - patch.object(runner.subprocess, "run", return_value=two): - runner._run_envs_have_identical_structure(result, expected) - self.assertEqual(result.actual["table_counts"], {"dev": 1}) - def test_genuinely_empty_schema_still_fails(self): empty = subprocess.CompletedProcess([], 0, stdout="", stderr="") result, _ = self._scenario(empty)