Skip to content

test(kattis): snapshot the real problem-page HTML parser - #105

Merged
cameroncuster merged 1 commit into
mainfrom
cameroncuster/kattis-parser-snapshot-test
Jul 25, 2026
Merged

test(kattis): snapshot the real problem-page HTML parser#105
cameroncuster merged 1 commit into
mainfrom
cameroncuster/kattis-parser-snapshot-test

Conversation

@cameroncuster

Copy link
Copy Markdown
Owner

ProblemparseKattisProblemPage scrapes open.kattis.com markup for a problem's name and difficulty, but it's the one Kattis function with no direct coverage: every existing test (unit and e2e) mocks parsePage out. A selector change or upstream markup drift would slip through silently.

Solution — add a Playwright spec that exercises the production function (serialized via .toString(), not a re-implementation) inside a real browser DOM through page.evaluate, asserting name + rating against captured fixtures:

  • full page (<h1> + .difficulty_number)
  • legacy .difficulty class fallback
  • missing difficulty → default rating 5
  • missing <h1> → falls back to the problem id

It runs in the browser because the parser uses the browser-only DOMParser; this deliberately avoids adding a jsdom/happy-dom dev dependency (per repo guidelines on new deps). The spec runs in the existing mocked desktop/mobile projects and uses about:blank, so it's decoupled from app/network state.

Testing — I could not run the suite in my environment (only Node 22 available; repo requires Node ≥24 with engine-strict). Opened as draft so CI on Node 24 validates lint/lint:es/check and the Playwright run. I'll address any CI feedback.

Opened by an Augment PR Author agent on behalf of @cameroncuster.

parseKattisProblemPage scrapes open.kattis.com markup for a problem's
name (<h1>) and difficulty (.difficulty_number, falling back to
.difficulty). Every existing Kattis test mocks parsePage out, so the
selector logic most likely to break on upstream markup drift was
unexercised.

Add a Playwright spec that runs the production function (serialized, not
copied) inside a real browser DOM via page.evaluate, asserting name and
rating against captured fixtures: full page, legacy .difficulty class,
missing difficulty (defaults rating to 5), and missing <h1> (falls back
to the problem id). Runs in the browser because the parser uses the
browser-only DOMParser; this avoids adding a jsdom/happy-dom dependency.
@cameroncuster cameroncuster self-assigned this Jul 24, 2026
@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitgud Ready Ready Preview, Comment Jul 24, 2026 11:31pm

@cameroncuster

Copy link
Copy Markdown
Owner Author

PR Author Agent⚡ on behalf of @cameroncuster

👋 This PR is monitored by an Augment Agent.

I'll answer questions, implement suggestions, fix CI failures, and resolve merge conflicts. I slow down if the PR goes quiet, but any new comment wakes me back up.

@cameroncuster
cameroncuster marked this pull request as ready for review July 24, 2026 23:42
@cameroncuster

cameroncuster commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

PR Risk Analyzer Agent🛡️

👀 Human Input Needed
A pair-review briefing is ready for you.

→ Pair Review Briefing

@cameroncuster

Copy link
Copy Markdown
Owner Author

PR Author Agent⚡ on behalf of @cameroncuster

Addressing the Risk Analyzer's two focus topics:

Fixtures pin real behavior and catch drift. The test runs the production parseKattisProblemPage (not a copy), so it asserts the actual selectors: h1 for the name and .difficulty_number (with .difficulty fallback) for the rating. The four fixtures cover each branch — primary selector, legacy-class fallback, missing-difficulty default (rating 5), and missing-h1 (falls back to the problem id). If Kattis renames/moves those elements or someone changes a selector, the name/rating assertions fail. It won't catch drift in markup the parser doesn't look at (by design — the parser only reads those two selectors).

Why .toString() + new Function. The parser depends on the browser-only DOMParser, so it can't run in the Node unit process. Serializing the real function and reconstructing it inside page.evaluate runs the genuine production code against a real DOM, which is the whole point — the alternative (re-implementing selectors in the test) would test a copy, not the code that ships. It's safe here: the function is self-contained (only DOMParser/document, no closure or module refs), and the fixture HTML is static test data, not untrusted input. The trade-off vs. adding jsdom/happy-dom was deliberate — this avoids a new dependency, per the repo's guidance.

Happy to switch to a DOM shim + Node unit test instead if a maintainer prefers that shape.

@cameroncuster cameroncuster left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Deep Code Review Agent🐛

Review completed with 0 suggestions.

@cameroncuster

Copy link
Copy Markdown
Owner Author

Verifier 🛰️

🛰️ Verifier Report

TL;DR: Ran the new Kattis-parser spec in a real browser across desktop + mobile, then broke the production selector to prove the spec actually catches a regression rather than passing blindly.

🔍 Walkthrough

  • Ran the new spec end-to-end in a real browser → all 8 checks pass (4 cases × mocked-desktop + mocked-mobile), driving the browser-only DOMParser path CI's Node unit tests can't reach.

    the four cases map 1:1 onto the parser's branches (name via h1, rating via .difficulty_number.difficulty → default 5)
    ✓ reads the h1 name and .difficulty_number rating        → { name: 'Two Stones', rating: 2.1 }
    ✓ falls back to the .difficulty class                    → { name: 'Legacy Problem', rating: 7 }
    ✓ defaults the rating to 5 when no difficulty is present → { name: 'Nameless Difficulty', rating: 5 }
    ✓ falls back to the problem id when the h1 is missing     → { name: 'missingtitle', rating: 5 }
    

    Each expected value matches the production function in src/lib/providers/kattis/ingestion.ts exactly.

  • Confirmed the spec binds to the production parser (via .toString() + page.evaluate), not a re-implementation → breaking the real selector fails exactly the two selector-dependent cases while the other two still pass.

    mutation: .difficulty_number, .difficulty → a wrong class in ingestion.ts, then re-ran
    ✘ reads the h1 name and .difficulty_number rating   Expected rating 2.1, Received 5
    ✘ falls back to the .difficulty class               Expected rating 7,   Received 5
    ✓ defaults the rating to 5 when no difficulty present
    ✓ falls back to the problem id when the h1 is missing
    

    On mainline the two difficulty cases assert 2.1 / 7; with the selector broken the parser returns its default 5 and they fail — so a markup/selector drift fails loudly, as the PR intends. Selector restored afterward; working tree left clean.

📋 Scope

Under test: a new Playwright spec (e2e/kattis-parser.spec.ts, +109 lines, the only changed file) that pins parseKattisProblemPage's name/difficulty scraping against captured HTML fixtures by running the serialized production function inside a real browser DOM.

Not tested:

  • CI's static gates (lint, lint:es, check, build) — CI owns these; not re-run here.
  • The rest of the e2e suite and the live-smoke path (SUPABASE_SMOKE=1) — unrelated to this change; the spec uses about:blank and is decoupled from app/network/DB state, so no Supabase surface was touched.

📦 Artifacts · clean run · mutation proof


Don't like what you see? Add a verification skill so the next run tests this kind of change.

Want another run? Comment cosmos verify on this PR.

👍 / 👎 · View session

@cameroncuster
cameroncuster merged commit e2718e8 into main Jul 25, 2026
7 checks passed
@cameroncuster
cameroncuster deleted the cameroncuster/kattis-parser-snapshot-test branch July 25, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant