From 7a9c230a27bd83fbd393fc8955775b1a47a2d346 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:06:20 +0000 Subject: [PATCH 1/9] test: regression tests for #175, #178, #183 and #189 These closed issues had no behavioural test. Each test fails when the original bug is put back (checked by hand for all four): key-prefix rate limit identity, U+001F delimiter hashing, uploader overwrite and mode switch over an existing dataset, and survivor-position T&E row numbers. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- tests/test_issue_regressions.py | 242 ++++++++++++++++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 tests/test_issue_regressions.py diff --git a/tests/test_issue_regressions.py b/tests/test_issue_regressions.py new file mode 100644 index 0000000..9c1f9c6 --- /dev/null +++ b/tests/test_issue_regressions.py @@ -0,0 +1,242 @@ +"""Regression tests for closed issues that had no behavioural test of their own. + +One class per issue, each named for the defect it keeps from coming back: + + #183 rate limiter used the (unvalidated) X-API-Key prefix as the caller's identity + #178 distinct CSV rows silently dropped as "duplicates" (ambiguous hash serialisation) + #175 uploader could destroy a dataset through overwrite or a mode switch + #189 T&E database errors reported the wrong source row + +Integration classes need a live PostgreSQL, as in CI. Run: pytest tests/test_issue_regressions.py +""" + +from __future__ import annotations + +import csv +import io +import os +import unittest +import uuid + +import pytest +from fastapi.testclient import TestClient +from psycopg2 import sql +from starlette.requests import Request + +from api.config import settings +from api.db import Conn +from api.main import app +from api.ratelimit import _client_id, limiter +from api.services import accounts +from api.services.accounts import ROLES +from api.tenancy import migrate_tenant, schema_for + +_PW = "correct horse battery staple" + + +def _csv(rows: list[list[str]]) -> str: + out = io.StringIO() + csv.writer(out, lineterminator="\n").writerows(rows) + return out.getvalue() + + +# ── #183 ───────────────────────────────────────────────────────────────────── + +def _request(api_key: str, host: str = "203.0.113.9") -> Request: + return Request({"type": "http", "headers": [(b"x-api-key", api_key.encode())], "client": (host, 4321)}) + + +@pytest.mark.unit +class RateLimiterIdentity(unittest.TestCase): + """#183: an attacker must not get a fresh rate-limit bucket by changing X-API-Key.""" + + def setUp(self): + limiter.reset() + self._saved = settings.RATE_LIMIT_WRITES_PER_MINUTE + self.client = TestClient(app) + + def tearDown(self): + settings.RATE_LIMIT_WRITES_PER_MINUTE = self._saved + limiter.reset() + + def test_identity_does_not_depend_on_the_api_key(self): + keys = [f"rotate{i:02d}-{'x' * i}" for i in range(10)] + self.assertEqual(len({_client_id(_request(k)) for k in keys}), 1) + + def test_identity_is_the_peer_address(self): + self.assertNotEqual(_client_id(_request("k", "203.0.113.9")), _client_id(_request("k", "203.0.113.10"))) + + def test_rotating_invalid_keys_from_one_client_share_one_budget(self): + settings.RATE_LIMIT_WRITES_PER_MINUTE = 2 + payload = {"fileName": "x.csv", "content": "a,b\n1,2\n"} + codes = [ + self.client.post("/api/csv/preview", json=payload, headers={"X-API-Key": f"rotate{i:02d}-aaaa"}).status_code + for i in range(5) + ] + # Whatever auth says to the first two, the budget is spent after them: the third is refused, + # not the sixth (one bucket per rotated prefix, the old behaviour, would never refuse). + self.assertEqual(codes[2:], [429, 429, 429], codes) + + +# ── #178 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.integration +class RowDedupKeepsDistinctRows(unittest.TestCase): + """#178: two different rows must not be skipped as duplicates because they serialise alike.""" + + @classmethod + def setUpClass(cls): + cls.ctx = TestClient(app) + cls.client = cls.ctx.__enter__() + + @classmethod + def tearDownClass(cls): + cls.ctx.__exit__(None, None, None) + + def setUp(self): + self.tag = f"{os.getpid()}_{uuid.uuid4().hex[:8]}" + + def _upload(self, rows: list[list[str]], name: str = "dedup") -> dict: + body = self.client.post("/api/csv/upload", json={ + "fileName": f"{name}_{self.tag}.csv", "content": _csv([["a", "b", "c"]] + rows)}).json() + self.assertEqual(body["status"], "ok", body) + return body + + def test_the_reported_unit_separator_case_keeps_both_rows(self): + # Row 1 = ("x\x1fy", "z"), row 2 = ("x", "y\x1fz"): the same characters joined by \x1f. + body = self._upload([["x\x1fy", "z", self.tag], ["x", "y\x1fz", self.tag]]) + self.assertEqual((body["insertedRows"], body["duplicateRowsSkipped"]), (2, 0)) + + def test_every_control_character_stays_distinct_as_a_delimiter_candidate(self): + rows = [] + for code in range(1, 32): # NUL can't be stored in PostgreSQL text + ch, marker = chr(code), f"{self.tag}_{code}" + rows += [[f"a{ch}b", "c", marker], ["a", f"b{ch}c", marker]] + body = self._upload(rows, "controls") + self.assertEqual((body["insertedRows"], body["duplicateRowsSkipped"]), (len(rows), 0)) + + def test_unicode_empty_quote_comma_and_newline_cells_stay_distinct(self): + t = self.tag + pairs = [ + (["héllo", "wörld", t + "1"], ["héllowörld", "", t + "1"]), + (["", "ab", t + "2"], ["ab", "", t + "2"]), + (['a"b', "c", t + "3"], ["a", 'b"c', t + "3"]), + (["a,b", "c", t + "4"], ["a", "b,c", t + "4"]), + (["a\nb", "c", t + "5"], ["a", "b\nc", t + "5"]), + (["日本", "語", t + "6"], ["日", "本語", t + "6"]), + ] + rows = [r for pair in pairs for r in pair] + body = self._upload(rows, "mixed") + self.assertEqual((body["insertedRows"], body["duplicateRowsSkipped"]), (len(rows), 0)) + + def test_a_genuinely_repeated_row_is_still_skipped(self): + body = self._upload([["same", "row", self.tag], ["same", "row", self.tag]], "repeat") + self.assertEqual((body["insertedRows"], body["duplicateRowsSkipped"]), (1, 1)) + + def test_stored_hashes_carry_the_version_marker(self): + body = self._upload([["v", "2", self.tag]], "hashver") + table = body["tableName"].split(".")[-1] + with Conn() as conn, conn.cursor() as cur: + cur.execute(sql.SQL("SELECT _row_hash FROM {}.{}").format( + sql.Identifier(settings.UPLOADS_SCHEMA), sql.Identifier(table))) + hashes = [r[0] for r in cur.fetchall()] + self.assertTrue(hashes and all(h.startswith("v2:") for h in hashes), hashes) + + +# ── #175 / #189 need a signed-in tenant ────────────────────────────────────── + +@pytest.mark.integration +class UploadPaths(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.ctx = TestClient(app) + cls.client = cls.ctx.__enter__() + cls.slug = f"issues_{os.getpid()}_{uuid.uuid4().hex[:6]}" + cls.headers = {} + with Conn() as conn, conn.cursor() as cur: + tid = accounts.create_tenant(cur, cls.slug, "Issue regressions") + for role in ROLES: + uid = accounts.create_user(cur, f"{cls.slug}_{role}", _PW) + accounts.add_membership(cur, uid, tid, role) + conn.commit() + migrate_tenant(cls.slug) + for role in ROLES: + res = cls.client.post("/api/auth/login", json={ + "tenant": cls.slug, "username": f"{cls.slug}_{role}", "password": _PW}) + assert res.status_code == 200, res.text + cls.headers[role] = {"Authorization": f"Bearer {res.json()['token']}"} + + @classmethod + def tearDownClass(cls): + with Conn() as conn, conn.cursor() as cur: + cur.execute(sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(sql.Identifier(schema_for(cls.slug)))) + cur.execute(sql.SQL("DELETE FROM {}.tenants WHERE slug = %s").format( + sql.Identifier(settings.CONTROL_SCHEMA)), (cls.slug,)) + cur.execute(sql.SQL("DELETE FROM {}.users WHERE username LIKE %s").format( + sql.Identifier(settings.CONTROL_SCHEMA)), (f"{cls.slug}_%",)) + conn.commit() + cls.ctx.__exit__(None, None, None) + + def setUp(self): + self.tag = uuid.uuid4().hex[:10] + + def _post(self, role: str, **body): + return self.client.post("/api/csv/upload", headers=self.headers[role], json=body) + + def _file_names(self, role: str = "tenant_admin") -> list[str]: + return [f["file_name"] for f in self.client.get("/api/csv/files", headers=self.headers[role]).json()] + + +@pytest.mark.integration +class UploadCannotBypassTheDeleteGate(UploadPaths): + """#175: overwrite and mode switch drop the prior table, so they need delete permission.""" + + def _dynamic(self, name: str) -> dict: + res = self._post("tenant_admin", fileName=name, content=f"k\n{self.tag}\n").json() + self.assertEqual(res["status"], "ok", res) + return res + + def test_uploader_cannot_overwrite_an_existing_dataset(self): + name = f"gate_{self.tag}.csv" + self._dynamic(name) + res = self._post("uploader", fileName=name, content=f"k\n{self.tag}_new\n", overwrite=True) + self.assertEqual(res.status_code, 403, res.text) + self.assertIn(name, self._file_names(), "the refused overwrite must leave the dataset in place") + + def test_uploader_cannot_switch_mode_over_an_existing_dataset(self): + name = f"switch_{self.tag}.csv" + self._dynamic(name) + res = self._post("uploader", fileName=name, mode="te", targetTable="organisations", + content=f"name,org_type,country\nORG_{self.tag},government,AU\n") + self.assertEqual(res.status_code, 403, res.text) + self.assertIn(name, self._file_names(), "the refused mode switch must leave the dataset in place") + + def test_tenant_admin_may_overwrite(self): + name = f"admin_{self.tag}.csv" + self._dynamic(name) + res = self._post("tenant_admin", fileName=name, content=f"k\n{self.tag}_new\n", overwrite=True) + self.assertEqual(res.status_code, 200, res.text) + self.assertEqual(res.json()["status"], "ok", res.text) + + def test_uploader_can_still_upload_a_new_file(self): + res = self._post("uploader", fileName=f"fresh_{self.tag}.csv", content=f"k\n{self.tag}\n") + self.assertEqual((res.status_code, res.json()["status"]), (200, "ok"), res.text) + + +@pytest.mark.integration +class TeDatabaseErrorsNameTheSourceRow(UploadPaths): + """#189: a row that fails in the database is reported by its CSV row, not its place among survivors.""" + + def test_type_failure_then_foreign_key_failure_report_rows_1_and_2(self): + content = _csv([ + ["org_id", "program_code", "program_name", "start_date"], + [str(uuid.uuid4()), f"P1_{self.tag}", "bad date", "not-a-date"], # row 1: fails the type check + [str(uuid.uuid4()), f"P2_{self.tag}", "no such org", "2026-01-01"], # row 2: fails the foreign key + ]) + res = self._post("tenant_admin", fileName=f"prog_{self.tag}.csv", mode="te", + targetTable="test_programs", content=content).json() + by_row = {e["rowNumber"]: e for e in res["rowErrors"]} + self.assertEqual(sorted(by_row), [1, 2], res) + self.assertEqual(by_row[1].get("column"), "start_date") + self.assertNotIn("column", by_row[2], "row 2 failed in the database, not the type check") + self.assertIn("foreign key", by_row[2]["reason"].lower()) From 09e429cd75c0c9b6a1351af1e676be2544c58eae Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:08:17 +0000 Subject: [PATCH 2/9] test: regression tests for #130, #169, #171, #180, #181, #182, #194, #207 Shell blocks are cut out of the real scripts and run (setup.sh config write, run_tests.sh failure parse, build_bundle.sh download step), not re-implemented. Re-introducing each original bug fails its test. #181's second half (Tier S psql calls without ON_ERROR_STOP) is recorded as a strict xfail: it is not on main. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- docs/TEST_CASES.md | 146 ++++++++++++++++++- tests/test_tooling_issues.py | 264 +++++++++++++++++++++++++++++++++++ 2 files changed, 409 insertions(+), 1 deletion(-) create mode 100644 tests/test_tooling_issues.py diff --git a/docs/TEST_CASES.md b/docs/TEST_CASES.md index 9dd0768..b6fdefd 100644 --- a/docs/TEST_CASES.md +++ b/docs/TEST_CASES.md @@ -1,7 +1,7 @@ # Test Catalogue > Auto-generated by `scripts/test_catalogue.py` — do not edit by hand. -> 732 tests across 38 files. +> 800 tests across 40 files. To regenerate: `python3 scripts/test_catalogue.py` @@ -961,6 +961,62 @@ To regenerate: `python3 scripts/test_catalogue.py` | `test_counts_are_per_batch_not_cumulative` | — | A second, larger upload must report its own totals, not a running sum. | +## `tests/test_issue_regressions.py` + +| Test | Markers | Objective | +|---|---|---| +| `RateLimiterIdentity.test_identity_does_not_depend_on_the_api_key` | unit | — | + +| `RateLimiterIdentity.test_identity_is_the_peer_address` | unit | — | + +| `RateLimiterIdentity.test_rotating_invalid_keys_from_one_client_share_one_budget` | unit | — | + +| `RowDedupKeepsDistinctRows.test_the_reported_unit_separator_case_keeps_both_rows` | integration | — | + +| `RowDedupKeepsDistinctRows.test_every_control_character_stays_distinct_as_a_delimiter_candidate` | integration | — | + +| `RowDedupKeepsDistinctRows.test_unicode_empty_quote_comma_and_newline_cells_stay_distinct` | integration | — | + +| `RowDedupKeepsDistinctRows.test_a_genuinely_repeated_row_is_still_skipped` | integration | — | + +| `RowDedupKeepsDistinctRows.test_stored_hashes_carry_the_version_marker` | integration | — | + +| `UploadCannotBypassTheDeleteGate.test_uploader_cannot_overwrite_an_existing_dataset` | integration | — | + +| `UploadCannotBypassTheDeleteGate.test_uploader_cannot_switch_mode_over_an_existing_dataset` | integration | — | + +| `UploadCannotBypassTheDeleteGate.test_tenant_admin_may_overwrite` | integration | — | + +| `UploadCannotBypassTheDeleteGate.test_uploader_can_still_upload_a_new_file` | integration | — | + +| `TeDatabaseErrorsNameTheSourceRow.test_type_failure_then_foreign_key_failure_report_rows_1_and_2` | integration | — | + +| `test_identity_does_not_depend_on_the_api_key` | — | — | + +| `test_identity_is_the_peer_address` | — | — | + +| `test_rotating_invalid_keys_from_one_client_share_one_budget` | — | — | + +| `test_the_reported_unit_separator_case_keeps_both_rows` | — | — | + +| `test_every_control_character_stays_distinct_as_a_delimiter_candidate` | — | — | + +| `test_unicode_empty_quote_comma_and_newline_cells_stay_distinct` | — | — | + +| `test_a_genuinely_repeated_row_is_still_skipped` | — | — | + +| `test_stored_hashes_carry_the_version_marker` | — | — | + +| `test_uploader_cannot_overwrite_an_existing_dataset` | — | — | + +| `test_uploader_cannot_switch_mode_over_an_existing_dataset` | — | — | + +| `test_tenant_admin_may_overwrite` | — | — | + +| `test_uploader_can_still_upload_a_new_file` | — | — | + +| `test_type_failure_then_foreign_key_failure_report_rows_1_and_2` | — | — | + ## `tests/test_migrations.py` | Test | Markers | Objective | @@ -1621,3 +1677,91 @@ To regenerate: `python3 scripts/test_catalogue.py` | `test_provisioning_is_idempotent` | — | — | +## `tests/test_tooling_issues.py` + +| Test | Markers | Objective | +|---|---|---| +| `SetupKeepsPriorConfigOnFailure.test_the_block_was_found_and_uses_many_variables` | unit | — | + +| `SetupKeepsPriorConfigOnFailure.test_success_replaces_the_config_and_leaves_no_temp_file` | unit | — | + +| `SetupKeepsPriorConfigOnFailure.test_an_unset_variable_fails_the_run_but_keeps_the_prior_config` | unit | — | + +| `EvalRunnerStopsOnSqlErrors.test_the_idempotency_eval_stops_at_the_first_sql_error` | unit | — | + +| `EvalRunnerStopsOnSqlErrors.test_the_tier_s_deploy_and_suite_calls_stop_at_the_first_sql_error` | unit | — | + +| `SqlWrapperFailsOnAssertionFailures.test_failed_assertions_mark_the_environment_failed` | unit | — | + +| `SqlWrapperFailsOnAssertionFailures.test_a_clean_run_is_not_marked_failed` | unit | — | + +| `OfflineBundleStopsOnDownloadFailure.test_the_old_swallow_is_gone` | unit | — | + +| `OfflineBundleStopsOnDownloadFailure.test_a_failed_download_stops_the_build` | unit | — | + +| `OfflineBundleStopsOnDownloadFailure.test_a_download_that_yields_no_packages_stops_the_build` | unit | — | + +| `OfflineBundleStopsOnDownloadFailure.test_a_complete_download_continues` | unit | — | + +| `FrontendLockFileMatchesPackageJson.test_root_entry_and_every_direct_dependency_agree` | unit | — | + +| `TestCatalogueIsCurrent.test_docs_test_cases_matches_the_tests` | unit | — | + +| `CoverageFloorGate.test_floors_cover_the_three_loaders` | unit | — | + +| `CoverageFloorGate.test_passes_at_100_percent` | unit | — | + +| `CoverageFloorGate.test_fails_when_one_loader_drops_below_its_floor` | unit | — | + +| `CoverageFloorGate.test_fails_when_a_loader_is_missing_from_the_report` | unit | — | + +| `ApiDocDefersToTheGeneratedSchema.test_no_hand_kept_route_table` | unit | — | + +| `ApiDocDefersToTheGeneratedSchema.test_points_at_the_real_schema_urls_not_the_old_404` | unit | — | + +| `ApiDocDefersToTheGeneratedSchema.test_the_documented_urls_are_served_without_a_key` | unit | — | + +| `ApiDocDefersToTheGeneratedSchema.test_the_routers_it_names_exist` | unit | — | + +| `test_the_block_was_found_and_uses_many_variables` | — | — | + +| `test_success_replaces_the_config_and_leaves_no_temp_file` | — | — | + +| `test_an_unset_variable_fails_the_run_but_keeps_the_prior_config` | — | — | + +| `test_the_idempotency_eval_stops_at_the_first_sql_error` | — | — | + +| `test_the_tier_s_deploy_and_suite_calls_stop_at_the_first_sql_error` | — | — | + +| `test_failed_assertions_mark_the_environment_failed` | — | — | + +| `test_a_clean_run_is_not_marked_failed` | — | — | + +| `test_the_old_swallow_is_gone` | — | — | + +| `test_a_failed_download_stops_the_build` | — | — | + +| `test_a_download_that_yields_no_packages_stops_the_build` | — | — | + +| `test_a_complete_download_continues` | — | — | + +| `test_root_entry_and_every_direct_dependency_agree` | — | — | + +| `test_docs_test_cases_matches_the_tests` | — | — | + +| `test_floors_cover_the_three_loaders` | — | — | + +| `test_passes_at_100_percent` | — | — | + +| `test_fails_when_one_loader_drops_below_its_floor` | — | — | + +| `test_fails_when_a_loader_is_missing_from_the_report` | — | — | + +| `test_no_hand_kept_route_table` | — | — | + +| `test_points_at_the_real_schema_urls_not_the_old_404` | — | — | + +| `test_the_documented_urls_are_served_without_a_key` | — | — | + +| `test_the_routers_it_names_exist` | — | — | + diff --git a/tests/test_tooling_issues.py b/tests/test_tooling_issues.py new file mode 100644 index 0000000..65442ca --- /dev/null +++ b/tests/test_tooling_issues.py @@ -0,0 +1,264 @@ +"""Regression tests for closed tooling, CI and docs issues. + + #171 setup.sh must not destroy the prior config.local.env when writing a new one fails + #180 every psql call in the evals runner stops at the first SQL error + #181 the SQL test wrapper must fail an environment whose assertions failed + #182 the offline bundle build must stop when a package download fails + #130 the frontend lock file must match package.json + #169 docs/TEST_CASES.md must be up to date with the tests + #194 the coverage-floor gate must fail below a floor + #207 API_INTEGRATION.md must defer to the generated schema, and the schema must be served + +No database needed. Shell blocks are cut out of the real scripts and run, not re-implemented here. +""" + +from __future__ import annotations + +import ast +import json +import os +import re +import subprocess +import sys +import tempfile +import textwrap +import unittest +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent + + +def _between(text: str, start: str, end: str, *, include_end: bool = False) -> str: + i = text.index(start) + j = text.index(end, i) + return text[i:j + (len(end) if include_end else 0)] + + +def _bash(script: str, **env: str) -> subprocess.CompletedProcess: + return subprocess.run(["bash", "-c", script], capture_output=True, text=True, + env={**os.environ, **env}, timeout=60) + + +# ── #171 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class SetupKeepsPriorConfigOnFailure(unittest.TestCase): + """#171: the config is written to a temp file and moved into place only once complete.""" + + BLOCK = _between((ROOT / "build/setup.sh").read_text(encoding="utf-8"), + '_config_tmp="${CONFIG_LOCAL}.new"', 'mv -f "$_config_tmp" "$CONFIG_LOCAL"', include_end=True) + VARS = sorted(set(re.findall(r"\$\{([A-Z][A-Z0-9_]*)\}", BLOCK)) - {"CONFIG_LOCAL"}) + + def _run(self, omit: str | None): + with tempfile.TemporaryDirectory() as d: + config = Path(d) / "config.local.env" + config.write_text("PRIOR=keep-me\n") + exports = "".join(f'{v}=x; ' for v in self.VARS if v != omit) + script = f'set -euo pipefail; error(){{ echo "$@" >&2; }}; CONFIG_LOCAL="{config}"; {exports}\n{self.BLOCK}\n' + res = _bash(script) + return res, config.read_text(), sorted(p.name for p in Path(d).iterdir()) + + def test_the_block_was_found_and_uses_many_variables(self): + self.assertGreater(len(self.VARS), 30) + + def test_success_replaces_the_config_and_leaves_no_temp_file(self): + res, content, files = self._run(omit=None) + self.assertEqual(res.returncode, 0, res.stderr) + self.assertNotIn("PRIOR=keep-me", content) + self.assertIn("PG_PORT=x", content) + self.assertEqual(files, ["config.local.env"]) + + def test_an_unset_variable_fails_the_run_but_keeps_the_prior_config(self): + res, content, files = self._run(omit="PG_PORT") + self.assertNotEqual(res.returncode, 0) + self.assertEqual(content, "PRIOR=keep-me\n") + self.assertEqual(files, ["config.local.env"], "the temp file must be cleaned up") + + +# ── #180 ───────────────────────────────────────────────────────────────────── + +def _psql_calls(function: str | None = None) -> list[str]: + tree = ast.parse((ROOT / "evals/runner.py").read_text(encoding="utf-8")) + scope = [n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef) and n.name == function] if function else [tree] + return [ast.unparse(n) for root in scope for n in ast.walk(root) + if isinstance(n, ast.List) and n.elts and isinstance(n.elts[0], ast.Constant) and n.elts[0].value == "psql"] + + +@pytest.mark.unit +class EvalRunnerStopsOnSqlErrors(unittest.TestCase): + def test_the_idempotency_eval_stops_at_the_first_sql_error(self): + calls = _psql_calls("_run_deploy_dev_twice") # #180 + self.assertTrue(calls, "the idempotency eval's psql call was not found") + for call in calls: + self.assertIn("ON_ERROR_STOP=1", call) + + @pytest.mark.xfail(strict=True, reason=( + "#181 second half, not on main: the Tier S deploy and SQL-suite psql calls in evals/runner.py " + "have no ON_ERROR_STOP, so a SQL error mid-suite is not an error exit. Fix is on the unmerged " + "branch fix/181-sql-suite-exit-code.")) + def test_the_tier_s_deploy_and_suite_calls_stop_at_the_first_sql_error(self): + calls = [c for c in _psql_calls() if "env_dev_sql" in c and "-v" not in c or "run_tests" in c] + self.assertEqual(len(calls), 2, calls) + for call in calls: + self.assertIn("ON_ERROR_STOP=1", call) + + +# ── #181 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class SqlWrapperFailsOnAssertionFailures(unittest.TestCase): + """#181: psql exits 0 even when assertions record FAIL rows; the wrapper must read the output.""" + + BLOCK = _between((ROOT / "tests/run_tests.sh").read_text(encoding="utf-8"), + "if grep -qiE 'FAIL|TEST\\(S\\) FAILED'", 'rm -f "${outfile}"') + + def _failed_envs(self, output: str) -> str: + with tempfile.NamedTemporaryFile("w", suffix=".out", delete=False) as f: + f.write(output) + try: + # The block uses `local`, as it does inside run_suite(). + script = ('set -euo pipefail; error(){ :; }; FAILED_ENVS=(); check(){ local env=dev outfile="%s"\n%s\n}; ' + 'check; echo "failed:${FAILED_ENVS[*]:-}"' % (f.name, self.BLOCK)) + res = _bash(script) + self.assertEqual(res.returncode, 0, res.stderr) + return res.stdout.strip() + finally: + os.unlink(f.name) + + def test_failed_assertions_mark_the_environment_failed(self): + self.assertEqual(self._failed_envs("ok 1\nFAIL test_x\n2 TEST(S) FAILED\n"), "failed:dev") + + def test_a_clean_run_is_not_marked_failed(self): + self.assertEqual(self._failed_envs("142 passed\nALL TESTS PASSED\n"), "failed:") + + +# ── #182 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class OfflineBundleStopsOnDownloadFailure(unittest.TestCase): + SCRIPT = (ROOT / "scripts/offline/build_bundle.sh").read_text(encoding="utf-8") + BLOCK = _between(SCRIPT, "REAL=()", "( cd \"$OUT/debs\" && apt-ftparchive", include_end=False) + + def _run(self, apt_get_body: str) -> subprocess.CompletedProcess: + with tempfile.TemporaryDirectory() as d: + bin_dir, out = Path(d) / "bin", Path(d) / "out" + (out / "debs").mkdir(parents=True) + bin_dir.mkdir() + (bin_dir / "apt-cache").write_text("#!/bin/bash\necho 'Candidate: 1.0'\n") + (bin_dir / "apt-get").write_text("#!/bin/bash\n" + apt_get_body) + for f in bin_dir.iterdir(): + f.chmod(0o755) + script = (f'set -euo pipefail; DEPS="pkg-a pkg-b"; PKGS="pkg-a pkg-b"; OUT="{out}"\n{self.BLOCK}\necho BUILT') + return _bash(script, PATH=f"{bin_dir}:{os.environ['PATH']}") + + def test_the_old_swallow_is_gone(self): + self.assertNotRegex(self.SCRIPT, r"apt-get download[^\n]*\|\|\s*true") + + def test_a_failed_download_stops_the_build(self): + res = self._run("exit 1\n") + self.assertNotEqual(res.returncode, 0) + self.assertNotIn("BUILT", res.stdout) + + def test_a_download_that_yields_no_packages_stops_the_build(self): + res = self._run("exit 0\n") # claims success but fetched nothing + self.assertNotEqual(res.returncode, 0) + self.assertIn("missing deb", res.stderr) + + def test_a_complete_download_continues(self): + res = self._run('for p in "$@"; do [ "$p" = download ] || touch "${p}_1.0_amd64.deb"; done\n') + self.assertEqual(res.returncode, 0, res.stderr) + self.assertIn("BUILT", res.stdout) + + +# ── #130 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class FrontendLockFileMatchesPackageJson(unittest.TestCase): + """#130: `npm ci` refuses a lock file that disagrees with package.json.""" + + def test_root_entry_and_every_direct_dependency_agree(self): + pkg = json.loads((ROOT / "csv-table-hub-main/package.json").read_text(encoding="utf-8")) + lock = json.loads((ROOT / "csv-table-hub-main/package-lock.json").read_text(encoding="utf-8")) + root = lock["packages"][""] + for section in ("dependencies", "devDependencies"): + with self.subTest(section=section): + self.assertEqual(root.get(section, {}), pkg.get(section, {})) + for name in {**pkg.get("dependencies", {}), **pkg.get("devDependencies", {})}: + with self.subTest(package=name): + self.assertIn(f"node_modules/{name}", lock["packages"]) + + +# ── #169 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class TestCatalogueIsCurrent(unittest.TestCase): + def test_docs_test_cases_matches_the_tests(self): + res = subprocess.run([sys.executable, "scripts/test_catalogue.py", "--check"], cwd=ROOT, + capture_output=True, text=True, timeout=120) + self.assertEqual(res.returncode, 0, "run `python3 scripts/test_catalogue.py` and commit:\n" + res.stdout[-800:]) + + +# ── #194 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class CoverageFloorGate(unittest.TestCase): + FILES = ("api/services/csv_parse.py", "api/services/dynamic_loader.py", "api/services/te_loader.py") + + def _gate(self, pct: dict[str, float], drop: str | None = None) -> int: + report = {"files": {f: {"summary": {"percent_covered": pct[f]}} for f in self.FILES if f != drop}} + with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as f: + json.dump(report, f) + try: + return subprocess.run([sys.executable, "scripts/check_coverage_floors.py", f.name], cwd=ROOT, + capture_output=True, text=True, timeout=30).returncode + finally: + os.unlink(f.name) + + def test_floors_cover_the_three_loaders(self): + sys.path.insert(0, str(ROOT / "scripts")) + try: + import check_coverage_floors as gate + finally: + sys.path.pop(0) + self.assertEqual(set(gate.FLOORS), set(self.FILES)) + + def test_passes_at_100_percent(self): + self.assertEqual(self._gate({f: 100.0 for f in self.FILES}), 0) + + def test_fails_when_one_loader_drops_below_its_floor(self): + self.assertEqual(self._gate({**{f: 100.0 for f in self.FILES}, "api/services/te_loader.py": 10.0}), 1) + + def test_fails_when_a_loader_is_missing_from_the_report(self): + self.assertEqual(self._gate({f: 100.0 for f in self.FILES}, drop="api/services/dynamic_loader.py"), 1) + + +# ── #207 ───────────────────────────────────────────────────────────────────── + +@pytest.mark.unit +class ApiDocDefersToTheGeneratedSchema(unittest.TestCase): + DOC = (ROOT / "docs/API_INTEGRATION.md").read_text(encoding="utf-8") + + def test_no_hand_kept_route_table(self): + self.assertIsNone(re.search(r"^\|\s*`?(GET|POST|PUT|DELETE)\b", self.DOC, re.M), + "a hand-kept route table drifts from the code (#207)") + + def test_points_at_the_real_schema_urls_not_the_old_404(self): + self.assertIn("/docs", self.DOC) + self.assertIn("/openapi.json", self.DOC) + self.assertNotIn("/api/docs", self.DOC) + + def test_the_documented_urls_are_served_without_a_key(self): + from fastapi.testclient import TestClient + from api.main import app + client = TestClient(app) + self.assertEqual(client.get("/docs").status_code, 200) + paths = client.get("/openapi.json").json()["paths"] + self.assertIn("/api/csv/upload", paths) + self.assertIn("/api/csv/uploads/{upload_id}/rows", paths) + + def test_the_routers_it_names_exist(self): + for name in ("auth", "csv", "review", "te", "audit"): + with self.subTest(router=name): + self.assertTrue((ROOT / "api/routers" / f"{name}_routes.py").exists()) From b211843c05c146e823d6b2c5f7c36233c8893087 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:11:06 +0000 Subject: [PATCH 3/9] test: OIDC https JWKS path (#113), SECURITY.md (#57), offline install scripts (#60) #113: a local TLS server with a throwaway certificate exercises the real PyJWKClient https fetch, key rotation past the refresh cooldown, no re-fetch inside it, and refusal of an untrusted certificate. A sign-in against the real IdP is still the manual step. #57: SECURITY.md is on main with a reporting channel, which is the issue's done-when. #60: the build and install scripts parse and stop on error and the guide names them; the dry run itself stays manual. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- docs/TEST_CASES.md | 62 ++++++++++++- tests/test_oidc_https_jwks.py | 158 ++++++++++++++++++++++++++++++++++ tests/test_open_issues.py | 69 +++++++++++++++ 3 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 tests/test_oidc_https_jwks.py create mode 100644 tests/test_open_issues.py diff --git a/docs/TEST_CASES.md b/docs/TEST_CASES.md index b6fdefd..2d79be2 100644 --- a/docs/TEST_CASES.md +++ b/docs/TEST_CASES.md @@ -1,7 +1,7 @@ # Test Catalogue > Auto-generated by `scripts/test_catalogue.py` — do not edit by hand. -> 800 tests across 40 files. +> 826 tests across 42 files. To regenerate: `python3 scripts/test_catalogue.py` @@ -1097,6 +1097,66 @@ To regenerate: `python3 scripts/test_catalogue.py` | `test_missing_audience_or_insecure_jwks_url_refuses_to_start` | — | — | +## `tests/test_oidc_https_jwks.py` + +| Test | Markers | Objective | +|---|---|---| +| `HttpsJwksFetch.test_a_token_signed_by_a_key_fetched_over_https_is_accepted` | unit | — | + +| `HttpsJwksFetch.test_a_token_signed_by_an_unpublished_key_is_rejected` | unit | — | + +| `HttpsJwksFetch.test_a_rotated_key_is_picked_up_without_a_restart` | unit | — | + +| `HttpsJwksFetch.test_unknown_key_ids_cannot_make_the_api_hammer_the_idp` | unit | — | + +| `HttpsJwksFetch.test_a_certificate_that_is_not_trusted_is_refused` | unit | — | + +| `test_a_token_signed_by_a_key_fetched_over_https_is_accepted` | — | — | + +| `test_a_token_signed_by_an_unpublished_key_is_rejected` | — | — | + +| `test_a_rotated_key_is_picked_up_without_a_restart` | — | — | + +| `test_unknown_key_ids_cannot_make_the_api_hammer_the_idp` | — | — | + +| `test_a_certificate_that_is_not_trusted_is_refused` | — | — | + +## `tests/test_open_issues.py` + +| Test | Markers | Objective | +|---|---|---| +| `SecurityPolicyIsPublished.test_says_how_to_report_privately` | unit | — | + +| `SecurityPolicyIsPublished.test_names_a_contact_address` | unit | — | + +| `SecurityPolicyIsPublished.test_says_which_versions_are_supported` | unit | — | + +| `OfflineInstallProcedureIsConsistent.test_the_build_and_install_scripts_exist` | unit | — | + +| `OfflineInstallProcedureIsConsistent.test_every_script_parses` | unit | — | + +| `OfflineInstallProcedureIsConsistent.test_the_build_and_install_scripts_stop_on_the_first_error` | unit | — | + +| `OfflineInstallProcedureIsConsistent.test_the_guide_names_the_scripts_it_relies_on` | unit | — | + +| `OfflineInstallProcedureIsConsistent.test_the_guide_gives_the_checksum_step` | unit | — | + +| `test_says_how_to_report_privately` | — | — | + +| `test_names_a_contact_address` | — | — | + +| `test_says_which_versions_are_supported` | — | — | + +| `test_the_build_and_install_scripts_exist` | — | — | + +| `test_every_script_parses` | — | — | + +| `test_the_build_and_install_scripts_stop_on_the_first_error` | — | — | + +| `test_the_guide_names_the_scripts_it_relies_on` | — | — | + +| `test_the_guide_gives_the_checksum_step` | — | — | + ## `tests/test_original_bytes.py` | Test | Markers | Objective | diff --git a/tests/test_oidc_https_jwks.py b/tests/test_oidc_https_jwks.py new file mode 100644 index 0000000..41c739b --- /dev/null +++ b/tests/test_oidc_https_jwks.py @@ -0,0 +1,158 @@ +"""OIDC: the https:// JWKS fetch path (#113, the part testable without the real IdP). + +#95 was tested with a file:// JWKS only, so PyJWT's PyJWKClient (the https:// path a +deployment uses) was never run. Here the IdP is a local TLS server with a throwaway +self-signed certificate, so the real fetch, the TLS check and key rotation all run. + +What this cannot show is a sign-in against the deployment's own Keycloak or Entra ID: +#113 stays open for that one manual run. +""" + +from __future__ import annotations + +import datetime +import http.server +import ipaddress +import json +import os +import ssl +import tempfile +import threading +import time +import unittest +from pathlib import Path + +import jwt +import pytest +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509.oid import NameOID + +from api import oidc +from api.config import settings + +_ISS, _AUD = "https://idp.example.test/realms/te", "te-migration" + + +def _rsa(): + return rsa.generate_private_key(public_exponent=65537, key_size=2048) + + +def _jwk(private_key, kid: str) -> dict: + jwk = json.loads(jwt.algorithms.RSAAlgorithm.to_jwk(private_key.public_key())) + jwk.update({"kid": kid, "use": "sig", "alg": "RS256"}) + return jwk + + +def _self_signed(directory: Path) -> tuple[Path, Path]: + key = _rsa() + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "localhost")]) + now = datetime.datetime.now(datetime.timezone.utc) + cert = (x509.CertificateBuilder().subject_name(name).issuer_name(name).public_key(key.public_key()) + .serial_number(x509.random_serial_number()).not_valid_before(now - datetime.timedelta(minutes=5)) + .not_valid_after(now + datetime.timedelta(days=1)) + .add_extension(x509.SubjectAlternativeName( + [x509.DNSName("localhost"), x509.IPAddress(ipaddress.ip_address("127.0.0.1"))]), critical=False) + .sign(key, hashes.SHA256())) + cert_path, key_path = directory / "cert.pem", directory / "key.pem" + cert_path.write_bytes(cert.public_bytes(serialization.Encoding.PEM)) + key_path.write_bytes(key.private_bytes(serialization.Encoding.PEM, serialization.PrivateFormat.TraditionalOpenSSL, + serialization.NoEncryption())) + return cert_path, key_path + + +@pytest.mark.unit +class HttpsJwksFetch(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls._dir = tempfile.TemporaryDirectory() + cert, key = _self_signed(Path(cls._dir.name)) + cls.keys = {"key-1": _rsa()} + cls.hits = [] + + outer = cls + + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + outer.hits.append(self.path) + body = json.dumps({"keys": [_jwk(k, kid) for kid, k in outer.keys.items()]}).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): # keep test output quiet + pass + + cls.server = http.server.HTTPServer(("127.0.0.1", 0), Handler) + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + ctx.load_cert_chain(cert, key) + cls.server.socket = ctx.wrap_socket(cls.server.socket, server_side=True) + threading.Thread(target=cls.server.serve_forever, daemon=True).start() + + cls._env = {k: os.environ.get(k) for k in ("SSL_CERT_FILE", "NO_PROXY", "no_proxy")} + os.environ["SSL_CERT_FILE"] = str(cert) # trust the throwaway certificate, and only it + os.environ["NO_PROXY"] = os.environ["no_proxy"] = "127.0.0.1,localhost" + cls._orig = (settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL) + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE = _ISS, _AUD + settings.OIDC_JWKS_URL = f"https://127.0.0.1:{cls.server.server_port}/jwks" + + @classmethod + def tearDownClass(cls): + cls.server.shutdown() + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL = cls._orig + for k, v in cls._env.items(): + os.environ.pop(k, None) if v is None else os.environ.__setitem__(k, v) + oidc._jwks.cache_clear() + cls._dir.cleanup() + + def setUp(self): + oidc._jwks.cache_clear() + self.hits.clear() + + @staticmethod + def _token(key, kid: str, username: str = "alice") -> str: + now = int(time.time()) + claims = {"iss": _ISS, "aud": _AUD, "iat": now, "exp": now + 300, "sub": "s", "preferred_username": username} + return jwt.encode(claims, key, algorithm="RS256", headers={"kid": kid}) + + def test_a_token_signed_by_a_key_fetched_over_https_is_accepted(self): + self.assertEqual(oidc.username_from(self._token(self.keys["key-1"], "key-1")), "alice") + self.assertEqual(self.hits, ["/jwks"], "the key must come from the https endpoint") + + def test_a_token_signed_by_an_unpublished_key_is_rejected(self): + self.assertIsNone(oidc.username_from(self._token(_rsa(), "key-1"))) + + def _rotate_to_a_new_key(self, cooldown: float, username: str): + self.assertEqual(oidc.username_from(self._token(self.keys["key-1"], "key-1")), "alice") + oidc._jwks(settings.OIDC_JWKS_URL).cooldown_duration = cooldown + self.keys["key-2"] = _rsa() # published at the IdP after the first fetch + try: + return oidc.username_from(self._token(self.keys["key-2"], "key-2", username)) + finally: + self.keys.pop("key-2", None) + + def test_a_rotated_key_is_picked_up_without_a_restart(self): + # Past PyJWT's refresh cooldown, an unknown key id triggers a re-fetch. + self.assertEqual(self._rotate_to_a_new_key(cooldown=0, username="bob"), "bob") + self.assertEqual(self.hits, ["/jwks", "/jwks"]) + + def test_unknown_key_ids_cannot_make_the_api_hammer_the_idp(self): + # Inside the cooldown (PyJWT's default is 30 s) an unknown key id is refused, not re-fetched: + # a stream of forged tokens must not become a stream of requests to the IdP. + self.assertIsNone(self._rotate_to_a_new_key(cooldown=30, username="bob")) + self.assertEqual(self.hits, ["/jwks"]) + + def test_a_certificate_that_is_not_trusted_is_refused(self): + with tempfile.TemporaryDirectory() as other: + stranger, _ = _self_signed(Path(other)) # trusted instead of the server's own certificate + trusted = os.environ["SSL_CERT_FILE"] + os.environ["SSL_CERT_FILE"] = str(stranger) + try: + oidc._jwks.cache_clear() + self.assertIsNone(oidc.username_from(self._token(self.keys["key-1"], "key-1")), + "a JWKS endpoint whose certificate isn't trusted must not yield a session") + finally: + os.environ["SSL_CERT_FILE"] = trusted diff --git a/tests/test_open_issues.py b/tests/test_open_issues.py new file mode 100644 index 0000000..e6baf3c --- /dev/null +++ b/tests/test_open_issues.py @@ -0,0 +1,69 @@ +"""Checks for open issues that can be verified from the repository alone. + +Each class says how far it goes. Nothing here closes an issue whose acceptance step is +manual or needs another system. + + #57 SECURITY.md is on main with a reporting channel + #60 the air-gapped install procedure: scripts parse and the guide names them + (the real offline dry run, due 15 Nov, is manual) +""" + +from __future__ import annotations + +import re +import subprocess +import unittest +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent + + +@pytest.mark.unit +class SecurityPolicyIsPublished(unittest.TestCase): + """#57: 'SECURITY.md is on main' is the issue's whole done-when.""" + + TEXT = (ROOT / "SECURITY.md").read_text(encoding="utf-8") + + def test_says_how_to_report_privately(self): + self.assertIn("## Reporting a Vulnerability", self.TEXT) + self.assertRegex(self.TEXT, r"(?i)do not open a public issue") + + def test_names_a_contact_address(self): + self.assertRegex(self.TEXT, r"\*\*Email:\*\*\s*[\w.+-]+@[\w-]+\.[\w.]+") + + def test_says_which_versions_are_supported(self): + self.assertIn("## Supported Versions", self.TEXT) + + +@pytest.mark.unit +class OfflineInstallProcedureIsConsistent(unittest.TestCase): + """#60, as far as it can be checked without the offline VM.""" + + SCRIPTS = sorted((ROOT / "scripts/offline").glob("*.sh")) + GUIDE = (ROOT / "docs/offline-install.md").read_text(encoding="utf-8") + + def test_the_build_and_install_scripts_exist(self): + self.assertTrue({"build_bundle.sh", "install.sh"} <= {p.name for p in self.SCRIPTS}) + + def test_every_script_parses(self): + for script in self.SCRIPTS: + with self.subTest(script=script.name): + res = subprocess.run(["bash", "-n", str(script)], capture_output=True, text=True) + self.assertEqual(res.returncode, 0, res.stderr) + + def test_the_build_and_install_scripts_stop_on_the_first_error(self): + # smoke_test.sh is excluded on purpose: it counts PASS/FAIL checks and exits non-zero at the end. + for name in ("build_bundle.sh", "install.sh"): + with self.subTest(script=name): + self.assertRegex((ROOT / "scripts/offline" / name).read_text(encoding="utf-8"), + r"(?m)^set -euo pipefail") + + def test_the_guide_names_the_scripts_it_relies_on(self): + for name in ("build_bundle.sh", "install.sh"): + with self.subTest(script=name): + self.assertIn(name, self.GUIDE) + + def test_the_guide_gives_the_checksum_step(self): + self.assertRegex(self.GUIDE, r"(?i)sha256") From e52c7f9276b4dd27ed60602f3d76ab27da37af56 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:20:40 +0000 Subject: [PATCH 4/9] fix(ui): accept the header_mapping log step, so imports no longer show as Failed #186 made the API return a header_mapping step in an upload's logs. The React app's response schema did not list it, and the React app sends a header mapping with every dynamic upload. zod rejected the whole response ("Unexpected response shape ... logs.3.step"), so an import that had succeeded on the server was shown as Failed. Add the step, export the schema, and test that it accepts every step the API's loaders can pass to _log(), so the next new step fails a test, not the upload screen. Found by the browser end-to-end tests. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- .../src/__tests__/log-step-contract.test.ts | 41 +++++++++++++++++++ csv-table-hub-main/src/lib/csv.functions.ts | 3 +- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 csv-table-hub-main/src/__tests__/log-step-contract.test.ts diff --git a/csv-table-hub-main/src/__tests__/log-step-contract.test.ts b/csv-table-hub-main/src/__tests__/log-step-contract.test.ts new file mode 100644 index 0000000..669532b --- /dev/null +++ b/csv-table-hub-main/src/__tests__/log-step-contract.test.ts @@ -0,0 +1,41 @@ +// The upload response is validated against a schema. A log step the API emits but the schema does not +// list makes zod reject the WHOLE response, so an import that succeeded on the server is reported as +// "Failed: Unexpected response shape" (what #186 did when it added the header_mapping step). + +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { processingLogSchema } from "@/lib/csv.functions"; + +const API = join(__dirname, "../../../api"); + +/** Every step name the loaders pass to _log(logs, "", ...). */ +function stepsTheApiEmits(): Set { + const steps = new Set(); + for (const dir of ["services", "routers"]) { + for (const file of readdirSync(join(API, dir)).filter((f) => f.endsWith(".py"))) { + const source = readFileSync(join(API, dir, file), "utf8"); + for (const m of source.matchAll(/_log\(\s*logs,\s*"([a-z_]+)"/g)) steps.add(m[1]); + } + } + return steps; +} + +describe("processing log steps", () => { + it("finds the steps the API emits (guards the scan itself)", () => { + const steps = stepsTheApiEmits(); + expect(steps.size).toBeGreaterThan(8); + expect(steps.has("receive")).toBe(true); + }); + + it("the schema accepts every step the API can emit", () => { + const known = new Set(processingLogSchema.shape.step.options); + const unknown = [...stepsTheApiEmits()].filter((s) => !known.has(s)); + expect(unknown).toEqual([]); + }); + + it("accepts the header_mapping entry a dynamic upload with a header mapping returns", () => { + const entry = { ts: 1, step: "header_mapping", level: "info", message: "Applied 2 header renames" }; + expect(processingLogSchema.safeParse(entry).success).toBe(true); + }); +}); diff --git a/csv-table-hub-main/src/lib/csv.functions.ts b/csv-table-hub-main/src/lib/csv.functions.ts index bd07654..6b65bec 100644 --- a/csv-table-hub-main/src/lib/csv.functions.ts +++ b/csv-table-hub-main/src/lib/csv.functions.ts @@ -13,13 +13,14 @@ const rowErrorSchema = z.object({ }); export type RowError = z.infer; -const processingLogSchema = z.object({ +export const processingLogSchema = z.object({ ts: z.number(), step: z.enum([ "receive", "hash", "duplicate_check", "parse", + "header_mapping", "validate_columns", "create_table", "cast_rows", From 439e4090fb813c85531d0fc437e3ac499a4f8a9f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:27:30 +0000 Subject: [PATCH 5/9] test: browser end-to-end tests (#176, #187, #188, #154) and the #176 leak test e2e/ drives the real API, the production build of the React app and the Streamlit spike in Chromium. #187: a failed upload is listed and opens its row errors. #188: no Stop once the request is on the server. #154: Streamlit role gating, edit with a required reason, exclude, history, and sign-out leaving nothing for the next tenant. #176 is a strict expected failure, in vitest and in the browser: the leak is live in the shipped app (a second tenant's history page lists the first tenant's file). The browser tests are not part of the default run. Found by these tests: every dynamic import showed as Failed (fixed in the separate header_mapping PR, merged here so the suite runs on it). Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- .../src/__tests__/tenant-isolation.test.tsx | 54 +++++ e2e/README.md | 24 +++ e2e/conftest.py | 190 ++++++++++++++++++ e2e/test_react_ui.py | 93 +++++++++ e2e/test_streamlit_spike.py | 89 ++++++++ 5 files changed, 450 insertions(+) create mode 100644 csv-table-hub-main/src/__tests__/tenant-isolation.test.tsx create mode 100644 e2e/README.md create mode 100644 e2e/conftest.py create mode 100644 e2e/test_react_ui.py create mode 100644 e2e/test_streamlit_spike.py diff --git a/csv-table-hub-main/src/__tests__/tenant-isolation.test.tsx b/csv-table-hub-main/src/__tests__/tenant-isolation.test.tsx new file mode 100644 index 0000000..dd40b1d --- /dev/null +++ b/csv-table-hub-main/src/__tests__/tenant-isolation.test.tsx @@ -0,0 +1,54 @@ +// #176 (BR-36): upload history is kept in localStorage under one global key, so the next person +// to sign in on the same browser sees the previous tenant's file names and rejected values. +// +// #176 was closed as moot because the Streamlit migration (#212) replaces this screen, but the +// React app still ships until then. `it.fails` keeps this suite green while the leak exists and +// turns it red the moment it is fixed, so the test is then switched to a plain `it`. + +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, expect, it } from "vitest"; +import { useLocalStorageState } from "@/hooks/use-local-storage"; +import { clearSession } from "@/lib/session"; + +type Job = { name: string }; + +function signInAs(tenant: string) { + sessionStorage.setItem( + "te.session", + JSON.stringify({ + token: `t-${tenant}`, + expiresAt: "2099-01-01T00:00:00Z", + username: `user-${tenant}`, + tenant, + role: "uploader", + }), + ); +} + +beforeEach(() => { + window.localStorage.clear(); + clearSession(); +}); + +it("keeps a tenant's own history across a reload (the feature)", () => { + signInAs("acme"); + const first = renderHook(() => useLocalStorageState([])); + act(() => first.result.current[1]([{ name: "acme-payroll.csv" }])); + first.unmount(); + + const reloaded = renderHook(() => useLocalStorageState([])); + expect(reloaded.result.current[0]).toEqual([{ name: "acme-payroll.csv" }]); +}); + +it.fails("#176: another tenant signing in on the same browser does not see the previous tenant's history", () => { + signInAs("acme"); + const acme = renderHook(() => useLocalStorageState([])); + act(() => acme.result.current[1]([{ name: "acme-payroll.csv" }])); + acme.unmount(); + + clearSession(); // sign out: only the session token is removed + signInAs("globex"); + const globex = renderHook(() => useLocalStorageState([])); + + expect(globex.result.current[0]).toEqual([]); +}); diff --git a/e2e/README.md b/e2e/README.md new file mode 100644 index 0000000..3199ceb --- /dev/null +++ b/e2e/README.md @@ -0,0 +1,24 @@ +# Browser end-to-end tests + +Playwright (Python) against the real API, the React app and the Streamlit spike, in Chromium. +They cover what the vitest and pytest suites cannot: #187 and #188 in a real browser, the live #176 +leak (an expected failure until it is fixed), and the #154 Streamlit review screen. + +Not part of the default `pytest` run (`testpaths = tests`) and not wired into CI: they need three +local servers and a browser, and #168 / PR #214 decide how browser tests join CI. + +## Run + +PostgreSQL must be reachable as for the integration tests (`PG*` environment variables), the dev +database deployed (`bash build/deploy_all.sh dev`), and ports 8000, 8080 and 8501 free. + +```bash +pip install playwright streamlit && playwright install chromium +(cd csv-table-hub-main && npm ci) +pytest e2e -p no:cacheprovider +``` + +To use a Chromium you already have, set `E2E_CHROMIUM=/path/to/chrome`. + +The fixture creates two tenants and four users, builds the React app (`NITRO_PRESET=node-server`, as the offline bundle does) and serves that production build, starts the API and Streamlit, and removes everything afterwards. +The dev server (`vite dev`) is not used: it double-runs effects and hides or invents bugs the shipped app does not have. diff --git a/e2e/conftest.py b/e2e/conftest.py new file mode 100644 index 0000000..269b457 --- /dev/null +++ b/e2e/conftest.py @@ -0,0 +1,190 @@ +"""Browser end-to-end tests: the real API, the real React app and the Streamlit spike, in Chromium. + +Not collected by the default `pytest` run (testpaths = tests): they need Node dependencies, a +browser and three local servers. See e2e/README.md. +""" + +from __future__ import annotations + +import os +import shutil +import signal +import socket +import subprocess +import sys +import time +import uuid +from dataclasses import dataclass, field +from pathlib import Path + +import pytest +import requests +from playwright.sync_api import Page, sync_playwright +from psycopg2 import sql + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT)) + +from api import db # noqa: E402 +from api.config import settings # noqa: E402 +from api.db import Conn # noqa: E402 +from api.services import accounts # noqa: E402 +from api.tenancy import migrate_tenant, schema_for # noqa: E402 + +API_PORT, WEB_PORT, ST_PORT = 8000, 8080, 8501 +PASSWORD = "e2e correct horse battery" +REMEDY = "stop whatever listens on it (or the previous run's servers) and rerun" + + +@dataclass +class Account: + tenant: str + username: str + role: str + + +@dataclass +class Stack: + api: str + web: str + streamlit: str + accounts: dict[str, Account] = field(default_factory=dict) + + def token(self, who: str) -> str: + a = self.accounts[who] + res = requests.post(f"{self.api}/api/auth/login", timeout=30, + json={"tenant": a.tenant, "username": a.username, "password": PASSWORD}) + res.raise_for_status() + return res.json()["token"] + + def upload(self, who: str, name: str, content: str, **extra) -> dict: + res = requests.post(f"{self.api}/api/csv/upload", timeout=60, + headers={"Authorization": f"Bearer {self.token(who)}"}, + json={"fileName": name, "content": content, **extra}) + res.raise_for_status() + return res.json() + + +def _port_is_free(port: int) -> bool: + with socket.socket() as s: + return s.connect_ex(("127.0.0.1", port)) != 0 + + +def _wait_for(url: str, what: str, seconds: int = 120) -> None: + deadline = time.time() + seconds + while time.time() < deadline: + try: + if requests.get(url, timeout=3).status_code < 500: + return + except requests.RequestException: + pass + time.sleep(1) + raise RuntimeError(f"{what} did not come up at {url} within {seconds}s") + + +@pytest.fixture(scope="session") +def stack(): + for port in (API_PORT, WEB_PORT, ST_PORT): + if not _port_is_free(port): + raise RuntimeError(f"port {port} is in use: {REMEDY}") + if not (ROOT / "csv-table-hub-main/node_modules").exists(): + raise RuntimeError("run `npm ci` in csv-table-hub-main first") + + db.init_pool() # as scripts/create_tenant_admin.py does, outside the API process + db.bootstrap() + tag = f"{os.getpid()}_{uuid.uuid4().hex[:5]}" + accts = { + "admin": Account(f"e2ea_{tag}", f"admin_{tag}", "tenant_admin"), + "uploader": Account(f"e2ea_{tag}", f"uploader_{tag}", "uploader"), + "reviewer": Account(f"e2ea_{tag}", f"reviewer_{tag}", "reviewer"), + "other": Account(f"e2eb_{tag}", f"other_{tag}", "tenant_admin"), + } + with Conn() as conn, conn.cursor() as cur: + tenant_ids = {t: accounts.create_tenant(cur, t, t) for t in {a.tenant for a in accts.values()}} + for a in accts.values(): + accounts.add_membership(cur, accounts.create_user(cur, a.username, PASSWORD), tenant_ids[a.tenant], a.role) + conn.commit() + for t in tenant_ids: + migrate_tenant(t) + + api, web, st = (f"http://127.0.0.1:{p}" for p in (API_PORT, WEB_PORT, ST_PORT)) + env = {**os.environ, "CORS_ORIGINS": web, "API_RATE_LIMIT_WRITES": "0", "API_RATE_LIMIT_READS": "0"} + subprocess.run(["npm", "run", "build"], cwd=ROOT / "csv-table-hub-main", check=True, capture_output=True, + env={**env, "NITRO_PRESET": "node-server", "VITE_API_BASE": api}) # VITE_API_BASE is baked in + procs = [ + subprocess.Popen([sys.executable, "-m", "uvicorn", "api.main:app", "--port", str(API_PORT)], + cwd=ROOT, env=env, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, start_new_session=True), + # The production build, served as the offline bundle serves it: what ships, not the dev server. + subprocess.Popen(["node", ".output/server/index.mjs"], cwd=ROOT / "csv-table-hub-main", + env={**env, "PORT": str(WEB_PORT), "HOST": "127.0.0.1"}, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, start_new_session=True), + subprocess.Popen([sys.executable, "-m", "streamlit", "run", "spikes/streamlit_review/app.py", + "--server.headless", "true", "--server.port", str(ST_PORT), + "--browser.gatherUsageStats", "false"], + cwd=ROOT, env={**env, "API_URL": api}, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, start_new_session=True), + ] + try: + _wait_for(f"{api}/api/health", "the API") + _wait_for(f"{web}/auth", "the React app") + _wait_for(f"{st}/_stcore/health", "Streamlit") + yield Stack(api, web, st, accts) + finally: + for p in procs: # each server leads its own process group, so npx's child dies with it + try: + os.killpg(p.pid, signal.SIGTERM) + except ProcessLookupError: + pass + for p in procs: + try: + p.wait(timeout=15) + except subprocess.TimeoutExpired: + os.killpg(p.pid, signal.SIGKILL) + with Conn() as conn, conn.cursor() as cur: + for t in tenant_ids: + cur.execute(sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format(sql.Identifier(schema_for(t)))) + cur.execute(sql.SQL("DELETE FROM {}.tenants WHERE slug = %s").format( + sql.Identifier(settings.CONTROL_SCHEMA)), (t,)) + cur.execute(sql.SQL("DELETE FROM {}.users WHERE username LIKE %s OR username LIKE %s OR username LIKE %s " + "OR username LIKE %s").format(sql.Identifier(settings.CONTROL_SCHEMA)), + tuple(f"{p}_{tag}" for p in ("admin", "uploader", "reviewer", "other"))) + conn.commit() + db.close_pool() + + +@pytest.fixture(scope="session") +def browser(): + with sync_playwright() as p: + exe = os.environ.get("E2E_CHROMIUM") or (shutil.which("chromium") if os.environ.get("E2E_SYSTEM_CHROMIUM") else None) + b = p.chromium.launch(executable_path=exe) if exe else p.chromium.launch() + yield b + b.close() + + +@pytest.fixture() +def page(browser) -> Page: + ctx = browser.new_context(viewport={"width": 1400, "height": 1000}) + ctx.set_default_timeout(20_000) + pg = ctx.new_page() + yield pg + ctx.close() + + +def react_sign_in(page: Page, stack: Stack, who: str) -> None: + a = stack.accounts[who] + page.goto(f"{stack.web}/auth") + page.get_by_label("Tenant").fill(a.tenant) + page.get_by_label("Username").fill(a.username) + page.get_by_label("Password").fill(PASSWORD) + page.get_by_role("button", name="Sign in").click() + page.get_by_role("button", name="Sign out").wait_for() + + +def streamlit_sign_in(page: Page, stack: Stack, who: str) -> None: + a = stack.accounts[who] + page.goto(stack.streamlit) + side = page.locator('[data-testid="stSidebar"]') + side.get_by_label("Tenant").fill(a.tenant) + side.get_by_label("Username").fill(a.username) + side.get_by_role("textbox", name="Password").fill(PASSWORD) + side.get_by_role("button", name="Sign in").click() + side.get_by_text(a.role, exact=True).wait_for() diff --git a/e2e/test_react_ui.py b/e2e/test_react_ui.py new file mode 100644 index 0000000..a010085 --- /dev/null +++ b/e2e/test_react_ui.py @@ -0,0 +1,93 @@ +"""React app in a real browser: #187, #188 and the live #176 leak.""" + +from __future__ import annotations + +import re +import uuid + +import pytest +from playwright.sync_api import expect + +from conftest import react_sign_in + +CSV = b"id,name\n1,a\n2,b\n" + + +def _tag() -> str: + return uuid.uuid4().hex[:8] + + +def _choose_file(page, name: str, content: bytes = CSV) -> None: + page.set_input_files('input[type="file"]', {"name": name, "mimeType": "text/csv", "buffer": content}) + page.get_by_role("button", name=re.compile("confirm & import", re.I)).click() + + +def test_187_a_failed_upload_is_reachable_and_opens_its_row_errors(page, stack): + name = f"failed_{_tag()}.csv" + res = stack.upload("admin", name, "n\nx\ny\n", types=["int8"]) # every row fails the type check + assert res["status"] == "error" and res["uploadId"], res + + react_sign_in(page, stack, "admin") + row = page.get_by_role("listitem").filter(has_text=name) + expect(row).to_be_visible() + expect(row.get_by_text("Failed", exact=True)).to_be_visible() + row.get_by_role("link", name="Review").click() + + expect(page).to_have_url(re.compile(r"/review/\d+$")) + expect(page.get_by_text("Invalid").first).to_be_visible() # the stored rows and their validation errors + + +def test_187_clean_imports_are_hidden_until_the_filter_is_turned_off(page, stack): + clean, partial = f"clean_{_tag()}.csv", f"partial_{_tag()}.csv" + stack.upload("admin", clean, "n\n1\n2\n", types=["int8"]) + stack.upload("admin", partial, "n\n1\nx\n", types=["int8"]) + + react_sign_in(page, stack, "admin") + partial_row = page.get_by_role("listitem").filter(has_text=partial) # not the migrated-files table below + expect(partial_row).to_be_visible() + expect(partial_row.get_by_text("Partly imported")).to_be_visible() + expect(page.get_by_role("listitem").filter(has_text=clean)).to_have_count(0) + page.get_by_label("Only failed or partial").uncheck() + expect(page.get_by_role("listitem").filter(has_text=clean)).to_be_visible() + + +def test_188_stop_is_not_offered_once_the_request_is_on_the_server(page, stack): + react_sign_in(page, stack, "admin") + held = [] + page.route("**/api/csv/upload", lambda route: held.append(route)) # the request stays pending until released + + _choose_file(page, f"slow_{_tag()}.csv") + + expect(page.get_by_text(re.compile("sent to the server", re.I))).to_be_visible() + expect(page.get_by_role("button", name="Stop")).to_have_count(0) + + held[0].continue_() + expect(page.get_by_text("Imported", exact=True)).to_be_visible() + expect(page.get_by_text("Cancelled", exact=True)).to_have_count(0) + + +@pytest.mark.xfail(strict=True, reason=( + "#176 is live: upload history is kept under one global localStorage key, so a second tenant on the same " + "browser sees the first one's file names. Closed as moot (the React screen is being replaced, #212), " + "but the app still ships. This test turns red when the leak is fixed: then drop the marker.")) +def test_176_the_next_tenant_does_not_see_the_previous_tenants_history(page, stack): + name = f"acme_private_{_tag()}.csv" + react_sign_in(page, stack, "admin") + _choose_file(page, name) + expect(page.get_by_text("Imported", exact=True)).to_be_visible() + + # Control: the history screen does list this tenant's own import, so "not listed" below means something. + page.goto(f"{stack.web}/audit") + expect(page.get_by_text(name)).to_be_visible() + expect(page.get_by_text("1 import · saved locally")).to_be_visible() + + page.goto(stack.web) + page.get_by_role("button", name="Sign out").click() + page.wait_for_url(re.compile("/auth")) + react_sign_in(page, stack, "other") + page.goto(f"{stack.web}/audit") + page.get_by_role("heading", name="Import history").wait_for() + page.wait_for_load_state("networkidle") + page.wait_for_timeout(1000) # a negative check needs the page to have loaded its stored history + expect(page.get_by_text(name)).to_have_count(0) + expect(page.get_by_text("0 imports · saved locally")).to_be_visible() diff --git a/e2e/test_streamlit_spike.py b/e2e/test_streamlit_spike.py new file mode 100644 index 0000000..2f0dc66 --- /dev/null +++ b/e2e/test_streamlit_spike.py @@ -0,0 +1,89 @@ +"""The Streamlit review screen from the #154 spike, in a real browser.""" + +from __future__ import annotations + +import re +import uuid + +import pytest +from playwright.sync_api import expect + +from conftest import streamlit_sign_in + +ROWS = "".join(f"{i},person{i},{'x' if i % 7 == 0 else i * 10}\n" for i in range(1, 31)) + + +@pytest.fixture(scope="module") +def upload(stack): + name = f"people_{uuid.uuid4().hex[:6]}.csv" + res = stack.upload("admin", name, "id,name,amount\n" + ROWS, types=["int8", "text", "int8"]) + assert res["status"] == "ok" and res["failedRows"] == 4, res + return name + + +def _select_row(page, index: int) -> None: + """st.dataframe draws on a canvas: click the selection checkbox of the row at `index`.""" + grid = page.locator('[data-testid="stDataFrame"]').first + grid.wait_for() + box = grid.bounding_box() + page.mouse.click(box["x"] + 15, box["y"] + 35 + 35 * index + 17) + page.get_by_role("heading", name=f"Row {index + 1}").wait_for() + + +def test_an_uploader_can_look_but_not_change(page, stack, upload): + streamlit_sign_in(page, stack, "uploader") + expect(page.get_by_text(upload, exact=False).first).to_be_visible() + _select_row(page, 6) # row 7: amount "x" + expect(page.get_by_text(re.compile(r'amount: "x" is not a whole number'))).to_be_visible() + expect(page.get_by_role("button", name="History")).to_be_visible() + expect(page.get_by_role("button", name="Edit a cell")).to_have_count(0) + expect(page.get_by_role("button", name="Exclude")).to_have_count(0) + expect(page.get_by_text("read-only for your role")).to_be_visible() + + +def test_a_reviewer_edits_with_a_required_reason_and_it_shows_in_the_history(page, stack, upload): + streamlit_sign_in(page, stack, "reviewer") + _select_row(page, 6) + page.get_by_role("button", name="Edit a cell").click() + dialog = page.get_by_role("dialog") + dialog.get_by_role("combobox").click() + page.get_by_role("option", name="amount").click() + # Choosing a column re-runs the script and rebuilds the field: wait for it, as a person would. + expect(dialog.get_by_text(re.compile(r"Original:\s*x\b"))).to_be_visible() + dialog.get_by_label("New value").fill("70") + dialog.get_by_label("New value").press("Tab") + save = dialog.get_by_role("button", name="Save") + expect(save).to_be_disabled() # no reason yet + dialog.get_by_label("Reason (required)").fill("typo in source") + dialog.get_by_label("Reason (required)").press("Tab") + expect(save).to_be_enabled() + save.click() + + expect(page.get_by_role("dialog")).to_have_count(0) + expect(page.get_by_text(re.compile(r"1 edited"))).to_be_visible() + _select_row(page, 6) + page.get_by_role("button", name="History").click() + expect(page.get_by_role("dialog").get_by_text("typo in source")).to_be_visible() + + +def test_a_reviewer_excludes_a_row_with_a_reason(page, stack, upload): + streamlit_sign_in(page, stack, "reviewer") + _select_row(page, 13) # row 14: amount "x" + page.get_by_role("button", name="Exclude").click() + dialog = page.get_by_role("dialog") + dialog.get_by_label("Reason (required)").fill("duplicate") + dialog.get_by_label("Reason (required)").press("Tab") + dialog.get_by_role("button", name="Save").click() + expect(page.get_by_text(re.compile(r"\d+ excluded"))).to_be_visible() + _select_row(page, 13) + expect(page.get_by_text(re.compile("Decision: exclude"))).to_be_visible() + + +def test_signing_out_leaves_nothing_for_the_next_tenant(page, stack, upload): + streamlit_sign_in(page, stack, "admin") + expect(page.get_by_text(upload, exact=False).first).to_be_visible() + page.locator('[data-testid="stSidebar"]').get_by_role("button", name="Sign out").click() + + streamlit_sign_in(page, stack, "other") # a different tenant, with no uploads + expect(page.get_by_text("No uploads yet.")).to_be_visible() + expect(page.get_by_text(upload, exact=False)).to_have_count(0) From 745ae6033d14f6af95cdd9941f8eaf8bdd488e6e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:33:16 +0000 Subject: [PATCH 6/9] docs: issue-to-test matrix; lint tidy-ups in the new tests docs/ISSUE_TESTS.md has a row for each of the 69 issues (46 closed, 23 open): the test that guards it, or why none can. Notes the two strict expected failures (#176, #181 second half) and what the end-to-end run found. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- docs/ISSUE_TESTS.md | 98 ++++++++++++++++++++++++++++++++++++ e2e/conftest.py | 3 +- tests/test_open_issues.py | 1 - tests/test_tooling_issues.py | 1 - 4 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 docs/ISSUE_TESTS.md diff --git a/docs/ISSUE_TESTS.md b/docs/ISSUE_TESTS.md new file mode 100644 index 0000000..1c43bdb --- /dev/null +++ b/docs/ISSUE_TESTS.md @@ -0,0 +1,98 @@ +# Which test verifies which issue + +Every issue in the tracker at 9 Nov 2026 (46 closed, 23 open), with the test that fails if it comes back, or the reason there is none. +Hand-kept: when you close an issue, add its row. `docs/TEST_CASES.md` (generated) lists every pytest test; this file answers "is issue N guarded?". + +Legend: **✅** guarded by a test · **⚠️** guarded, but the test is a known failure until the defect is fixed (strict, so it turns red when it is) · **📎** fix and tests are in an open PR, not on `main` yet · **👤** needs a person or another system · **—** nothing to test. + +Frontend tests are vitest in `csv-table-hub-main/src/__tests__/` (`fe:` below). Browser tests are in `e2e/` and are not part of the default run (see `e2e/README.md`). + +## Closed issues + +| # | What | Status | Verified by | +|---|---|---|---| +| 99 | Per-tenant T&E tables | ✅ | `tests/test_tenant_isolation.py` (own tables, per-tenant counts, keys and triggers kept) | +| 102 | Lock accounts after failed sign-ins | ✅ | `tests/test_accounts.py` (`Lockout`) | +| 107 | Immutable source, append-only audit | ✅ | `tests/test_preserved_uploads.py`, `test_audit_append_only.py`, `test_original_bytes.py` | +| 108 | Review: paging, include/exclude, versioned edits | ✅ | `tests/test_review.py`, `test_review_cases.py`, `fe: review-table.test.tsx`, `e2e/test_streamlit_spike.py` | +| 114 | Delete merged branches | — | housekeeping | +| 116 | Local PostgreSQL not reachable on 5433 | 👤 | one dev machine's setup; closed on evidence attached to the issue | +| 117 | Password change and admin reset | ✅ | `tests/test_password_reset.py`, `fe: change-password.test.tsx` | +| 118 | "Failed to fetch" when the API is down | ✅ | `fe: session.test.ts` ("API unreachable"). The PowerShell start-up guard in `scripts/start-api.ps1` has no test | +| 119 | Upload times shown in UTC | ✅ | `fe: local-time.test.ts` | +| 120 | House writing style doc | — | docs | +| 123 | Every-row-rejected upload shown as migrated | ✅ | `tests/test_failed_uploads.py`, `fe: job-outcome.test.ts` | +| 130 | Frontend lock file out of sync | ✅ | `tests/test_tooling_issues.py` (lock file agrees with `package.json`) | +| 132 | `npm audit` critical and high | ✅ / 👤 | `fe: table-rows-contract.test.ts` guards the zod 4 change. `npm audit` itself needs the registry and was not run | +| 133 | React hydration errors on `/auth` | ✅ | `fe: session.test.ts` | +| 151 | Terraform storage accounts audit | — | investigation | +| 152 | Frontend architecture decision | — | decision, see #154 and #212 | +| 154 | Streamlit spike | ✅ | `e2e/test_streamlit_spike.py` | +| 156 | Tracker home decision | — | decision | +| 162 | Cosmetic tracker doc edits | — | docs | +| 164 | Live pointers to superseded API doc | — | docs, closed not planned | +| 165 | STARTER_GUIDE dedup claim | — | docs, closed not planned | +| 166 | STARTER_GUIDE skips sign-in | — | docs | +| 169 | Test catalogue generator | ✅ | `tests/test_tooling_issues.py` and the CI freshness step | +| 170 | All-failed T&E import removed the prior dataset | ✅ | `tests/test_te_reupload_registry.py` (`…keeps_a_prior_dynamic_table…`), `tests/test_failed_uploads.py` | +| 171 | `setup.sh` truncated the config on failure | ✅ | `tests/test_tooling_issues.py` (runs the real config-write block) | +| 175 | Overwrite and mode switch bypassed the delete gate | ✅ | `tests/test_issue_regressions.py` (`UploadCannotBypassTheDeleteGate`) | +| 176 | Browser history leaks across tenants | ⚠️ | `fe: tenant-isolation.test.tsx`, `e2e/test_react_ui.py`. Closed as moot (the React screen is being replaced, #212), but the leak is live in the shipped app. Both tests turn red when it is fixed | +| 177 | Composite FKs for program consistency | ✅ | `tests/suites/test_06_program_consistency.sql` (Tier S) | +| 178 | Distinct rows dropped as duplicates | ✅ | `tests/test_issue_regressions.py` (`RowDedupKeepsDistinctRows`) | +| 179 | Frontend lint baseline | — | not a test: `npm run lint` is the check, and is not in CI. Closed as moot | +| 180 | Tier I eval ignored SQL errors | ✅ | `tests/test_tooling_issues.py` | +| 181 | SQL test wrapper exited 0 on failures | ✅ / ⚠️ | `tests/test_tooling_issues.py`. The second half (`ON_ERROR_STOP` on the Tier S psql calls in `evals/runner.py`) is not on `main`: strict expected failure | +| 182 | Offline bundle swallowed download failures | ✅ | `tests/test_tooling_issues.py` (runs the real download block with a failing `apt-get`) | +| 183 | Rate limiter used the API key prefix | ✅ | `tests/test_issue_regressions.py` (`RateLimiterIdentity`) | +| 185 | Preserved original differed from uploaded bytes | ✅ | `tests/test_original_bytes.py`, `fe: file-bytes.test.ts` | +| 186 | Header mappings ignored | ✅ | `tests/test_header_mapping.py`, `fe: log-step-contract.test.ts` (the schema accepts the step it added) | +| 187 | Failed uploads unreachable from the browser | ✅ | `fe: upload-attempts.test.tsx`, `fe: files-refresh.test.ts`, `e2e/test_react_ui.py` | +| 188 | Cancel did not interrupt an import | ✅ | `fe: uploader-state-machine.test.ts`, `fe: job-outcome.test.ts`, `e2e/test_react_ui.py`. Server-side cancel is not built (#110) | +| 189 | T&E errors named the wrong source row | ✅ | `tests/test_issue_regressions.py` (`TeDatabaseErrorsNameTheSourceRow`) | +| 190 | T&E review showed no validation errors | ✅ | `tests/test_review.py`, `tests/test_api_coverage.py`, `fe: review-table.test.tsx` | +| 191 | Registry race | ✅ | `tests/test_registry_race.py` | +| 192 | Ragged rows accounting | ✅ | `tests/test_ragged_rows.py` | +| 193 | Formula injection in exports | ✅ | `fe: export.test.ts` | +| 194 | Coverage floors for the loaders | ✅ | `tests/test_tooling_issues.py` (the gate fails below a floor) | +| 207 | API doc covered 8 of 22 routes | ✅ | `tests/test_tooling_issues.py` (the doc defers to the generated schema, which is served) | +| 215 | All-failed T&E re-upload overwrote the registry record | ✅ | `tests/test_te_reupload_registry.py`, `tests/test_te_loader_integration.py` | + +## Open issues + +| # | What | Status | Verified by | +|---|---|---|---| +| 31 | One-time cloud setup for terraform-test/staging | 👤 | cloud account work | +| 32 | `az` CLI SSL error on the dev machine | 👤 | one machine's setup | +| 34 | `terraform plan` provider handshake fails locally | 👤 | one machine's setup | +| 57 | Add SECURITY.md | ✅ | `tests/test_open_issues.py`. The issue's done-when ("on `main`") is met, so it could be closed. The file still carries a placeholder address, noted in a comment inside it | +| 59 | Sample Migration Verification Report | 📎 | PR #216 (docs) | +| 60 | Air-gapped install procedure | ✅ / 👤 | `tests/test_open_issues.py` (scripts parse, stop on error, guide names them). The offline dry run, due 15 Nov, is manual | +| 109 | ART M5: dedup and naming | — | milestone, not built | +| 110 | ART M6: approval and staged import | — | milestone, not built. Server-side cancel (#188 option b) belongs here | +| 111 | ART M7: sovereignty and reports | — | milestone, not built | +| 112 | Security review before the first external tenant | 👤 | review by a person. The #176 tests are evidence for it | +| 113 | Check OIDC sign-in against the real IdP | ✅ / 👤 | `tests/test_oidc_https_jwks.py` covers the https key fetch, rotation and certificate checks. A sign-in against Keycloak or Entra ID is manual | +| 128 | Offline bundle for Windows, macOS and Linux | — | only the Ubuntu bundle exists | +| 159 | Verify `docs/VCRM.md` against the DID | 👤 | compare with an external document | +| 167 | Reorganise `tests/` into category folders | 📎 | PR #214 | +| 168 | Accessibility CI job | 📎 | PR #214 (`tests/accessibility/`) | +| 172 | Production database name disagreement | 📎 | PR #213 (`tests/test_prod_db_naming.py`) | +| 173 | Cluster-wide `te_app` role | 📎 | PR #208 (`tests/test_runtime_role_per_env.py`) | +| 174 | Docker image can bake local credentials | 📎 | PR #204 (`tests/test_docker_image_secrets.py`) | +| 184 | Tenant T&E schemas have no upgrade path | 📎 | PR #213 (`tests/test_tenant_te_migrations.py`) | +| 195 | Confirm inferred types, provenance flag | 📎 | PR #213 (`tests/test_type_provenance.py`) | +| 198 | Track checks that completed after merging #197 | — | process | +| 210 | Background job queue for large imports | — | feature, not built | +| 212 | Plan the Streamlit migration | — | a plan. `e2e/test_streamlit_spike.py` is the baseline it must keep passing | + +## Known failures on purpose + +Strict expected failures: the suite is green now and turns red when the defect is fixed, so the marker gets removed then. + +- **#176** (React localStorage history leak): `fe: tenant-isolation.test.tsx` (`it.fails`) and `e2e/test_react_ui.py`. +- **#181, second half** (Tier S psql calls without `ON_ERROR_STOP`): `tests/test_tooling_issues.py`. + +## Found while writing these tests + +- **Every dynamic import in the React app showed as Failed** since #202: the app's response schema did not list the new `header_mapping` log step. Fixed in PR #223, with a test that checks the schema against every step the API can emit. diff --git a/e2e/conftest.py b/e2e/conftest.py index 269b457..1221f98 100644 --- a/e2e/conftest.py +++ b/e2e/conftest.py @@ -7,7 +7,6 @@ from __future__ import annotations import os -import shutil import signal import socket import subprocess @@ -154,7 +153,7 @@ def stack(): @pytest.fixture(scope="session") def browser(): with sync_playwright() as p: - exe = os.environ.get("E2E_CHROMIUM") or (shutil.which("chromium") if os.environ.get("E2E_SYSTEM_CHROMIUM") else None) + exe = os.environ.get("E2E_CHROMIUM") or None # else the browser `playwright install` fetched b = p.chromium.launch(executable_path=exe) if exe else p.chromium.launch() yield b b.close() diff --git a/tests/test_open_issues.py b/tests/test_open_issues.py index e6baf3c..4956e87 100644 --- a/tests/test_open_issues.py +++ b/tests/test_open_issues.py @@ -10,7 +10,6 @@ from __future__ import annotations -import re import subprocess import unittest from pathlib import Path diff --git a/tests/test_tooling_issues.py b/tests/test_tooling_issues.py index 65442ca..3185e20 100644 --- a/tests/test_tooling_issues.py +++ b/tests/test_tooling_issues.py @@ -21,7 +21,6 @@ import subprocess import sys import tempfile -import textwrap import unittest from pathlib import Path From b69b24dc1f0349b4d6bbe0c96850e77017d11a79 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:34:32 +0000 Subject: [PATCH 7/9] test: drop the #181 expected-fail test; CI fails any skip scripts/test_report.py --strict counts an expected failure as a skip and fails the build ("unmet prerequisites must FAIL, not skip"). Remove the strict xfail for the second half of #181 and record the gap in docs/ISSUE_TESTS.md instead. #176's expected failures are vitest and browser tests, which that step does not run. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- docs/ISSUE_TESTS.md | 9 ++++----- docs/TEST_CASES.md | 6 +----- tests/test_tooling_issues.py | 10 ---------- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/docs/ISSUE_TESTS.md b/docs/ISSUE_TESTS.md index 1c43bdb..f415571 100644 --- a/docs/ISSUE_TESTS.md +++ b/docs/ISSUE_TESTS.md @@ -42,7 +42,7 @@ Frontend tests are vitest in `csv-table-hub-main/src/__tests__/` (`fe:` below). | 178 | Distinct rows dropped as duplicates | ✅ | `tests/test_issue_regressions.py` (`RowDedupKeepsDistinctRows`) | | 179 | Frontend lint baseline | — | not a test: `npm run lint` is the check, and is not in CI. Closed as moot | | 180 | Tier I eval ignored SQL errors | ✅ | `tests/test_tooling_issues.py` | -| 181 | SQL test wrapper exited 0 on failures | ✅ / ⚠️ | `tests/test_tooling_issues.py`. The second half (`ON_ERROR_STOP` on the Tier S psql calls in `evals/runner.py`) is not on `main`: strict expected failure | +| 181 | SQL test wrapper exited 0 on failures | ✅ / 👤 | `tests/test_tooling_issues.py` covers the wrapper. The second half (`ON_ERROR_STOP` on the Tier S psql calls in `evals/runner.py`, fix on the unmerged `fix/181-sql-suite-exit-code` branch) has no test: CI fails any skipped or expected-fail pytest test, so it can't be recorded as one | | 182 | Offline bundle swallowed download failures | ✅ | `tests/test_tooling_issues.py` (runs the real download block with a failing `apt-get`) | | 183 | Rate limiter used the API key prefix | ✅ | `tests/test_issue_regressions.py` (`RateLimiterIdentity`) | | 185 | Preserved original differed from uploaded bytes | ✅ | `tests/test_original_bytes.py`, `fe: file-bytes.test.ts` | @@ -86,12 +86,11 @@ Frontend tests are vitest in `csv-table-hub-main/src/__tests__/` (`fe:` below). | 210 | Background job queue for large imports | — | feature, not built | | 212 | Plan the Streamlit migration | — | a plan. `e2e/test_streamlit_spike.py` is the baseline it must keep passing | -## Known failures on purpose +## Known failure on purpose -Strict expected failures: the suite is green now and turns red when the defect is fixed, so the marker gets removed then. +A strict expected failure: the suite is green now and turns red when the defect is fixed, so the marker gets removed then. -- **#176** (React localStorage history leak): `fe: tenant-isolation.test.tsx` (`it.fails`) and `e2e/test_react_ui.py`. -- **#181, second half** (Tier S psql calls without `ON_ERROR_STOP`): `tests/test_tooling_issues.py`. +- **#176** (React localStorage history leak): `fe: tenant-isolation.test.tsx` (`it.fails`) and `e2e/test_react_ui.py`. Neither runs in the pytest CI step, which fails any skipped or expected-fail test (`scripts/test_report.py --strict`). ## Found while writing these tests diff --git a/docs/TEST_CASES.md b/docs/TEST_CASES.md index 2d79be2..db9fc58 100644 --- a/docs/TEST_CASES.md +++ b/docs/TEST_CASES.md @@ -1,7 +1,7 @@ # Test Catalogue > Auto-generated by `scripts/test_catalogue.py` — do not edit by hand. -> 826 tests across 42 files. +> 824 tests across 42 files. To regenerate: `python3 scripts/test_catalogue.py` @@ -1749,8 +1749,6 @@ To regenerate: `python3 scripts/test_catalogue.py` | `EvalRunnerStopsOnSqlErrors.test_the_idempotency_eval_stops_at_the_first_sql_error` | unit | — | -| `EvalRunnerStopsOnSqlErrors.test_the_tier_s_deploy_and_suite_calls_stop_at_the_first_sql_error` | unit | — | - | `SqlWrapperFailsOnAssertionFailures.test_failed_assertions_mark_the_environment_failed` | unit | — | | `SqlWrapperFailsOnAssertionFailures.test_a_clean_run_is_not_marked_failed` | unit | — | @@ -1791,8 +1789,6 @@ To regenerate: `python3 scripts/test_catalogue.py` | `test_the_idempotency_eval_stops_at_the_first_sql_error` | — | — | -| `test_the_tier_s_deploy_and_suite_calls_stop_at_the_first_sql_error` | — | — | - | `test_failed_assertions_mark_the_environment_failed` | — | — | | `test_a_clean_run_is_not_marked_failed` | — | — | diff --git a/tests/test_tooling_issues.py b/tests/test_tooling_issues.py index 3185e20..ee8e9a9 100644 --- a/tests/test_tooling_issues.py +++ b/tests/test_tooling_issues.py @@ -93,16 +93,6 @@ def test_the_idempotency_eval_stops_at_the_first_sql_error(self): for call in calls: self.assertIn("ON_ERROR_STOP=1", call) - @pytest.mark.xfail(strict=True, reason=( - "#181 second half, not on main: the Tier S deploy and SQL-suite psql calls in evals/runner.py " - "have no ON_ERROR_STOP, so a SQL error mid-suite is not an error exit. Fix is on the unmerged " - "branch fix/181-sql-suite-exit-code.")) - def test_the_tier_s_deploy_and_suite_calls_stop_at_the_first_sql_error(self): - calls = [c for c in _psql_calls() if "env_dev_sql" in c and "-v" not in c or "run_tests" in c] - self.assertEqual(len(calls), 2, calls) - for call in calls: - self.assertIn("ON_ERROR_STOP=1", call) - # ── #181 ───────────────────────────────────────────────────────────────────── From b657cb14a51a2c609684c7c73078ccf691549836 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:41:16 +0000 Subject: [PATCH 8/9] test: keep the shell-script tests off the Windows job The Windows job runs every unit-marked test, and `bash` there is not a POSIX shell, so the tests that run the repo's real shell blocks, plus `bash -n` and the catalogue check, failed. Add a `tooling` marker for them (Linux jobs and local runs) and leave the pure-Python tests as unit. The coverage-gate test also failed on Windows: the script prints box-drawing characters and a Windows console's default encoding can't. The test now runs it with UTF-8 output. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- docs/TEST_CASES.md | 28 ++++++++++++++-------------- pytest.ini | 1 + tests/test_open_issues.py | 17 +++++++++++------ tests/test_tooling_issues.py | 15 +++++++++------ 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/docs/TEST_CASES.md b/docs/TEST_CASES.md index db9fc58..1f3b6f3 100644 --- a/docs/TEST_CASES.md +++ b/docs/TEST_CASES.md @@ -1133,14 +1133,14 @@ To regenerate: `python3 scripts/test_catalogue.py` | `OfflineInstallProcedureIsConsistent.test_the_build_and_install_scripts_exist` | unit | — | -| `OfflineInstallProcedureIsConsistent.test_every_script_parses` | unit | — | - | `OfflineInstallProcedureIsConsistent.test_the_build_and_install_scripts_stop_on_the_first_error` | unit | — | | `OfflineInstallProcedureIsConsistent.test_the_guide_names_the_scripts_it_relies_on` | unit | — | | `OfflineInstallProcedureIsConsistent.test_the_guide_gives_the_checksum_step` | unit | — | +| `OfflineScriptsParse.test_every_script_parses` | — | — | + | `test_says_how_to_report_privately` | — | — | | `test_names_a_contact_address` | — | — | @@ -1149,14 +1149,14 @@ To regenerate: `python3 scripts/test_catalogue.py` | `test_the_build_and_install_scripts_exist` | — | — | -| `test_every_script_parses` | — | — | - | `test_the_build_and_install_scripts_stop_on_the_first_error` | — | — | | `test_the_guide_names_the_scripts_it_relies_on` | — | — | | `test_the_guide_gives_the_checksum_step` | — | — | +| `test_every_script_parses` | — | — | + ## `tests/test_original_bytes.py` | Test | Markers | Objective | @@ -1741,29 +1741,29 @@ To regenerate: `python3 scripts/test_catalogue.py` | Test | Markers | Objective | |---|---|---| -| `SetupKeepsPriorConfigOnFailure.test_the_block_was_found_and_uses_many_variables` | unit | — | +| `SetupKeepsPriorConfigOnFailure.test_the_block_was_found_and_uses_many_variables` | — | — | -| `SetupKeepsPriorConfigOnFailure.test_success_replaces_the_config_and_leaves_no_temp_file` | unit | — | +| `SetupKeepsPriorConfigOnFailure.test_success_replaces_the_config_and_leaves_no_temp_file` | — | — | -| `SetupKeepsPriorConfigOnFailure.test_an_unset_variable_fails_the_run_but_keeps_the_prior_config` | unit | — | +| `SetupKeepsPriorConfigOnFailure.test_an_unset_variable_fails_the_run_but_keeps_the_prior_config` | — | — | | `EvalRunnerStopsOnSqlErrors.test_the_idempotency_eval_stops_at_the_first_sql_error` | unit | — | -| `SqlWrapperFailsOnAssertionFailures.test_failed_assertions_mark_the_environment_failed` | unit | — | +| `SqlWrapperFailsOnAssertionFailures.test_failed_assertions_mark_the_environment_failed` | — | — | -| `SqlWrapperFailsOnAssertionFailures.test_a_clean_run_is_not_marked_failed` | unit | — | +| `SqlWrapperFailsOnAssertionFailures.test_a_clean_run_is_not_marked_failed` | — | — | -| `OfflineBundleStopsOnDownloadFailure.test_the_old_swallow_is_gone` | unit | — | +| `OfflineBundleStopsOnDownloadFailure.test_the_old_swallow_is_gone` | — | — | -| `OfflineBundleStopsOnDownloadFailure.test_a_failed_download_stops_the_build` | unit | — | +| `OfflineBundleStopsOnDownloadFailure.test_a_failed_download_stops_the_build` | — | — | -| `OfflineBundleStopsOnDownloadFailure.test_a_download_that_yields_no_packages_stops_the_build` | unit | — | +| `OfflineBundleStopsOnDownloadFailure.test_a_download_that_yields_no_packages_stops_the_build` | — | — | -| `OfflineBundleStopsOnDownloadFailure.test_a_complete_download_continues` | unit | — | +| `OfflineBundleStopsOnDownloadFailure.test_a_complete_download_continues` | — | — | | `FrontendLockFileMatchesPackageJson.test_root_entry_and_every_direct_dependency_agree` | unit | — | -| `TestCatalogueIsCurrent.test_docs_test_cases_matches_the_tests` | unit | — | +| `TestCatalogueIsCurrent.test_docs_test_cases_matches_the_tests` | — | — | | `CoverageFloorGate.test_floors_cover_the_three_loaders` | unit | — | diff --git a/pytest.ini b/pytest.ini index 49bc7cc..636a801 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,6 +7,7 @@ markers = security: static security and credential pattern scans snapshot: golden-file output comparison tests parity: cross-environment row-count consistency tests + tooling: runs the repo's real shell scripts and needs bash; Linux jobs and local runs, not the Windows job addopts = -ra -q testpaths = tests diff --git a/tests/test_open_issues.py b/tests/test_open_issues.py index 4956e87..0374d0a 100644 --- a/tests/test_open_issues.py +++ b/tests/test_open_issues.py @@ -46,12 +46,6 @@ class OfflineInstallProcedureIsConsistent(unittest.TestCase): def test_the_build_and_install_scripts_exist(self): self.assertTrue({"build_bundle.sh", "install.sh"} <= {p.name for p in self.SCRIPTS}) - def test_every_script_parses(self): - for script in self.SCRIPTS: - with self.subTest(script=script.name): - res = subprocess.run(["bash", "-n", str(script)], capture_output=True, text=True) - self.assertEqual(res.returncode, 0, res.stderr) - def test_the_build_and_install_scripts_stop_on_the_first_error(self): # smoke_test.sh is excluded on purpose: it counts PASS/FAIL checks and exits non-zero at the end. for name in ("build_bundle.sh", "install.sh"): @@ -66,3 +60,14 @@ def test_the_guide_names_the_scripts_it_relies_on(self): def test_the_guide_gives_the_checksum_step(self): self.assertRegex(self.GUIDE, r"(?i)sha256") + + +@pytest.mark.tooling +class OfflineScriptsParse(unittest.TestCase): + """#60: `bash -n` on each offline script. Needs bash, so it is marked tooling (not run on Windows).""" + + def test_every_script_parses(self): + for script in sorted((ROOT / "scripts/offline").glob("*.sh")): + with self.subTest(script=script.name): + res = subprocess.run(["bash", "-n", str(script)], capture_output=True, text=True) + self.assertEqual(res.returncode, 0, res.stderr) diff --git a/tests/test_tooling_issues.py b/tests/test_tooling_issues.py index ee8e9a9..d30a960 100644 --- a/tests/test_tooling_issues.py +++ b/tests/test_tooling_issues.py @@ -9,7 +9,8 @@ #194 the coverage-floor gate must fail below a floor #207 API_INTEGRATION.md must defer to the generated schema, and the schema must be served -No database needed. Shell blocks are cut out of the real scripts and run, not re-implemented here. +No database needed. Shell blocks are cut out of the real scripts and run, not re-implemented here; +those classes are marked `tooling` (they need bash), so the Windows job leaves them out. """ from __future__ import annotations @@ -42,7 +43,7 @@ def _bash(script: str, **env: str) -> subprocess.CompletedProcess: # ── #171 ───────────────────────────────────────────────────────────────────── -@pytest.mark.unit +@pytest.mark.tooling class SetupKeepsPriorConfigOnFailure(unittest.TestCase): """#171: the config is written to a temp file and moved into place only once complete.""" @@ -96,7 +97,7 @@ def test_the_idempotency_eval_stops_at_the_first_sql_error(self): # ── #181 ───────────────────────────────────────────────────────────────────── -@pytest.mark.unit +@pytest.mark.tooling class SqlWrapperFailsOnAssertionFailures(unittest.TestCase): """#181: psql exits 0 even when assertions record FAIL rows; the wrapper must read the output.""" @@ -125,7 +126,7 @@ def test_a_clean_run_is_not_marked_failed(self): # ── #182 ───────────────────────────────────────────────────────────────────── -@pytest.mark.unit +@pytest.mark.tooling class OfflineBundleStopsOnDownloadFailure(unittest.TestCase): SCRIPT = (ROOT / "scripts/offline/build_bundle.sh").read_text(encoding="utf-8") BLOCK = _between(SCRIPT, "REAL=()", "( cd \"$OUT/debs\" && apt-ftparchive", include_end=False) @@ -181,7 +182,7 @@ def test_root_entry_and_every_direct_dependency_agree(self): # ── #169 ───────────────────────────────────────────────────────────────────── -@pytest.mark.unit +@pytest.mark.tooling class TestCatalogueIsCurrent(unittest.TestCase): def test_docs_test_cases_matches_the_tests(self): res = subprocess.run([sys.executable, "scripts/test_catalogue.py", "--check"], cwd=ROOT, @@ -200,8 +201,10 @@ def _gate(self, pct: dict[str, float], drop: str | None = None) -> int: with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as f: json.dump(report, f) try: + # The gate prints box-drawing characters; a Windows console's default encoding can't. return subprocess.run([sys.executable, "scripts/check_coverage_floors.py", f.name], cwd=ROOT, - capture_output=True, text=True, timeout=30).returncode + capture_output=True, text=True, timeout=30, + env={**os.environ, "PYTHONIOENCODING": "utf-8"}).returncode finally: os.unlink(f.name) From 31d205ee7e15a771891d2e25837e0b493571bf2f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 14:54:14 +0000 Subject: [PATCH 9/9] docs: #167 and #168 are done on main Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6Wqi8yaiaYWUVH8C4QAY --- docs/ISSUE_TESTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/ISSUE_TESTS.md b/docs/ISSUE_TESTS.md index c810c71..40ebc5c 100644 --- a/docs/ISSUE_TESTS.md +++ b/docs/ISSUE_TESTS.md @@ -75,8 +75,8 @@ Frontend tests are vitest in `csv-table-hub-main/src/__tests__/` (`fe:` below). | 113 | Check OIDC sign-in against the real IdP | ✅ / 👤 | `tests/unit/test_oidc_https_jwks.py` covers the https key fetch, rotation and certificate checks. A sign-in against Keycloak or Entra ID is manual | | 128 | Offline bundle for Windows, macOS and Linux | — | only the Ubuntu bundle exists | | 159 | Verify `docs/VCRM.md` against the DID | 👤 | compare with an external document | -| 167 | Reorganise `tests/` into category folders | 📎 | PR #214 | -| 168 | Accessibility CI job | 📎 | PR #214 (`tests/accessibility/`) | +| 167 | Reorganise `tests/` into category folders | ✅ | Done on `main` (#221); `tests/README.md` says where a new test goes. The issue is still open, and could be closed | +| 168 | Accessibility CI job | ✅ | Done on `main` (#222): `tests/accessibility/` and its CI job. The issue is still open, and could be closed | | 172 | Production database name disagreement | 📎 | PR #213 (`tests/test_prod_db_naming.py`) | | 173 | Cluster-wide `te_app` role | 📎 | PR #208 (`tests/test_runtime_role_per_env.py`) | | 174 | Docker image can bake local credentials | 📎 | PR #204 (`tests/test_docker_image_secrets.py`) |