Skip to content

test: one way to install app state, and a CI run that cannot hide a leak - #18

Closed
glatinone wants to merge 1 commit into
fix/lifecycle-update-modelfrom
test/shared-app-state
Closed

glatinone wants to merge 1 commit into
fix/lifecycle-update-modelfrom
test/shared-app-state

Conversation

@glatinone

Copy link
Copy Markdown
Owner

test: one way to install app state, and a CI run that cannot hide a leak

Two test files reached the HTTP app through state a previous file had left on a
module global. tests/test_memory_crud.py failed four of its own tests and
tests/test_error_shape.py two when run on their own - and they pass in a
single-process run, which is what CI does and what this session had been doing.
A test that only passes in the company of another test is not measuring what it
claims to.

Root cause is the shape, not the two files: five files had each grown their own
slightly different installer (_install_app, _client, _install_app_state,
an autouse fixture), because nothing shared existed. That is now one function.

  • conftest.install_app_state(**overrides) installs fresh storage, engine, key
    store and scoring limiter, and returns what it installed; app_state and
    app_client fixtures build on it. It takes the knobs the files actually vary -
    api_key_store, scoring_limit, scoring_clock, lifecycle_settings,
    retention_days - so test_auth, test_ratelimit, test_scheduler keep their
    specifics in a two-line adapter instead of a ten-line copy.
  • Six files migrated; the duplicated installers are gone. test_transitions' three
    HTTP tests also lost the created_at they echoed back, which the previous PR
    made unnecessary.
  • CI runs the server suite one process per file. The shell already runs with
    -e, so the first failing file stops the build. This is the discipline
    scripts/run_tests.sh enforces in the project this repo borrows its test
    practice from, and it is what makes the fix stick: without it, someone
    reintroduces a cross-file dependency and the build stays green because the order
    happens to work.
  • CONTRIBUTING.md states the rule and shows the fixture pattern, so the next
    file has an obvious right way to do it rather than a blank page.

Verified with the CI loop itself, run locally: all 16 files pass alone, 244 tests,
26 skipped (the Postgres half runs in its own job).

Two test files reached the HTTP app through state a *previous* file had left on a
module global. `tests/test_memory_crud.py` failed four of its own tests and
`tests/test_error_shape.py` two when run on their own - and they pass in a
single-process run, which is what CI does and what this session had been doing.
A test that only passes in the company of another test is not measuring what it
claims to.

Root cause is the shape, not the two files: five files had each grown their own
slightly different installer (`_install_app`, `_client`, `_install_app_state`,
an autouse fixture), because nothing shared existed. That is now one function.

- `conftest.install_app_state(**overrides)` installs fresh storage, engine, key
  store and scoring limiter, and returns what it installed; `app_state` and
  `app_client` fixtures build on it. It takes the knobs the files actually vary -
  `api_key_store`, `scoring_limit`, `scoring_clock`, `lifecycle_settings`,
  `retention_days` - so `test_auth`, `test_ratelimit`, `test_scheduler` keep their
  specifics in a two-line adapter instead of a ten-line copy.
- Six files migrated; the duplicated installers are gone. `test_transitions`' three
  HTTP tests also lost the `created_at` they echoed back, which the previous PR
  made unnecessary.
- **CI runs the server suite one process per file.** The shell already runs with
  `-e`, so the first failing file stops the build. This is the discipline
  `scripts/run_tests.sh` enforces in the project this repo borrows its test
  practice from, and it is what makes the fix stick: without it, someone
  reintroduces a cross-file dependency and the build stays green because the order
  happens to work.
- `CONTRIBUTING.md` states the rule and shows the fixture pattern, so the next
  file has an obvious right way to do it rather than a blank page.

Verified with the CI loop itself, run locally: all 16 files pass alone, 244 tests,
26 skipped (the Postgres half runs in its own job).
@glatinone

Copy link
Copy Markdown
Owner Author

Landed on master in the v0.1.0 chain: the branch was fast-forward merged as part of b940905..92b88ee and released as v0.1.0. Closing so the open list matches reality - the commits are in master, and the tag points at them.

@glatinone glatinone closed this Oct 4, 2026
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.

1 participant