Skip to content

Lift the two fixtures that had nothing left to decide - #376

Merged
haksungjang merged 1 commit into
mainfrom
er66-lift-shared-fixtures
Sep 5, 2026
Merged

Lift the two fixtures that had nothing left to decide#376
haksungjang merged 1 commit into
mainfrom
er66-lift-shared-fixtures

Conversation

@haksungjang

Copy link
Copy Markdown
Contributor

sync_session (23 files) and db_factory (3) had one body, one scope and no
autouse between them, so there was nothing left to decide about them. They move
to conftest.py and the 26 local definitions go.

Everything else stays, and the conftest records why, because the next person
counting fixtures will otherwise repeat the investigation:

  • _workspace - 12 files, one body, one scope, but autouse and spread across
    four directories. The only conftest covering all four is the top one, so
    lifting it hands an autouse fixture to the whole backend suite: 12 recipients
    becomes thousands. Its cost is one monkeypatch.setenv, but the real cost is
    establishing that none of those thousands depends on WORKSPACE_HOST_PATH
    being unset, which exceeds the value of removing 12 duplicates.
  • client (89 files, 14 bodies), app (79, 5), db_session (66, 11)
    • a majority body exists in each, but lifting it means the minority inherits
      silently unless every one of them overrides, and pytest gives no sign of which
      file diverges or why. Visible duplication beats invisible inheritance: these
      have already diverged, and the divergence would stop showing.
  • session (15 files, 9 bodies), _clean_env (12, 10) - as many shapes
    as files. There is no common version to lift.
  • Naming drift left alone but recorded, so somebody editing one finds the other:
    temp_backups/temp_backups_root, anon_client/client,
    _patch_async_client/patch_async_client.

How the comparison was done is written down too, since it decides the answer:
bodies compared with string literals and decorators kept. Erasing strings
merged eight unrelated fixtures, the strings being the payload; erasing
decorators would have merged different lifetimes, and that is what would have
made _workspace look liftable.

An unrelated failure this surfaced

Running the 26 affected files together, test_reset_demo_scope_db.py fails two
tests on a database trigger: "last active super_admin cannot be removed". It does
the same on main with the same selection and a fresh database, and the same
when run alone. What it does not do is fail against a database that earlier
runs have already populated - there it passes.

So the file depends on state it does not create, and the dependency is invisible
whenever the suite happens to supply it. Not caused by this change and not fixed
here; recorded because "passes on a used database, fails on a fresh one" is the
kind of thing that gets attributed to whatever change is in flight when it
finally shows.

sync_session (23 files) and db_factory (3) had one body, one scope and no
autouse between them, so they move to conftest and the 26 local definitions
go. Everything else stays where it is, and the reason for each is recorded
next to them: what was considered, and why lifting it would trade visible
duplication for invisible inheritance.
@haksungjang
haksungjang merged commit be479b7 into main Sep 5, 2026
24 checks passed
@haksungjang
haksungjang deleted the er66-lift-shared-fixtures branch September 5, 2026 05:54
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