Skip to content

Unit suite leaves ./ad_seller.db behind, so a second consecutive local run can fail #82

Description

@atc964

Symptom

Running the unit suite twice in a row from the same working directory, with no other changes in between, can turn a passing run into a failing one:

tests/unit/test_trust_tier_verification.py::TestNegotiationMessagesCeiling::test_self_asserted_advertiser_identity_is_floored

fails with:

    assert resp.status_code == 200, resp.text
>   ctx = counter.await_args.kwargs["buyer_context"]
          ^^^^^^^^^^^^^^^^^^^^^^^^^
E   AttributeError: 'NoneType' object has no attribute 'kwargs'

tests/unit/test_trust_tier_verification.py:562: AttributeError

counter.await_args is None because the mocked counter_proposal was never called on the second run — the endpoint short-circuited and returned a cached idempotent response instead.

Reproduction

From a fresh origin/main checkout (worktree at commit d540b58, clean tree, no ad_seller.db present):

  1. uv sync --extra dev
  2. uv run pytest tests/unit -q → 1467 passed, and a fresh ./ad_seller.db is created in the working directory (via the default database_url = "sqlite:///./ad_seller.db").
  3. uv run pytest tests/unit -q again, no files deleted in between → 1 failed, 1466 passed, with the failure above.
  4. Ran a third time, no deletion → same failure, same test, consistently.
  5. rm -f ad_seller.db* and run once more → 1467 passed again.

So: green → red → red → green (after deleting the leftover db), with nothing else changed between runs. This is a clean causal reproduction, not a flake.

Mechanism

  • Settings.database_url (src/ad_seller/config/settings.py) defaults to the relative path sqlite:///./ad_seller.db.
  • get_storage() in src/ad_seller/storage/factory.py lazily creates and caches a single process-level SQLiteBackend instance (_storage_instance) backed by that file.
  • The negotiation endpoint's idempotency check (src/ad_seller/interfaces/api/routers/negotiation.py) stores the response for a given idempotency_key in that same storage backend, keyed as idempotency:negotiation:{buyer_pricing_key}:{idempotency_key}.
  • The failing test uses a hardcoded literal idempotency key ("idem-1") and exercises the real app (via ASGITransport) rather than a storage double, so it goes through the real singleton/backend.
  • On the first run, the file doesn't exist yet, so the key is unused, the mock gets called, and the response is written to ./ad_seller.db.
  • On any subsequent run in the same directory, the file still exists with that key already populated, so the idempotency check short-circuits and returns the cached response — the mock is never invoked, and the test's assertion on counter.await_args blows up on None.
  • Nothing in tests/conftest.py points the unit suite's storage at a temp path, resets the _storage_instance singleton, or cleans up ad_seller.db between sessions.

Why CI doesn't see this

Every CI run (and every fresh git clone) starts with no ad_seller.db present, so the first invocation is always the only invocation, and it's always green. The failure only shows up for a developer running the suite more than once from the same working directory without deleting the file in between — which is a very normal thing to do locally.

Why it matters

A local "run the unit suite again" after making an edit is one of the most basic feedback loops in a change-review workflow. If a second consecutive run in the same directory can fail independent of the actual code change under test, that feedback loop can't be trusted, and it teaches people to shrug off red output ("oh, just delete the db file and rerun") instead of treating it as a signal.

Suggested direction

I have not included a fix. A plausible direction: an autouse fixture (probably in tests/conftest.py or a tests/unit/conftest.py) that points the unit suite at a tmp_path-scoped database URL and resets ad_seller.storage.factory._storage_instance to None before/after each test (or at least each session), rather than relying on developers to remember to delete ad_seller.db by hand.

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