Skip to content

Tests: registry race, ragged-row stored reasons, XLSX text cells (#191, #192, #193) - #214

Merged
amar-python merged 12 commits into
mainfrom
enh/04-167-168-191-192
Nov 10, 2026
Merged

amar-python merged 12 commits into
mainfrom
enh/04-167-168-191-192

Conversation

@amar-python

@amar-python amar-python commented Nov 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Tests only. No product code changes. Four files.

This PR started as a larger batch (#167, #168, #191–#193). While it sat in draft, main moved 70 commits and absorbed most of it: #167 (test layout, #221), #168 (accessibility job, #222) and #195 (#213), on top of #169, #193 and #194 from #202. I merged main in and dropped everything it already covers, so what is left is only what main still lacks. The earlier description of this PR (layout, accessibility job, a registry-race loader fix) no longer applies.

Issue What this PR adds
#191 A real concurrency test for the registry, replacing the existing one
#192 Checks that each ragged row's reason is stored, and the same accounting for T&E mode
#193 Assertions that the XLSX cell is stored as text with no formula

#167, #168, #169, #194, #195 need nothing from this PR.

Changes

#191, tests/system/test_registry_race.py. The existing test used no barrier and accepted status: "error" as a valid loser outcome, so it could not fail for the race it was written about. This one releases uploads together with a barrier and repeats 40 rounds, for three cases, because the guard differs:

  • identical file, same name: the per-dataset advisory lock serialises them; exactly one wins, the rest get an attributable duplicate_file
  • identical content under different names: different datasets, so different locks; only UNIQUE(file_hash) stands in the way
  • same name, different content: since M5 this is the next version, not a rejection, so it asserts distinct, gapless versions

#192, tests/system/test_ragged_rows.py. The existing tests check the accounting identity and a reason per ragged row in the response. These add that every reason is also stored (GET /api/csv/uploads/{id}), and the same checks for T&E mode, with a file mixing good rows, an in-file duplicate and several ragged rows.

#193, export.test.ts. The existing XLSX tests read values back through sheet_to_json. The issue asks for the cell to be asserted as stored as text, the property that stops a spreadsheet evaluating it: added for =cmd, +cmd, -cmd, @cmd and a tab-prefixed value, in the error and audit reports.

Evidence

PostgreSQL 16, all four environments deployed, on main (018edc2) plus this PR.

  • Backend: 635 collected, 635 passed, 0 failed, 0 skipped. Frontend: 180 passed. Lint, bandit and the catalogue check pass
  • Unmodified main: the race tests pass 12 of 12 runs
  • Mutation checks, each fails when its guard is removed:
    • UNIQUE(file_hash) dropped: the different-names race test fails 6 of 6
    • per-dataset advisory lock removed: both same-name race tests fail 4 of 4
    • extra_cells() returning []: 6 of 9 tests in test_ragged_rows.py fail
    • sanitizeForXlsx guard disabled: 11 of 41 tests in export.test.ts fail

Correction to my earlier claim

I previously reported that the #191 test found a real defect, where a losing upload could get a generic "database error" instead of a duplicate rejection, and I proposed a loader fix. That was true on the old main. It no longer reproduces: M5 changed table naming and added the dataset lock, and the test passes 12 of 12 without any fix. I dropped the fix rather than ship a change for a defect that is gone. The same race still exists in principle if the lock is removed, which is why the lock-removal mutation above fails the tests.

Not verified

  • CI has not run on the latest head (7b6beb7).
  • Microsoft Excel was not available. A LibreOffice 24.2 check of the exported XLSX (done earlier) showed the guarded cells rendering literally, not evaluated.
  • Still open from the original batch: the product owner's confirmation of the Gate: measure api/services coverage with floors for the three loaders #194 coverage floors (79/84/96).

Refs #191, #192, #193.

https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL


Generated by Claude Code

Flat tests/*.py made it impossible to run one category, and the M2/M3 tests
landed uncategorised. Six folders, each a package, with the renames PR #41
proposed plus the 17 test files that landed after it was drafted.

- tests/{unit,system,system_integration,performance,accessibility,uat}/
- tests/conftest.py: folder-derived markers, plus the shared live_api,
  registry_cleanup, te_org_cleanup, destructive_enabled and unique_tag fixtures
- pytest.ini: system, sit, performance, accessibility, uat markers
- Makefile: test-unit / test-system / test-sit / test-performance /
  test-accessibility / test-uat
- scripts/lint.sh: every Python test file is now collected by find, so the list
  cannot drift again; tests get E702/E401/E128 relaxed, source does not
- requirements-dev.txt: playwright, axe-playwright-python (for #168)
- health_check.py rglob, select_tests.py / run_qa.ps1 / .gitignore path updates

365 tests collected before and after; the no-database tiers pass unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
tests/test_header_mapping.py, test_original_bytes.py and test_registry_race.py
landed flat on main; all three need PostgreSQL, so they go to system_integration/.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
- tests/accessibility/: Playwright fixtures and scans of /auth, / and /audit.
  Signed-out, / and /audit redirect to /auth, so they would have been scanned
  as the sign-in page and reported a false green. The signed_in_page fixture
  seeds the te.session sessionStorage entry and stubs /api/**, and every scan
  asserts it landed on the route it asked for.
- quality-gate.yml: new `accessibility` job on pull_request. The scan step is
  continue-on-error because the frontend has known unfixed defects; results are
  uploaded as an artifact. The integration job now runs with
  --markers "not accessibility".
- The job installs with `npm ci`, not bun: bun.lock is stale (zod ^3 vs ^4) and
  `bun install --frozen-lockfile` fails against package.json.

First local run against the Vite dev server: 4 pass, 3 fail. The failures are
real frontend defects (unlabelled select on /audit; no <main> on /auth and
/audit), which is what the issue says to expect.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
…191, F1)

Adds tests/system_integration/test_registry_race.py: several uploads released
together by a barrier, 40 rounds each, for (a) the identical file, (b) identical
content under different names, (c) the same name with different content. Asserts
exactly one winner, one registry row, an attributable duplicate_file for every
loser, and no orphan csv_<hash> tables or duplicated rows.

The test found a defect. When identical uploads overlap, the losing request
could get a generic status "error" ("relation ... already exists" /
"type ... already exists") instead of duplicate_file: the #81 handler caught
only UniqueViolation, and these arrive as DuplicateTable / DuplicateObject. The
registry stayed consistent; the loser just was not told why. The handler also
named the loser's own file, not the winner's. upload_dynamic now catches all
three and a small _lost_race() looks up the winner in the registry.

Evidence, PostgreSQL 16:
- fix reverted: 5 of 5 runs fail
- fix in place: 12 of 12 (at 8 rounds) and 6 of 6 (at 40 rounds) pass
- mutation, file_hash UNIQUE dropped: the different-names test fails 10 of 10.
  The same-name test does not detect it, because UNIQUE (file_name) and the
  csv_<hash> table name also stop that case; documented in the test.
- full suite, all four environments deployed: 370 passed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
…ounting (#192)

Upstream's tests already check the accounting identity and that each ragged row
gets a reason in the response. Adds what they do not: that every reason is also
stored (GET /api/csv/uploads/{id}), and the same checks for T&E mode, with a file
mixing good rows, an in-file duplicate and several ragged rows.

Mutation, extra_cells() forced to return []: 6 of 9 tests in the file fail,
including all the new ones. Passing run against PostgreSQL 16: 9 passed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
…back (#193)

Upstream's guard and tests already cover the payloads and plain numbers by
reading values back through sheet_to_json. The issue asks for the cell to be
asserted as stored as text, which is the property that stops a spreadsheet from
evaluating it: adds that for =cmd, +cmd, -cmd, @cmd and a tab-prefixed value in
the error report, and for the audit report.

Mutation, upstream's guard disabled: 11 of 38 tests in export.test.ts fail.

Spreadsheet-application check, as the issue asks: the XLSX produced by
downloadErrorReport, opened in LibreOffice 24.2.7.2, shows '=1+1, '=cmd, '+cmd,
'-cmd, '@cmd literally (not evaluated); -5 and plain are untouched. Microsoft
Excel was not available, so it is not covered.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
Only conflict was the generated docs/TEST_CASES.md, regenerated with
scripts/test_catalogue.py. tests/test_te_reupload_registry.py (#215) landed flat on
main and is moved to tests/system_integration/ to match the #167 layout.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
, #193)

main now contains #167 (test layout, #221), #168 (accessibility job, #222) and #195
(#213), so those parts of this PR are dropped in favour of main's. The registry-race
loader fix is dropped too: M5 changed table naming and added a per-dataset lock, and
the race no longer reproduces on main (12 of 12 runs pass without it).

What remains is the test work main lacks, written against M5's semantics. See the
preceding commit message for the evidence.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
@amar-python amar-python changed the title Test layout, accessibility job, registry-race fix, ragged/XLSX test deltas (#167, #168, #191, #192, #193) Tests: registry race, ragged-row stored reasons, XLSX text cells (#191, #192, #193) Nov 10, 2026
amar-python and others added 2 commits November 10, 2026 18:11
The GitHub merge left the generated header count stale (1293, should be 1299), so
scripts/test_catalogue.py --check failed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6Wqj8HS5EUEcvTUM24SL
@amar-python
amar-python merged commit 7f0b666 into main Nov 10, 2026
4 checks passed
@amar-python
amar-python deleted the enh/04-167-168-191-192 branch November 10, 2026 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants