Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fc68e5ef6e |
@@ -175,6 +175,36 @@ def resolve_upgrade_base(
|
|||||||
flush=True,
|
flush=True,
|
||||||
)
|
)
|
||||||
rec = None
|
rec = None
|
||||||
|
# STALE-CANONICAL GUARD (phase relbase): the canonical is only "last-green", NOT "current". Its
|
||||||
|
# promotion can fail for reasons unrelated to any PR (the WC5 promote deploys a warm-<recipe> app
|
||||||
|
# and health-checks it), and a failed promote leaves the canonical pinned at an OLD release
|
||||||
|
# indefinitely — gitea sat at 3.5.3+1.24.2 from 2026-06-17 to 2026-08-10 because warm-gitea
|
||||||
|
# crash-looped on a read-only app.ini. Basing the upgrade tier on that stale release tests a
|
||||||
|
# transition no deployment performs, and can silently MISS breaks in the transition users do
|
||||||
|
# perform: gitea 3.5.3→head crosses an APP_INI_VERSION change (v21→v22) so Swarm creates a fresh
|
||||||
|
# config, while the real 3.6.1→3.6.2 upgrade keeps v22 and aborts on Swarm's immutable-config
|
||||||
|
# rule. Prefer the newest published release older than head whenever it is newer than the
|
||||||
|
# canonical: that is what real installs upgrade from.
|
||||||
|
if rec and rec.get("version") and not skip_canonicals and head_version:
|
||||||
|
_rel_tags = warm_reconcile.recipe_tags(recipe)
|
||||||
|
if floor:
|
||||||
|
_rel_tags = [t for t in _rel_tags if not _below_floor(t)]
|
||||||
|
newest_rel = warm_reconcile.newest_older_version(_rel_tags, head_version)
|
||||||
|
if newest_rel and warm_reconcile.version_key(newest_rel) > warm_reconcile.version_key(
|
||||||
|
rec["version"]
|
||||||
|
):
|
||||||
|
print(
|
||||||
|
f"== upgrade base: newest published release {newest_rel} is NEWER than the "
|
||||||
|
f"last-green canonical {rec['version']} — using the release (what deployments "
|
||||||
|
f"actually upgrade from); canonical is stale, check its WC5 promote",
|
||||||
|
flush=True,
|
||||||
|
)
|
||||||
|
return BasePlan(
|
||||||
|
"version",
|
||||||
|
newest_rel,
|
||||||
|
None,
|
||||||
|
f"newest published release older than head (canonical {rec['version']} is stale)",
|
||||||
|
)
|
||||||
if rec and rec.get("version") and not skip_canonicals:
|
if rec and rec.get("version") and not skip_canonicals:
|
||||||
canon = rec["version"]
|
canon = rec["version"]
|
||||||
same = head_version is not None and warm_reconcile.version_key(
|
same = head_version is not None and warm_reconcile.version_key(
|
||||||
|
|||||||
-116
@@ -1,116 +0,0 @@
|
|||||||
# 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,11 +14,7 @@ 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). Sites are therefore provisioned through plausible's OWN creation path
|
never appears in events_v2). So each test first registers a site row in the metadata postgres, then
|
||||||
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.
|
||||||
@@ -44,10 +40,6 @@ _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)."""
|
||||||
@@ -58,61 +50,18 @@ 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:
|
||||||
"""Provision `site` via plausible's own site-creation path, so it is a site the app will ingest for.
|
"""Insert a site row into the metadata postgres (`db` service) so plausible will ingest events for
|
||||||
|
it. Idempotent (ON CONFLICT DO NOTHING)."""
|
||||||
Idempotent: an existing domain is reused rather than re-created.
|
sql = (
|
||||||
|
"INSERT INTO sites (domain, timezone, inserted_at, updated_at, native_stats_start_at) "
|
||||||
Do NOT reach into postgres to do this. A `sites` INSERT was enough under v2, but v3 requires the
|
f"VALUES ('{site}','UTC', now(), now(), now()) ON CONFLICT (domain) DO NOTHING; "
|
||||||
site to belong to a TEAM and silently discards events for a teamless site — `POST /api/event`
|
f"SELECT domain FROM sites WHERE domain = '{site}';"
|
||||||
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:
|
||||||
@@ -144,9 +93,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 last_status == 202, (
|
assert (
|
||||||
f"POST /api/event for {name!r} → HTTP {last_status} (expected 202)"
|
last_status == 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
|
||||||
@@ -194,6 +143,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 stored_name == event_name, (
|
assert (
|
||||||
f"custom event stored as {stored_name!r}, expected {event_name!r}"
|
stored_name == event_name
|
||||||
)
|
), f"custom event stored as {stored_name!r}, expected {event_name!r}"
|
||||||
|
|||||||
@@ -17,12 +17,6 @@ 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=300, interval=5)
|
status, _ = harness_http.retry_http_get(url, expect_status=(200,), max_wait=60, interval=3)
|
||||||
assert status == 200, f"GET {url} HTTP {status}"
|
assert status == 200, f"GET {url} HTTP {status}"
|
||||||
|
|||||||
Reference in New Issue
Block a user