Skip to content

docs(decisions): record ADR-0020 — adopt upstream's testing methodology - #291

Merged
serina-mcfall merged 2 commits into
launchpadfrom
docs/adr-0020-testing-methodology
Aug 24, 2026
Merged

docs(decisions): record ADR-0020 — adopt upstream's testing methodology#291
serina-mcfall merged 2 commits into
launchpadfrom
docs/adr-0020-testing-methodology

Conversation

@tucktuck101

Copy link
Copy Markdown
Collaborator

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

Field Value
Harness / provider Claude Code
Model claude-opus-5[1m]
Session reference N/A - the harness exposes no shareable run id
Initiating human @tucktuck101

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 : 0 plus 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.ts and 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:

grep -rh -E "#\[(tokio::)?test\]" crates | wc -l
ls crates/buzz-test-client/tests/*.rs | wc -l ; ls desktop/tests/e2e/*.spec.ts | wc -l
grep -n "retries" desktop/playwright.config.ts
gh api repos/launchpad-26/buzz/branches/launchpad/protection/required_status_checks

Raw output:

    4615

      19
     146

6:  retries: process.env.CI ? 2 : 0,

{"message":"Required status checks not enabled","documentation_url":"https://docs.github.com/rest/branches/branch-protection#get-status-checks-protection","status":"404"}

Measured in a worktree at launchpad tip db4305a4a, 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.

  • Tests or checks were run and the raw output is pasted above
  • The diff is confined to the scope of the linked issue
  • No secrets, keys, tokens or hostnames were added to tracked files

Not verified

Whether the inherited suite passes reliably. Two of the five most recent ci.yml runs 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 under launchpad/project-intelligence/ and launchpad/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 gitleaks stderr 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

  1. Quarantine becomes necessary the moment checks are required, and nothing owns it yet beyond a criterion in prd: the cohort test suite — what we test that upstream doesn't #290. Upstream needs none because upstream gates nothing; the cohort will, with 146 Playwright specs behind a required check.
  2. This record closes the methodology question while enforcement stays open elsewhere (ADR-0019, deferred to buzz-infrastructure ansible/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.
  3. The nine product-code divergences are unprotected today. Named here, owned by prd: the cohort test suite — what we test that upstream doesn't #290 and prd: the change agent — own the block/buzz → launchpad-26/buzz relationship, including conflict resolution #273's register, but not scheduled.

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 serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 suite4,615-test suite at :86, and 137 Playwright specs146 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-filter gating jobs; the #[ignore] E2E opt-in convention in TESTING.md.
  • required_status_checks on launchpad returns 404 and rulesets returns [] — I checked both surfaces, not just the legacy endpoint, so "nothing is enforced today" holds.
  • CONTRIBUTING.md line 241 says "PRs that fail just ci will 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 benmitchell11 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@serina-mcfall

Copy link
Copy Markdown

Requested changes NOT yet done — worth a look soon

Checked at head bcc8927da. No commit has landed since the review, and the blocker is unchanged. Grepping the diff still returns all three figures side by side:

:54  Measured on 2026-08-21 at `launchpad` tip `db4305a4a`: **4,615 Rust test...
:92  that are actually its own rather than duplicating a 4,342-test suite...
:102 because upstream gates nothing. With 137 Playwright specs and 19 relay...

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:

$ git grep -rhE "#\[(tokio::)?test\]" db4305a4a -- crates | wc -l
4615
$ git ls-tree -r --name-only db4305a4a -- desktop/tests/e2e | grep '\.spec\.ts$' | wc -l
146

Fix: 4,342-test suite4,615-test suite at :92, and 137 Playwright specs146 at :102. Two numbers.

Everything else in the record verified exactly — 19 relay E2E suites, retries: process.env.CI ? 2 : 0, the #[ignore] convention, required_status_checks 404 with rulesets empty, CONTRIBUTING.md's line 241, and the nine product-code divergences against #273. The non-blocking CI-job count is filed as #383.

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>
@tucktuck101

Copy link
Copy Markdown
Collaborator Author

Re-review requested — pushed 1566f61. Both stale figures in Consequences are corrected: 4,342-test suite4,615-test suite (line 86) and 137 Playwright specs146 (line 96). I re-ran both of your commands at the cited tip db4305a4a before editing and reproduced 4,615 and 146 exactly, so Context, Decision, and Consequences now carry one consistent pair. Nothing else changed; the #383 job-count follow-up stays where you filed it. Thanks for pinning the exact lines twice over — this one was pure mechanics.

@serina-mcfall serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@serina-mcfall
serina-mcfall merged commit d6ef428 into launchpad Aug 24, 2026
22 checks passed
@serina-mcfall
serina-mcfall deleted the docs/adr-0020-testing-methodology branch August 24, 2026 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci GitHub Actions, checks, automation by:agent Filed or authored by an AI agent, not a human type:adr A decision to make and record. Not a work item.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adr: testing methodology — what we test, at what level, and what evidence counts

3 participants