Skip to content

Document TKO's testing coverage bar (real browsers only)#331

Closed
brianmhunt wants to merge 1 commit into
mainfrom
docs/testing-coverage-bar
Closed

Document TKO's testing coverage bar (real browsers only)#331
brianmhunt wants to merge 1 commit into
mainfrom
docs/testing-coverage-bar

Conversation

@brianmhunt
Copy link
Copy Markdown
Member

Summary

Codifies the coverage policy for TKO in AGENTS.md, under the existing Testing section. Motivated by the closed PR #330 β€” I proposed a happy-dom split for speed, and the trade didn't hold up for a library at TKO's level. Filing the reasoning so future agents don't re-propose the same pattern.

The policy

TKO is a low-level DOM binding framework published to npm and used by unknown downstream apps. The dark-factory thesis (small teams + AI assistance maintaining a framework) puts the test suite in the role humans used to play. So the coverage bar is higher than typical.

Three concrete rules:

  1. Every spec runs in every supported real browser. CI matrix (chromium, firefox, webkit) is load-bearing.
  2. No partial-DOM environments. happy-dom/jsdom/linkedom give false signal for a DOM binding library β€” the behaviors TKO most needs to verify (form-control event ordering, MutationObserver, namespaced attributes, custom elements) are exactly where partial DOMs diverge.
  3. Excluding files from a run is a red flag. Concentrates risk; doesn't save coverage.

Also points at PR #330 as a worked example so the rationale is traceable.

Test plan

Docs-only. No code changes.

πŸ€– Generated with Claude Code

Copilot AI review requested due to automatic review settings April 17, 2026 12:46
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 17, 2026

Warning

Rate limit exceeded

@brianmhunt has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 8 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 8 seconds.

βŒ› How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c0875a4c-83a4-4b71-8045-12008147f626

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 94413fe and 958451f.

πŸ“’ Files selected for processing (1)
  • AGENTS.md
✨ Finishing Touches
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/testing-coverage-bar

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates contributor guidance in AGENTS.md to codify TKO’s testing coverage policy, emphasizing real-browser fidelity and discouraging partial-DOM environments for this DOM-binding framework.

Changes:

  • Expands the Testing section with an explicit β€œcoverage bar” policy (real browsers, no partial DOM, no excluding files).
  • Documents rationale and links to closed PR #330 as a worked example.
  • Adds a suggested approach for faster local feedback (run a narrower subset rather than lowering fidelity).

Comment thread AGENTS.md Outdated
Comment on lines +77 to +79
1. **Every spec runs in every supported real browser** (chromium, firefox,
webkit). The CI matrix is load-bearing. Do not consolidate it to a single
browser for speed.
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

The new policy states tests must run across Chromium/Firefox/WebKit, but earlier in this same Testing section the runner is described as "headless Chromium". Given vitest.config.ts supports multiple browsers via VITEST_BROWSERS and CI runs a 3-browser matrix, this wording is now inconsistent and may mislead contributors. Consider updating the runner description (and/or calling out that local default is Chromium, CI runs the full matrix).

Copilot uses AI. Check for mistakes.
TKO is a low-level DOM binding framework shipped to unknown downstream
apps, and the dark-factory thesis makes the test suite the safety net
in a way human review used to be. That raises the bar above the usual
"enough for a product" level.

Three concrete rules:

1. Every spec runs in every supported real browser (chromium, firefox,
   webkit). CI matrix is load-bearing β€” don't consolidate for speed.
2. No partial-DOM environments (happy-dom, jsdom, linkedom). Exactly the
   behaviors TKO most needs to verify β€” form-control event ordering,
   MutationObserver timing, namespaced attributes, custom elements β€”
   are where partial DOMs diverge from real browsers. A green there can
   mask a real-browser regression.
3. Excluding files from a run is a red flag. A file that would fail in
   some environment is a file testing something important; skipping
   doesn't save coverage, it concentrates risk.

Reference: closed PR #330 worked through this trade and concluded the
speed-for-correctness swap doesn't hold at TKO's level. Documenting so
future agents don't re-propose the same pattern.

For fast local feedback: scope the run (bunx vitest run packages/observable)
rather than reducing fidelity of the full run.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@brianmhunt brianmhunt force-pushed the docs/testing-coverage-bar branch from b82d383 to 958451f Compare April 17, 2026 12:50
@brianmhunt
Copy link
Copy Markdown
Member Author

Closing β€” I misunderstood the intent. The goal of happy-dom/node isn't to replace browser testing for speed, it's to add coverage for non-browser runtimes TKO should support (TUI, SSR, server-side reactivity). Will redo as additive coverage, not a swap. Leaving the closed-PR reference intact for context.

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.

2 participants