Skip to content

DB-backed storage-node tests pass vacuously (harness self-disables); repair before adding stateful ack properties #631

Description

@cryptskii

Summary

The dsm_storage_node DB-backed tests pass vacuously. maybe_state_and_auth returns None on any setup failure, and every DB test opens with:

let Some((_app_state, _auth_state, app)) = maybe_state_and_auth().await else {
    return;   // <-- reports "ok"
};

So a broken harness is indistinguishable from a passing suite. Observed: test result: ok. 1 passed in 0.01s — far too fast to have created a pool, run init_db, registered a device, and driven submit/retrieve/ack.

This is pre-existing on main and independent of #630.

Evidence

Instrumenting the helper to panic instead of returning None:

  • Against the existing dsm_storage database → DIAG init_db failed: db error. That DB has mixed table ownership (dsm vs cryptskii), so CREATE TABLE IF NOT EXISTS … fails and every DB test has been silently inert.
  • Against a fresh database (createdb dsm_ack_test) → the harness genuinely engages and enters the test body.
  • It then fails at the first POST /api/v2/b0x/submit with 401 Unauthorized — an auth-fixture gap. Reproduced identically on unmodified main, so it is not caused by fix(storage): give /b0x/ack its own minimal wire contract (route-contract mismatch broke every acknowledgement) #630.

Why it matters

Any regression in a DB-backed path is invisible: the suite is green while executing nothing. DSM_RUN_DB_TESTS=1 looks like an opt-in switch but does not actually guarantee the tests ran.

Proposed work

  1. Fail loudly. When DSM_RUN_DB_TESTS=1 is set, a setup failure must panic! with the underlying error, never return None. Self-disabling is only acceptable when the opt-in is absent.
  2. Isolated clean database. Create/drop a per-run database (or schema) rather than reusing a long-lived one with drifted ownership.
  3. Repair the auth fixture so an authorized submit reaches the handler (the current 401 blocks every stateful path).
  4. Then add the three stateful ack properties that could not be landed with fix(storage): give /b0x/ack its own minimal wire contract (route-contract mismatch broke every acknowledgement) #630:
    • real spooled row + valid minimal ack → 204 and the row becomes acked
    • repeated ack → idempotent success
    • mixed valid/invalid batch → no partial state mutation (the entry preceding the bad one stays unacked)

Item 4 was deliberately not shipped in #630 — writing tests that were never observed to pass would have left a landmine for whoever enabled the harness.

Context

#630 fixed the /b0x/ack route-contract defect and is proven by 18 ungated wire-contract tests plus a mutation reproducing the exact Err(400) seen on hardware. The stateful properties above are complementary, not a substitute.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions