fix(tests): test-created devices and fixture dirs now meet the cleanup contract

Test suites that attach devices to the SHARED admin (testuser, the live
dev account) escaped the harness sweeps whenever their names lacked
"test": createContractDeviceNamed named its devices by surface type
("koreader" — the web progress push adopts the device name as sync
source, so conflict tests drive a second named source without a
separate account), the reinstall regression test uses the web flow's
real "Reinstall Device" label, and opds_test.go's folders test named
its device "OPDS Folders Device". None had self-cleanup either, so
every suite run left 3 more rows on the dev account's device grid —
476 by the time anyone looked. TestDeviceSetup.CreateDevice also never
cleanup-registered, so any CURRENT or future helper-created device was
one rename away from the same leak.

Changes:
- TestDeviceSetup.CreateDevice self-deletes its row at test end
  (t.Cleanup): callers owe nothing to name hygiene any more.
- createContractDeviceNamed self-deletes likewise (contract-% prefix).
- device_test.go: core via cleanupDeviceByIdentifier on every device it
  attaches to the admin ("Reinstall Device" by unique identifier —
  the NAME is user-facing-labeled, sweeps must never torch it).
- Harness setup-time admin-device sweep gains the unambiguous harness
  patterns (dev_contract_* tokens, contract-%/opds-%/repro-reinstall-%
  identifiers) as the drift net; ran once with the rebuilt tests image
  and that sweep alone already purged the existing 476 rows.
- metadata_write_test.go now globs ALL test-metadata-write-* dirs in
  its RemoveAll instead of only its own pid's — killed runs left root-
  owned dirs on the user's uploads bind mount (7 found, deleted).

 audits: collections/saved-filters/libraries/sync_queue are clean;
 the only "test"-named media rows left are scans of the user's own
 on-disk test files (Test/, Bookhoard Test/, embedded " test" in two
 real books' metadata) — content, not suite debris.

Verified: filtered runs (contract/OPDS/reinstall/pairing/metadata)
green, zero device residue after the run, uploads mount clean.
This commit is contained in:
John O'Keefe committed 2026-11-02 17:39:29 -05:00
1 parent 0c31e9b88d
commit 7d88ec5184
5 files changed
+68 -4

No files matched your search

+32 -2
View File
@@ -96,6 +96,21 @@ type TestServerSetup struct {
closed bool
}
// cleanupDeviceByIdentifier deletes any devices rows carrying the given
// identifier at test end. API-register tests need this instead of a
// token-based delete: the status poll is single-use, so a row created
// through register+approve changes tokens mid-test — the stable
// identity is UNIQUE(device_identifier). Use for every device a test
// attaches to the SHARED admin (testuser); See the DOCTRINE comment in
// setupTestServer — rows that survive their test pile up on the live
// dev account's device grid.
func cleanupDeviceByIdentifier(t *testing.T, setup *TestServerSetup, identifier string) {
t.Cleanup(func() {
_, _ = setup.DBPool.Exec(context.Background(),
"DELETE FROM devices WHERE device_identifier = $1", identifier)
})
}
// Close cleans up all resources in the correct order
func (s *TestServerSetup) Close() error {
s.mu.Lock()
@@ -481,6 +496,13 @@ func (s *TestDeviceSetup) CreateDevice(t *testing.T, deviceName, deviceType, dev
})
require.NoError(t, err, "Should create device")
// Devices created through this helper self-delete at test end, so
// callers owe nothing to name hygiene: setupDeviceTest attaches to
// the SHARED admin, which the harness deliberately never deletes —
// unqualified rows would pile up on the live dev account forever
// (the "OPDS Folders Device" graveyard was exactly this gap).
t.Cleanup(func() { _ = s.DB.DeleteDevice(context.Background(), device.ID) })
deviceUUID, err := uuid.FromBytes(device.ID.Bytes[0:16])
require.NoError(t, err, "Should parse device ID")
@@ -809,9 +831,17 @@ func setupTestServer(t *testing.T) *TestServerSetup {
// Same passes for test DEVICES (identifier/name containing "test")
// and test SAVED FILTERS (name containing "test"): both accumulated
// across runs and poisoned later runs (duplicate identifiers, 409s,
// non-empty "initially empty" assertions).
// non-empty "initially empty" assertions). The second half of the
// device predicate catches test suites that attach to the shared
// admin with surface-specific names ("koreader" from the contract
// pins, "OPDS Folders Device", "Reinstall Device") — every suite
// now also self-deletes its rows, this is the drift safety net:
// dev_contract_% auth tokens, contract-% / opds-folders-% /
// repro-reinstall-% identifiers are unambiguously harness-owned.
_, derr := dbPool.Exec(ctx,
"DELETE FROM devices WHERE user_id = $1 AND (device_identifier ILIKE '%test%' OR device_name ILIKE '%test%')",
"DELETE FROM devices WHERE user_id = $1 AND (device_identifier ILIKE '%test%' OR device_name ILIKE '%test%' "+
"OR device_identifier ILIKE 'contract-%' OR device_identifier ILIKE 'opds-folders-%' OR device_identifier ILIKE 'repro-reinstall-%' "+
"OR auth_token LIKE 'dev_contract_%')",
adminUUID)
if derr != nil {
t.Logf("device cleanup failed: %v", derr)