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:
1 parent
0c31e9b88d
commit
7d88ec5184
5 files changed
+68
-4
No files matched your search
@@ -168,7 +168,13 @@ func createContractDeviceNamed(t *testing.T, setup *TestServerSetup, name, devic
|
|||||||
DeviceMetadata: []byte("{}"),
|
DeviceMetadata: []byte("{}"),
|
||||||
})
|
})
|
||||||
require.NoError(t, err, "Should create sync device")
|
require.NoError(t, err, "Should create sync device")
|
||||||
_ = device
|
// Self-delete at test end: the contract helper attaches to the
|
||||||
|
// SHARED admin (the web progress push adopts the device name, so
|
||||||
|
// conflict tests drive a second named source without a separate
|
||||||
|
// account) and names it for its surface — "koreader" passes every
|
||||||
|
// name-based sweep, so without this the row survived forever (294
|
||||||
|
// of them ended up on the dev account's device grid).
|
||||||
|
t.Cleanup(func() { _ = setup.DB.DeleteDevice(context.Background(), device.ID) })
|
||||||
return token
|
return token
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -19,6 +19,9 @@ import (
|
|||||||
|
|
||||||
func TestDeviceRegistrationFlow(t *testing.T) {
|
func TestDeviceRegistrationFlow(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
// Approved through the shared admin's session; the row must not
|
||||||
|
// outlive the test (name hygiene alone should never carry it).
|
||||||
|
cleanupDeviceByIdentifier(t, setup, "kindle-test-hw-id-12345")
|
||||||
|
|
||||||
// Step 1: Initiate device registration
|
// Step 1: Initiate device registration
|
||||||
regRequest := map[string]interface{}{
|
regRequest := map[string]interface{}{
|
||||||
@@ -208,6 +211,7 @@ func TestDeviceAuthentication(t *testing.T) {
|
|||||||
|
|
||||||
// Create a device directly in the database
|
// Create a device directly in the database
|
||||||
userID := getTestUserID(t, setup.DB)
|
userID := getTestUserID(t, setup.DB)
|
||||||
|
cleanupDeviceByIdentifier(t, setup, "test-device-123")
|
||||||
|
|
||||||
deviceToken := fmt.Sprintf("dev_%s", uuid.New().String())
|
deviceToken := fmt.Sprintf("dev_%s", uuid.New().String())
|
||||||
_, err := setup.DB.CreateDevice(context.Background(), database.CreateDeviceParams{
|
_, err := setup.DB.CreateDevice(context.Background(), database.CreateDeviceParams{
|
||||||
@@ -279,6 +283,10 @@ func TestApproveDeviceReapprovalRotatesToken(t *testing.T) {
|
|||||||
userID := getTestUserID(t, setup.DB)
|
userID := getTestUserID(t, setup.DB)
|
||||||
|
|
||||||
identifier := fmt.Sprintf("repro-reinstall-%s", uuid.New().String())
|
identifier := fmt.Sprintf("repro-reinstall-%s", uuid.New().String())
|
||||||
|
// Self-delete: the row is named "Reinstall Device" — the web
|
||||||
|
// reinstall flow's own label — so the name-based sweeps can never
|
||||||
|
// safely touch it. The identifier is this test's fixture alone.
|
||||||
|
cleanupDeviceByIdentifier(t, setup, identifier)
|
||||||
|
|
||||||
registerAndApprove := func() string {
|
registerAndApprove := func() string {
|
||||||
regRequest := map[string]interface{}{
|
regRequest := map[string]interface{}{
|
||||||
@@ -352,6 +360,9 @@ func TestApproveDeviceReapprovalRotatesToken(t *testing.T) {
|
|||||||
|
|
||||||
func TestApproveDeviceRegistration(t *testing.T) {
|
func TestApproveDeviceRegistration(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
// Approved through the shared admin's session; the row must not
|
||||||
|
// outlive the test (name hygiene alone should never carry it).
|
||||||
|
cleanupDeviceByIdentifier(t, setup, "test-approval-12345")
|
||||||
|
|
||||||
regRequest := map[string]interface{}{
|
regRequest := map[string]interface{}{
|
||||||
"device_name": "Test Device for Approval",
|
"device_name": "Test Device for Approval",
|
||||||
|
|||||||
@@ -438,7 +438,15 @@ func TestMetadataWriteToFile(t *testing.T) {
|
|||||||
assert.Equal(t, "Comic Series", series)
|
assert.Equal(t, "Comic Series", series)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Sweep ALL generations of the fixture dirs, not just this
|
||||||
|
// process's: a killed run never reaches its RemoveAll, and the stale
|
||||||
|
// root-owned dirs then sit on the user's bind mount forever (seven
|
||||||
|
// of them were found in uploads/ in Nov 2026, one per past run —
|
||||||
|
// each surviving run's cleanup skipped them by pid).
|
||||||
|
matches, _ := filepath.Glob(filepath.Join(uploadsDir, "test-metadata-write-*"))
|
||||||
t.Cleanup(func() {
|
t.Cleanup(func() {
|
||||||
_ = os.RemoveAll(filepath.Join(uploadsDir, fmt.Sprintf("test-metadata-write-%d", os.Getpid())))
|
for _, dir := range matches {
|
||||||
|
_ = os.RemoveAll(dir)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -96,6 +96,21 @@ type TestServerSetup struct {
|
|||||||
closed bool
|
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
|
// Close cleans up all resources in the correct order
|
||||||
func (s *TestServerSetup) Close() error {
|
func (s *TestServerSetup) Close() error {
|
||||||
s.mu.Lock()
|
s.mu.Lock()
|
||||||
@@ -481,6 +496,13 @@ func (s *TestDeviceSetup) CreateDevice(t *testing.T, deviceName, deviceType, dev
|
|||||||
})
|
})
|
||||||
require.NoError(t, err, "Should create device")
|
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])
|
deviceUUID, err := uuid.FromBytes(device.ID.Bytes[0:16])
|
||||||
require.NoError(t, err, "Should parse device ID")
|
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")
|
// Same passes for test DEVICES (identifier/name containing "test")
|
||||||
// and test SAVED FILTERS (name containing "test"): both accumulated
|
// and test SAVED FILTERS (name containing "test"): both accumulated
|
||||||
// across runs and poisoned later runs (duplicate identifiers, 409s,
|
// 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,
|
_, 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)
|
adminUUID)
|
||||||
if derr != nil {
|
if derr != nil {
|
||||||
t.Logf("device cleanup failed: %v", derr)
|
t.Logf("device cleanup failed: %v", derr)
|
||||||
|
|||||||
@@ -287,6 +287,15 @@ Test-created data rules (harness cleanup depends on them): users use
|
|||||||
`@tests.bookhoard.internal` / `@example.com` emails, libraries and
|
`@tests.bookhoard.internal` / `@example.com` emails, libraries and
|
||||||
collections carry "test"-prefixed names, and cleanups delete test
|
collections carry "test"-prefixed names, and cleanups delete test
|
||||||
libraries (cascades handle the rest) rather than table-wide deletes.
|
libraries (cascades handle the rest) rather than table-wide deletes.
|
||||||
|
Devices attach to the per-run regular user when it doesn't matter, or
|
||||||
|
self-delete via `TestDeviceSetup.CreateDevice` / `createContractDevice`
|
||||||
|
/ `cleanupDeviceByIdentifier` when they attach to the shared admin —
|
||||||
|
names lacking "test" (device-type names like "koreader", the
|
||||||
|
"Reinstall Device" label) escaped the name sweeps for weeks and piled
|
||||||
|
476 rows onto the live dev account; every suite now cleans its own
|
||||||
|
rows at test end, with the harness sweep's identifier/token patterns
|
||||||
|
(`contract-%`, `opds-folders-%`, `repro-reinstall-%`,
|
||||||
|
`dev_contract_*`) as the drift net.
|
||||||
|
|
||||||
### Media Fixtures (audio/books)
|
### Media Fixtures (audio/books)
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user