Files
cc-ci/tests/STYLE.md
T
cc-ci eecc4aaa51
continuous-integration/drone/push Build is failing
test(plausible): provision the site through the app, not SQL; add tests/STYLE.md
The first cut of this fix added a team INSERT next to the existing sites INSERT.
That fixed the symptom and kept the cause: a fixture that knows the apps table
layout breaks whenever the app changes it, which is exactly what happened here.

_register_site now calls Plausible.Sites.create/2 through the app release console,
so the app provisions whatever its data model currently requires - including the
team it introduced in v3. Verified against BOTH versions on cc-ci: the identical
expression works on v2.0.0, which has no teams table at all, and on v3.2.1. No
version gate is needed because the fixture no longer depends on the schema.

The HTTP provisioning API (POST /api/v1/sites) would have been first choice, but
it is gated behind a paid plan and answers :upgrade_required on CE. That is
recorded in the code so the next person does not re-derive it.

tests/STYLE.md writes the rule down, along with the others this failure exercised:
gate on version rather than supporting both schemas (old-version tests can just be
deleted - the older version is only exercised through the upgrade tier); correct
the fixture or the wait but never the assertion; assert stored state rather than a
202 ack; size waits from the recipes declared readiness; and read the apps own
telemetry before deciding a test is stale.

Full cold suite against the recipe PR head: level 5 of 5, GREEN.
2026-08-11 14:50:41 +00:00

6.1 KiB

cc-ci test style guide

Rules for writing and changing tests under tests/. Read this before any test edit — in particular before a /recipe-upgrade <recipe> --with-tests or /ci-test-review fix, where the temptation is to make a red run green rather than to make the test right.

The tests are the independent gate on recipe upgrades. Their value is entirely in being hard to fool, so every rule below exists to keep them (a) honest and (b) alive across upgrades.


1. Set up state through the application, not its database

Order of preference for any fixture that must create state:

  1. The app's public HTTP API.
  2. The app's official CLI or release console (docker exec … <app-cli>).
  3. Writing rows into its database — last resort only, and only with a comment saying which of the above were tried and why they did not work.

Direct SQL couples the test to the app's internal schema, which upgrades are free to change. The app's own interface is the thing it promises to keep working.

Why this rule exists. tests/plausible/custom/test_event_tracking.py used to register its test site with INSERT INTO sites (...). That was sufficient for plausible v2. In v3 a site must belong to a team, and the app silently discards events for a teamless site — POST /api/event still returns 202 and the row is still in postgres, so the only visible symptom was that nothing ever reached ClickHouse. It read as a mysterious ingestion stall and held the recipe RED for six weeks.

The fix was not to also INSERT a team row. It was to stop writing rows: the fixture now calls Plausible.Sites.create/2 through the app's release console, and the app provisions whatever its data model currently needs. The same expression works unchanged on v2 (which has no teams table at all) and v3 — not because the test handles both, but because it stopped depending on the schema.

When the ideal interface is unavailable, say so in the code. plausible's HTTP provisioning API (POST /api/v1/sites) is gated behind a paid plan and answers :upgrade_required on CE, so the test drops to option 2 and records that in a comment.

2. Gate on version rather than writing dual-path fixtures

If a behaviour genuinely only exists from version X, gate the test on the version instead of branching inside it:

pytest.mark.skipif(app_version < (3,), reason="teams were introduced in v3")

Do not write a fixture that carefully supports both schemas. Version-portable code is harder to read, harder to trust, and quietly rots once nobody runs the old path.

Corollary: old tests can simply be deleted once the fleet has moved past that version. The older version is only ever exercised through the upgrade tier (deploy base → upgrade → assert), so tests that only make sense for a superseded version are dead weight, not coverage.

Prefer §1 first: an app-level fixture often makes the version difference disappear, and then no gate is needed at all.

3. Never weaken an assertion to turn a run green

There is a hard line between these two, and only the second is allowed as a way out of a red run:

  • Weakening — relaxing what is asserted: dropping a field check, accepting a wider status set, asserting a 202 ack instead of the stored result, deleting the read-back.
  • Correcting the fixture or the wait — fixing how the test sets up or how long it allows, with the assertion untouched.

If a test can only pass by asserting less, it has found a real regression. Report it; do not edit it.

4. Assert real state, not acknowledgements

An HTTP 202 means "accepted", not "done". Read the effect back out of the system that owns it — the row in the analytics store, the file on disk, the record in the API — and assert on the values you sent. plausible's ingestion returns 202 for events it goes on to discard entirely; a test that stopped at the ack would have been permanently, silently green.

5. Derive waits from the recipe's declared readiness, not a guess

A per-recipe recipe_meta.py already declares DEPLOY_TIMEOUT / HTTP_TIMEOUT because someone measured that app's boot profile. A custom test that hard-codes a shorter window contradicts it and will flake or fail on a slower version.

Remember the tier order: custom runs after backup/restore, which disrupts the datastore and restarts the app. A window sized for a warm app is not sized for that. plausible's health check allowed 60s; v3 boots through sleep 10createdbmigrate → cache warmers first.

6. Diagnose from the app's own telemetry before touching a test

Before concluding a test is stale, find the app's account of what happened. It is usually definitive and it stops you fixing the wrong thing. plausible records dropped events in ClickHouse's ingest_counters: dropped_not_found with 0 rows before the fix, buffered with rows after — that single counter identified the root cause after the HTTP status had suggested everything was fine.

Prove the diagnosis both ways where you can: same input, broken state → symptom; corrected state → no symptom.

7. Fixtures must be idempotent

A fixture may run against a warm canonical, a restored volume, or a re-run. Creating state must be safe to repeat — look the object up first and reuse it, rather than assuming a clean database.

8. Keep test identities obviously synthetic

Use ccci--prefixed names and .example / .invalid domains for anything a test creates, so state it leaves behind is instantly attributable and can never be confused with real data.


Changing a test: the checklist

  1. Reproduce the failure and get the app's own explanation (§6).
  2. Classify: recipe bug, or stale test? Only a stale test justifies a test edit.
  3. Fix the fixture, wait, or setup — never the assertion (§3).
  4. Prefer the app's interface over its database (§1); gate on version rather than branching (§2).
  5. Verify green against the recipe PR head with the changed test, plus a regression sample.
  6. Say in the commit and PR what evidence proves the diagnosis, not just what changed.