Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4bc92c44eb | ||
|
|
8aa21356af | ||
|
|
eecc4aaa51 | ||
|
|
eb1d6d9161 | ||
|
|
0a229ac016 | ||
|
|
de1eb1ca75 |
@@ -0,0 +1,8 @@
|
|||||||
|
{
|
||||||
|
"$schema": "https://opencode.ai/config.json",
|
||||||
|
"agent": {
|
||||||
|
"general": {
|
||||||
|
"model": "opencode/deepseek-v4-pro"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
+116
@@ -0,0 +1,116 @@
|
|||||||
|
# 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:
|
||||||
|
|
||||||
|
```python
|
||||||
|
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 10` → `createdb` → `migrate` → 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.
|
||||||
@@ -14,7 +14,11 @@ Both assert real app state (the event reached the analytics store), not just the
|
|||||||
|
|
||||||
plausible only ingests events for *known* sites — the in-memory `sites_cache` gates ingestion and
|
plausible only ingests events for *known* sites — the in-memory `sites_cache` gates ingestion and
|
||||||
drops events for unregistered domains (empirically confirmed: an event for an unregistered domain
|
drops events for unregistered domains (empirically confirmed: an event for an unregistered domain
|
||||||
never appears in events_v2). So each test first registers a site row in the metadata postgres, then
|
never appears in events_v2). Sites are therefore provisioned through plausible's OWN creation path
|
||||||
|
rather than by writing rows — under v3 a site must belong to a TEAM, and a teamless site is dropped as
|
||||||
|
`dropped_not_found` while the POST still acks 202, which reads as a silent ingestion stall. Letting the
|
||||||
|
app create the site sidesteps that entirely, and works unchanged on v2. So each test first provisions
|
||||||
|
the site, then
|
||||||
POSTs repeatedly while polling ClickHouse: the sites_cache must refresh to admit the new site and the
|
POSTs repeatedly while polling ClickHouse: the sites_cache must refresh to admit the new site and the
|
||||||
event write-buffer must flush to ClickHouse, so the first landing is not instantaneous. Re-POSTing the
|
event write-buffer must flush to ClickHouse, so the first landing is not instantaneous. Re-POSTing the
|
||||||
same event is safe — we assert the row count is >= 1.
|
same event is safe — we assert the row count is >= 1.
|
||||||
@@ -40,6 +44,10 @@ _UA = (
|
|||||||
"(KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36"
|
"(KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Identity the harness provisions sites under. Ephemeral per-run deploy, never a real account.
|
||||||
|
_HARNESS_EMAIL = "cc-ci@ci.invalid"
|
||||||
|
_HARNESS_PW = "ccci-harness-passphrase-2026"
|
||||||
|
|
||||||
|
|
||||||
def _ch(domain: str, sql: str) -> str:
|
def _ch(domain: str, sql: str) -> str:
|
||||||
"""Run a ClickHouse query against the `plausible_events_db` service; return stdout (stripped)."""
|
"""Run a ClickHouse query against the `plausible_events_db` service; return stdout (stripped)."""
|
||||||
@@ -50,18 +58,61 @@ def _ch(domain: str, sql: str) -> str:
|
|||||||
).strip()
|
).strip()
|
||||||
|
|
||||||
|
|
||||||
|
# plausible's own provisioning path. Creating a site through the app (rather than INSERTing rows)
|
||||||
|
# means the app applies whatever its current data model requires — which is what makes this work
|
||||||
|
# unchanged across the v2→v3 jump, where sites gained a mandatory owning TEAM. Verified on cc-ci
|
||||||
|
# against BOTH v2.0.0 (no `teams` table at all) and v3.2.1: identical expression, site usable, events
|
||||||
|
# ingested. See tests/STYLE.md.
|
||||||
|
#
|
||||||
|
# The HTTP provisioning API (`POST /api/v1/sites`) would be the first choice, but it is gated behind
|
||||||
|
# a paid plan — on CE it answers `:upgrade_required` — so the app's release console is the closest
|
||||||
|
# public interface available here.
|
||||||
|
_PROVISION_SITE_EXS = """
|
||||||
|
pw = "__PW__"
|
||||||
|
email = "__EMAIL__"
|
||||||
|
user =
|
||||||
|
case Plausible.Auth.find_user_by(email: email) do
|
||||||
|
nil ->
|
||||||
|
{:ok, u} =
|
||||||
|
Plausible.Auth.User.new(%{name: "cc-ci", email: email, password: pw, password_confirmation: pw})
|
||||||
|
|> Plausible.Repo.insert()
|
||||||
|
u
|
||||||
|
u -> u
|
||||||
|
end
|
||||||
|
site = "__SITE__"
|
||||||
|
result =
|
||||||
|
case Plausible.Sites.get_by_domain(site) do
|
||||||
|
nil -> Plausible.Sites.create(user, %{"domain" => site, "timezone" => "UTC"})
|
||||||
|
s -> {:ok, s}
|
||||||
|
end
|
||||||
|
case result do
|
||||||
|
{:ok, _} -> IO.puts("CCCI_SITE_OK " <> site)
|
||||||
|
other -> IO.puts("CCCI_SITE_ERR " <> inspect(other))
|
||||||
|
end
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
def _register_site(domain: str, site: str) -> None:
|
def _register_site(domain: str, site: str) -> None:
|
||||||
"""Insert a site row into the metadata postgres (`db` service) so plausible will ingest events for
|
"""Provision `site` via plausible's own site-creation path, so it is a site the app will ingest for.
|
||||||
it. Idempotent (ON CONFLICT DO NOTHING)."""
|
|
||||||
sql = (
|
Idempotent: an existing domain is reused rather than re-created.
|
||||||
"INSERT INTO sites (domain, timezone, inserted_at, updated_at, native_stats_start_at) "
|
|
||||||
f"VALUES ('{site}','UTC', now(), now(), now()) ON CONFLICT (domain) DO NOTHING; "
|
Do NOT reach into postgres to do this. A `sites` INSERT was enough under v2, but v3 requires the
|
||||||
f"SELECT domain FROM sites WHERE domain = '{site}';"
|
site to belong to a TEAM and silently discards events for a teamless site — `POST /api/event`
|
||||||
|
still acks 202 and the row still exists, so the only symptom is that nothing reaches ClickHouse
|
||||||
|
(ClickHouse's own `ingest_counters` records it as `dropped_not_found`). That is what put this
|
||||||
|
recipe RED on build 1224. Going through the app removes the whole class of problem: it provisions
|
||||||
|
the team itself.
|
||||||
|
"""
|
||||||
|
exs = (
|
||||||
|
_PROVISION_SITE_EXS.replace("__PW__", _HARNESS_PW)
|
||||||
|
.replace("__EMAIL__", _HARNESS_EMAIL)
|
||||||
|
.replace("__SITE__", site)
|
||||||
|
)
|
||||||
|
out = lifecycle.exec_in_app(domain, ["/app/bin/plausible", "rpc", exs], service="app")
|
||||||
|
assert f"CCCI_SITE_OK {site}" in out, (
|
||||||
|
f"could not provision site {site!r} via the app: {out.strip()[-400:]}"
|
||||||
)
|
)
|
||||||
out = lifecycle.exec_in_app(
|
|
||||||
domain, ["psql", "-q", "-U", "plausible", "-d", "plausible", "-tAc", sql], service="db"
|
|
||||||
).strip()
|
|
||||||
assert out == site, f"site {site!r} not registered in postgres (got {out!r})"
|
|
||||||
|
|
||||||
|
|
||||||
def _post_event(base_domain: str, site: str, name: str, pathname: str) -> int:
|
def _post_event(base_domain: str, site: str, name: str, pathname: str) -> int:
|
||||||
@@ -93,9 +144,9 @@ def _ingest_and_count(
|
|||||||
last_status = None
|
last_status = None
|
||||||
while True:
|
while True:
|
||||||
last_status = _post_event(base_domain, site, name, pathname)
|
last_status = _post_event(base_domain, site, name, pathname)
|
||||||
assert (
|
assert last_status == 202, (
|
||||||
last_status == 202
|
f"POST /api/event for {name!r} → HTTP {last_status} (expected 202)"
|
||||||
), f"POST /api/event for {name!r} → HTTP {last_status} (expected 202)"
|
)
|
||||||
time.sleep(interval)
|
time.sleep(interval)
|
||||||
raw = _ch(base_domain, count_sql)
|
raw = _ch(base_domain, count_sql)
|
||||||
count = int(raw) if raw.isdigit() else 0
|
count = int(raw) if raw.isdigit() else 0
|
||||||
@@ -143,6 +194,6 @@ def test_custom_event_roundtrip(live_app):
|
|||||||
live_app,
|
live_app,
|
||||||
f"SELECT name FROM events_v2 WHERE pathname = '{pathname}' LIMIT 1",
|
f"SELECT name FROM events_v2 WHERE pathname = '{pathname}' LIMIT 1",
|
||||||
)
|
)
|
||||||
assert (
|
assert stored_name == event_name, (
|
||||||
stored_name == event_name
|
f"custom event stored as {stored_name!r}, expected {event_name!r}"
|
||||||
), f"custom event stored as {stored_name!r}, expected {event_name!r}"
|
)
|
||||||
|
|||||||
@@ -17,6 +17,12 @@ def test_plausible_root_serves(live_app):
|
|||||||
62-char SECRET_KEY_BASE, see recipe_meta.EXTRA_ENV); the dedicated
|
62-char SECRET_KEY_BASE, see recipe_meta.EXTRA_ENV); the dedicated
|
||||||
/api/health endpoint is.
|
/api/health endpoint is.
|
||||||
"""
|
"""
|
||||||
|
# The custom tier runs AFTER the backup/restore tier, which disrupts postgres under the app and
|
||||||
|
# restarts it. v3 (community-edition) then boots through `sleep 10` + `db createdb` + `db migrate`
|
||||||
|
# + cache warmers before /api/health flips to 200, which does not fit in 60s — that is what put
|
||||||
|
# this recipe RED on build 1224 while install/upgrade/backup/restore all passed. The assertion is
|
||||||
|
# unchanged (still a hard 200 from the real readiness endpoint); only the wait matches the boot
|
||||||
|
# profile the recipe already declares via recipe_meta.HTTP_TIMEOUT (1200).
|
||||||
url = f"https://{live_app}/api/health"
|
url = f"https://{live_app}/api/health"
|
||||||
status, _ = harness_http.retry_http_get(url, expect_status=(200,), max_wait=60, interval=3)
|
status, _ = harness_http.retry_http_get(url, expect_status=(200,), max_wait=300, interval=5)
|
||||||
assert status == 200, f"GET {url} HTTP {status}"
|
assert status == 200, f"GET {url} HTTP {status}"
|
||||||
|
|||||||
Reference in New Issue
Block a user