Skip to content

docs(tests) document Stern Playwright asymmetry + add pinning test#70

Merged
jkeeley2073 merged 1 commit into
mainfrom
Dev-SternPlaywrightAsymmetry
May 4, 2026
Merged

docs(tests) document Stern Playwright asymmetry + add pinning test#70
jkeeley2073 merged 1 commit into
mainfrom
Dev-SternPlaywrightAsymmetry

Conversation

@jkeeley2073
Copy link
Copy Markdown
Contributor

Summary

Closes docs/build-spec.md Phase 2 § Scope item 8 (Wave 2) via route (ii).

The family-wide scraper-pipeline integration test infrastructure (FakePolitenessGate + QueueingHttpMessageHandler) wires at the typed-HttpClient layer and covers 8 of 10 ISourceScraper implementations. The two remaining ones — Stern's GamePageScraper and ServiceBulletinScraper — drive a real Chromium browser via Playwright's IBrowserContext / IPage APIs and never call HttpClient.GetAsync, so the shared infra cannot intercept their page-load traffic.

This PR resolves the asymmetry by documenting it permanently plus adding a pinning test that fails if the documentation is removed.

What ships

  • tests/PinballWizard.Scraper.Tests/README.md — first README for the test project. Describes the test-project layout, the family-wide infra and the proven 5-test template, the 8 covered scrapers, the Stern Playwright asymmetry section (why the template doesn't fit, what coverage exists instead, and three concrete revisit triggers), and engineering conventions for new tests.
  • tests/PinballWizard.Scraper.Tests/Scraping/Stern/SternPlaywrightAsymmetryDocumentationTests.cs — single Fact pinning test (Stern_Playwright_Pipeline_Test_Asymmetry_IsAcknowledged, the identifier mandated by build-spec.md § Scope item 8). Asserts the README exists at the expected path and contains six load-bearing markers including the unique phrase "deliberately not covered" (catches the gutted-section false-pass case).
  • docs/guardrails.md R4 — risk row updated from "Known" to "Resolved (route ii)" and now points at the README + pinning test. A search from the guardrails risk register lands on the documentation.

Test Plan

  • dotnet test PinballWizard.slnx --nologo516 / 516 passing (was 515; +1 pinning test)
  • dotnet build PinballWizard.slnx --nologo → clean, zero warnings
  • Pinning-test verification: temporarily renamed README, ran the test, confirmed it failed with the expected actionable message naming the missing path and citing the spec section. Reverted before commit.

Local review

/local-review outcome: 0 🔴 / 4 ⚠️ / 6 categories ✅.

  • ⚠️ Fixed — README false-pass risk on generic markers: added Assert.Contains("deliberately not covered", ...), anchoring the pinning to a phrase that appears only inside the asymmetry justification.
  • ⚠️ Fixed — guardrails cross-reference: R4 row now names the README + pinning test, so a future "delete everything in one PR" gets caught by a dangling reference reviewers will notice.
  • ⚠️ DeferredFindRepoRoot helper duplicated across two test files (this one and IngestionSourceSeederTests). The in-file comment sets a "extract on third consumer" rule (N=2 is below the extraction threshold; ~12 lines of stable filesystem-walk code; copies are textually identical).
  • ⚠️ Deferred — sharpening the subjective "behavior-change" revisit trigger to a count. The other two revisit triggers in the README are already objective (manufacturer count, build-cost threshold); defer pending operational signal that the subjective trigger has bitten.

7-item self-audit

  1. Every option field is read — N/A (no new options).
  2. Sibling-diff — diffed against IngestionSourceSeederTests.cs (also has a FindRepoRoot helper); helpers are textually identical, drift acknowledged in the new file's comment.
  3. No bare catch { }FindRepoRoot throws InvalidOperationException with a clear message; no swallowed exceptions.
  4. CLI / orchestrator wiring — N/A (no scraper or CLI surface added).
  5. Tests assert behavior — pinning test asserts file existence + 6 specific content markers; not just "the file compiles."
  6. Build is zero-warning — verified.
  7. Identity checkgit log -1 --format='%an <%ae>'Jim Keeley <94459922+jkeeley2073@users.noreply.github.com>

Out of Scope

  • Building a Playwright-route test fixture (FakePlaywrightContext). Route (i) of Phase 2 § Scope item 8; deferred per the README's revisit criteria. Build it when ≥ 2 manufacturers adopt Playwright OR an official Microsoft.Playwright testing helper lands.
  • Other Phase 2 Wave 2 items (item 6 Playwright bump, item 4 OPDB --dry-run, item 7 round 2 clean Dependabot bumps) — separate PRs.

🤖 Generated with Claude Code

Phase 2 § Scope item 8 (Wave 2) — route (ii). Resolves the asymmetry
between the family-wide HttpMessageHandler-based scraper-pipeline test
infrastructure (covers 8 of 10 ISourceScrapers) and the two Stern
Playwright scrapers (GamePageScraper, ServiceBulletinScraper) that
drive a real browser and don't go through HttpClient.

What ships:

- tests/PinballWizard.Scraper.Tests/README.md — describes the test
  project layout, the family-wide scraper-pipeline integration test
  infrastructure (FakePolitenessGate + QueueingHttpMessageHandler),
  the proven 5-test template from CgcGamePageScraperTests, the 8
  scrapers covered, the Stern Playwright asymmetry section
  (deliberately not covered + why HttpMessageHandler can't intercept
  Playwright + what coverage exists instead), revisit criteria
  (three concrete triggers including "≥ 2 manufacturers adopt
  Playwright" and "build cost of fake-Playwright fixture drops below
  ~4 hours"), and engineering conventions.
- tests/PinballWizard.Scraper.Tests/Scraping/Stern/SternPlaywrightAsymmetryDocumentationTests.cs
  — single Fact pinning test (Stern_Playwright_Pipeline_Test_Asymmetry_IsAcknowledged)
  that asserts the README exists at the expected path and contains
  the load-bearing markers of the asymmetry section. If the README
  is deleted or the section is gutted, the test fails — forcing a
  re-read or a real fix (build a Playwright-route fixture and replace
  the asymmetry with actual coverage).
- docs/guardrails.md R4 — updates the risk row to "Resolved (route ii)"
  and points at the new README + pinning test, so a search from the
  guardrails risk register lands on the documentation.

Local review summary: 0 🔴 / 4 ⚠️ / 6 categories ✅.

⚠️ — Fixed:
- "Anchor on a unique phrase" — added an Assert.Contains for
  "deliberately not covered" (a phrase that appears only in the
  asymmetry section, catches the gutted-section false-pass case).
- "Cross-reference from guardrails" — R4 row now points at the
  README + pinning test by name.

⚠️ — Deferred:
- "Helper drift" (FindRepoRoot duplicated across two test files):
  defer per the in-file comment's "extract on third consumer" rule.
  N=2 is below the extraction threshold; ~12 lines of stable
  filesystem-walk code; both copies are textually identical.
- "Sharpen the 'behavior change' revisit trigger to a count":
  the other two triggers in the Revisit criteria section are
  already objective (manufacturer count, build-cost dollar
  threshold). Defer pending operational signal that the subjective
  trigger has bitten.
@jkeeley2073 jkeeley2073 added the claude-code Generated with Claude Code label May 4, 2026
@jkeeley2073 jkeeley2073 merged commit 633460e into main May 4, 2026
4 checks passed
public void Stern_Playwright_Pipeline_Test_Asymmetry_IsAcknowledged()
{
var repoRoot = FindRepoRoot();
var readmePath = Path.Combine(repoRoot, "tests", "PinballWizard.Scraper.Tests", "README.md");
// Mirrors the helper in IngestionSourceSeederTests; if a third
// consumer appears, extract to Scraping/_TestInfra/RepoPaths.cs.
var dir = new DirectoryInfo(AppContext.BaseDirectory);
while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "PinballWizard.slnx")))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-code Generated with Claude Code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants