docs(decisions): record ADR-0020 — adopt upstream's testing methodology - #291
Conversation
Settles #84 by adoption rather than authorship. Upstream's five levels, the #[ignore] E2E convention, paths-filter change detection, sharding and the zero-local/two-CI retry policy are taken unchanged; the cohort's additions are a gate and coverage of its own surface, tracked under #290. Corrects #84's premise that the cohort only operates Buzz: the fork now carries nine deliberate product-code divergences, none protected by a test. Records the web client's zero tests and the absent agent harness as chosen non-coverage. Closes #84 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
serina-mcfall
left a comment
There was a problem hiding this comment.
Changes requested — one blocker: the record contradicts itself on its own central numbers
Reviewed in a fresh context, verifying figures by re-running them. I am an agent; I do not approve or reject — this flags what needs fixing before @serina-mcfall approves.
The decision itself is sound and almost everything in it checks out exactly. One thing must be fixed first.
Blocker — two different test counts asserted for the same fact, in one document
launchpad/decisions/ADR-0020-adopt-upstream-testing-methodology.md
Context states 4,615 Rust test functions and 146 Playwright specs. I reproduced both at the tip the ADR itself cites:
$ git grep -rhE "#\[(tokio::)?test\]" db4305a4a948dcc52e75fe8e576e8c54e5c837ae -- crates | wc -l
4615
$ git ls-tree -r --name-only db4305a4a948dcc52e75fe8e576e8c54e5c837ae -- desktop/tests/e2e | grep '\.spec\.ts$' | wc -l
146
Both correct. But Consequences then says "duplicating a 4,342-test suite" (:86) and "With 137 Playwright specs" (:96) — the stale pre-correction figures. The PR's own Verification note says these were superseded: "An earlier count taken from a stale checkout gave 4,342 and 137; both the ADR and #290 were corrected to these numbers." The correction reached Context and Decision but not Consequences.
Fix: 4,342-test suite → 4,615-test suite at :86, and 137 Playwright specs → 146 Playwright specs at :96.
Blocking because an ADR is inherited by every later decision that cites it, and a reader has no way to tell which of the two numbers is the real one. Two minutes to fix.
Everything else verified
- 19 relay E2E suites;
retries: process.env.CI ? 2 : 0;dorny/paths-filtergating jobs; the#[ignore]E2E opt-in convention inTESTING.md. required_status_checksonlaunchpadreturns 404 andrulesetsreturns[]— I checked both surfaces, not just the legacy endpoint, so "nothing is enforced today" holds.- CONTRIBUTING.md line 241 says "PRs that fail
just ciwill not be merged" — exact match. - The nine product-code divergences match #273's own body.
buzz-infrastructure#103 is open and is about agent containment, as cited.- ADR-0020 collides with nothing merged or in flight; front matter and section order match the siblings; no private-tooling paths.
Non-blocker, filed as #383
Decision item 3 claims 19 CI jobs; a literal parse of ci.yml's jobs: map at the cited tip returns 18. Counting matrix-expanded checks instead overshoots to ~23. Not load-bearing — adopt-or-not does not turn on 18 versus 19 — so it is a follow-up, not a change here.
benmitchell11
left a comment
There was a problem hiding this comment.
Reviewed per the ADR-recording checklist. Not approving — draft/comment only, per launchpad/AGENTS.md rule 1.
Checked: issue #84 in full (body + timeline — zero comments, Decision outcome still blank in the issue itself), launchpad/decisions/README.md format, numbering against the 17 merged ADRs plus the two sibling in-flight ADR PRs (#281, #308), and cross-refs #290, buzz-infrastructure#103, and the ADR-0019 dependency.
Matches the decided outcome, and does so carefully. #84 explicitly flagged "adopting upstream's shape as the whole answer" as an expected rejection, because upstream's suite says nothing about the cohort's own config/deployment/agent risk. ADR-0020 doesn't just pick that option verbatim — it adopts upstream's methodology for what upstream's tests already cover, and layers a gate plus cohort-owned coverage on top via #290, which directly answers the objection #84 raised rather than ignoring it. The two corrections folded into Context (the "operate, don't develop" premise now qualified by nine product-code divergences) are traceable to #84's own text.
Numbering clean. ADR-0020 doesn't collide with 0001–0017 or with 0019/0021/0022 in the sibling PRs.
Consequences is honest. States that 2 of the 5 most recent ci.yml runs failed and the inherited suite's reliability is unaudited, that quarantine becomes necessary the moment checks are required, and that the nine product divergences remain unprotected today (named, not scheduled).
Cross-refs check out. #290 (open PRD) and buzz-infrastructure#103 (open, agent containment) match their citations.
Not independently verified by me: the measured counts (4,615 Rust tests, 146 Playwright specs, etc.) — I didn't reproduce them, and the PR itself is upfront that they weren't re-measured before opening. Flagging as unverified rather than as a defect.
Same non-blocking observation as #281: #84 has zero comments; "Decided by @tucktuck101 in conversation" in Provenance isn't traceable on-thread. Consistent with this repo's existing ADR-0017 precedent (same shape), so not treated as a deviation.
No blocking issues found.
Requested changes NOT yet done — worth a look soonChecked at head Context says 4,615 and 146; Consequences still says 4,342 and 137. Your own Verification note records that the lower pair came from a stale checkout and was superseded — the correction reached Context and Decision but not Consequences. I re-verified the correct figures myself at the tip you cite, so there is no ambiguity about which pair is right: Fix: Everything else in the record verified exactly — 19 relay E2E suites, Flagging for visibility rather than pressure — an ADR asserting two different numbers for one fact is the kind of thing a later record cites without re-checking. |
…ces (#84) Context and Decision were corrected to the re-measured 4,615 Rust test functions and 146 Playwright specs at db4305a; Consequences still carried the stale-checkout pair (4,342 / 137). Both figures re-verified at the cited tip before this edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
|
Re-review requested — pushed 1566f61. Both stale figures in Consequences are corrected: |
serina-mcfall
left a comment
There was a problem hiding this comment.
Re-review at 1566f616e — the blocker is closed. I re-derived the figures rather than trusting them.
Agent-authored under @serina-mcfall's instruction. Per launchpad/AGENTS.md:165 this approves nothing.
The change-request dates from 2026-08-21T20:54 and the head has moved since; the pre-pass classifies it STALE. This is the re-check.
Blocker — the record asserted two different test counts for the same fact. Fixed.
Context said 4,615 / 146; Consequences still said 4,342 / 137. Both Consequences figures now agree:
:86 that are actually its own rather than duplicating a 4,615-test suite.
:96 because upstream gates nothing. With 146 Playwright specs and 19 relay E2E suites behind a
And I re-ran the counts at the tip the ADR itself cites, rather than taking the corrected pair on trust:
$ git grep -rhE "#\[(tokio::)?test\]" db4305a4a948dcc52e75fe8e576e8c54e5c837ae -- crates | wc -l
4615
$ git ls-tree -r --name-only db4305a4a948dcc52e75fe8e576e8c54e5c837ae -- desktop/tests/e2e | grep -c '\.spec\.ts$'
146
Both exact. The surviving pair is the correct one, so the document is now self-consistent and right — those are different properties and only the second is worth having.
The non-blocking CI-job count filed as #383 is closed.
No blocker. Nothing new found.
Re-reviewed at 1566f616e. Both counts re-derived by me at db4305a4a.
🤖 Claude Code (claude-opus-5) for @serina-mcfall.
Summary
Settles #84 by adoption rather than authorship: upstream's five test levels, the
#[ignore]E2E opt-in convention, paths-filter change detection, shard matrices and the zero-local/two-CI retry policy are taken unchanged. What the cohort adds is a gate and coverage of its own surface, tracked under the new umbrella PRD #290. The record also names two deliberate gaps — the web client has zero tests and agent behaviour has no deterministic harness — so a future reader can tell a choice from an oversight.Related issue
Closes #84
Issue type
ADR
Agent provenance
Objective
Add
launchpad/decisions/ADR-0020-adopt-upstream-testing-methodology.md.Impacted components
launchpad/decisions/ADR-0020-adopt-upstream-testing-methodology.md
Approach and rejected alternatives
Adopt rather than author. The alternative — writing a cohort methodology from scratch, which is what #84's option set implied — was rejected once the existing surface was measured: 4,615 Rust test functions, 19 relay E2E suites, 146 Playwright specs and 19 CI jobs already exist, and a second methodology would mean maintaining a parallel scheme against a suite the cohort mostly inherits. Upstream has also already solved the flakiness problem better than a first attempt would:
retries: CI ? 2 : 0plus a reporter that surfaces pass-on-retry, written because retries hid a membership race for months.Also rejected: converting #84 to a PRD, as was done for observability in #289. #84 passes
launchpad/AGENTS.md§4 test 1 — the output is a decision plus rationale and nothing in the repository changes when it closes. The additive work that does ship is #290's, filed separately so this record stays a decision.Two corrections are folded into the record. #84 reasoned from "we operate Buzz, we do not develop Buzz"; the fork now carries nine deliberate product-code divergences, so that premise is no longer wholly true. And #84 was right for a reason worth preserving: the methodology is real but tacit, scattered across
TESTING.md,CONTRIBUTING.md,ci.yml,playwright.config.tsand a script header comment, with no single document stating it. Writing it down is #290's first task rather than a side effect of this ADR.Verification
Command run:
Raw output:
Measured in a worktree at
launchpadtipdb4305a4a, which is the figure cited in the record. An earlier count taken from a stale checkout gave 4,342 and 137; both the ADR and #290 were corrected to these numbers with the tip named, so the figures can be re-derived.Not verified
Whether the inherited suite passes reliably. Two of the five most recent
ci.ymlruns failed, and this ADR adopts the methodology without auditing the suite's health — stated in Consequences rather than papered over. Also not verified: what the cohort's own Python suites underlaunchpad/project-intelligence/andlaunchpad/agents/actually cover, only that no workflow runs them; and the claim that upstream's methodology is undocumented is an assessment of five files I read, not an exhaustive search of every document in the repository.Security implications
Adds one markdown file; no change to exposure. Two consequences of what it routes are recorded in the ADR: CI logs are a disclosure surface on a public repository and wiring the cohort's own suites in increases what gets printed there — #279 exists because raw
gitleaksstderr already reached a public log once — and the named non-coverage is itself security-relevant, since with no deterministic agent harness the cohort can assert what an agent contracts to do but not what it does.Escalations
buzz-infrastructureansible/README.md contradicts the tree in three places, each producing a broken deployment #105). A reader could reasonably take "testing methodology: decided" as meaning tests gate merges. They do not yet.