diff --git a/AGENTS.md b/AGENTS.md index d65a9ba1..ae3ce59b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,12 @@ Two things shape the coverage/safety bar here more than any specific rule: Together: coverage and signal are expensive to lose and cheap to keep. When a change trades either away, say so explicitly and justify the delta. +## Before you start + +- Check [`plans/`](plans/) — significant changes need a plan before code (see [Plans](#plans)). +- `verified-behaviors.json` in a package is a contract; don't break it without a plan. +- `bun run verify` passes before every commit. + ## Project Structure Monorepo with Bun workspaces. @@ -156,9 +162,15 @@ long-lived publish token. ## Plans -Significant changes should have a plan file in `plans/` before implementation -begins. Plans document the context, approach, and verification steps. Review -existing plans in that directory for format examples. +Significant changes need a plan in [`plans/`](plans/) before code. Plans +document context, approach, files touched, and verification. Match the shape +of existing plans. + +**Write one for:** new pages/routes, new build or CI steps, new cross-package +concepts, refactors across 5+ files. + +**Skip for:** bug fixes, single-file edits, doc tweaks, dep bumps, comment +cleanup, new tests in existing specs. ## Agent-First Documentation diff --git a/builds/knockout/helpers/browser-setup.js b/builds/knockout/helpers/browser-setup.js new file mode 100644 index 00000000..e102a48b --- /dev/null +++ b/builds/knockout/helpers/browser-setup.js @@ -0,0 +1,117 @@ +// Setup for running TKO specs in a real browser under Mocha. +// Counterpart to vitest-setup.js. Loaded as the first import of +// the test bundle (tko.io/scripts/bundle-tests.mjs). +// +// Assumes `mocha.setup('bdd')` already ran (so `before` / `after` +// / `beforeEach` / `afterEach` are global) and `globalThis.ko` +// was set by the bundled IIFE in /tests/source/setup.js, which +// this module is imported from. + +import * as chai from 'chai' +import sinon from 'sinon' +// Register punctuation filters on shared `@tko/utils` options so +// specs that construct Parsers directly (`new Parser().parse('x | tail')`) +// can resolve them. The builder registers the same filters at page +// startup but on a module-local options reference — not this one. +import { filters as punctuationFilters } from '@tko/filter.punches' +import { options as sharedOptions } from '@tko/utils' + +globalThis.chai = chai +globalThis.expect = chai.expect +globalThis.sinon = sinon +globalThis.isHappyDom = () => false + +sharedOptions.filters = Object.assign(sharedOptions.filters || {}, punctuationFilters) + +// mocha-test-helpers wires root beforeEach/afterEach hooks, so it +// must be imported after `mocha.setup('bdd')` ran. +import './mocha-test-helpers.js' + +// Disable the 25ms JSX cleanup timer so it can't race test teardown. +before(() => { + if (globalThis.ko?.options) { + globalThis.ko.options.jsxCleanBatchSize = 0 + } +}) + +// Iframe focus-event polyfill. +// +// Chromium refuses to grant programmatic `iframe.contentWindow.focus()` +// true system focus from a parent that already holds focus — the +// iframe never passes `document.hasFocus() === true`, so `focusin` +// / `focusout` are suppressed when specs call `element.focus()` +// inside. The `hasfocus` binding observes those events (not +// `document.activeElement`), so without this patch those specs +// fail under both Playwright and a real Chrome tab. +// +// Wrap `focus`/`blur` to dispatch the missing events synchronously +// after the native call. If the browser DOES regain system focus +// and fires them too, observers see duplicates — harmless for +// these specs (state-checking, not call-count). +// +// Scope-guarded to iframes (`window.parent !== window`) so the +// parent page is never patched. +// +// Refs: https://github.com/jsdom/jsdom/pull/2996 (same shape as +// this wrap), https://html.spec.whatwg.org/multipage/interaction.html#focusing-elements. +if (window.parent !== window && !HTMLElement.prototype.__tkoFocusPatched) { + const HE = HTMLElement.prototype + HE.__tkoFocusPatched = true + const origFocus = HE.focus + const origBlur = HE.blur + HE.focus = function (...args) { + const wasActive = this.ownerDocument.activeElement + origFocus.apply(this, args) + if (this.ownerDocument.activeElement === this && wasActive !== this) { + this.dispatchEvent(new FocusEvent('focus', { bubbles: false, relatedTarget: wasActive })) + this.dispatchEvent(new FocusEvent('focusin', { bubbles: true, relatedTarget: wasActive })) + } + } + HE.blur = function (...args) { + const wasActive = this.ownerDocument.activeElement + origBlur.apply(this, args) + if (wasActive === this && this.ownerDocument.activeElement !== this) { + this.dispatchEvent(new FocusEvent('blur', { bubbles: false, relatedTarget: this.ownerDocument.activeElement })) + this.dispatchEvent(new FocusEvent('focusout', { bubbles: true, relatedTarget: this.ownerDocument.activeElement })) + } + } +} + +// Unscoped sinon fakes (`sinon.spy(obj,'m')`, `sinon.useFakeTimers()`) +// leak across specs if not restored, producing bogus call-count +// diffs or "Can't install fake timers twice". `sinon.restore()` is +// a no-op for sandbox-scoped fakes. Vitest isolates per-file so +// doesn't need this hook. +afterEach(() => { + if (globalThis.sinon?.restore) globalThis.sinon.restore() +}) + +// Vitest-style context-arg shim. +// +// Specs written `function (ctx) { if (isHappyDom()) return ctx.skip(…) }` +// look like Mocha done-callback specs (`fn.length === 1`) and time +// out after ~10s because they never call done. Wrap `it` to detect +// the ctx shape (uses `.skip(...)` and never calls `done(`) and +// invoke with a synthetic `{ skip }` while hiding arity from Mocha. +{ + const wrap = orig => + function (name, fn) { + if (typeof fn === 'function' && fn.length === 1) { + const src = fn.toString() + const ctxStyle = /\.skip\s*\(/.test(src) && !/\bdone\s*\(/.test(src) + if (ctxStyle) { + const wrapped = function () { + return fn.call(this, { skip: reason => this.skip(reason) }) + } + Object.defineProperty(wrapped, 'length', { value: 0 }) + return orig.call(this, name, wrapped) + } + } + return orig.apply(this, arguments) + } + const origIt = globalThis.it + const wrappedIt = wrap(origIt) + wrappedIt.only = wrap(origIt.only) + wrappedIt.skip = origIt.skip + globalThis.it = wrappedIt +} diff --git a/packages/binding.component/spec/componentBindingBehaviors.ts b/packages/binding.component/spec/componentBindingBehaviors.ts index 266e4d8b..a3b83f4e 100644 --- a/packages/binding.component/spec/componentBindingBehaviors.ts +++ b/packages/binding.component/spec/componentBindingBehaviors.ts @@ -1273,7 +1273,12 @@ describe('Components: Component binding', function () { if (isHappyDom()) return ctx.skip('happy-dom: innerText whitespace rendering differs from real browsers') applyBindings(outerViewModel, testNode) - expect((testNode.children[0] as HTMLInputElement).innerText.trim()).to.deep.equal(`beep / beep`) + // `.trim()` alone strips only leading/trailing; real browsers + // preserve internal newlines + indentation from the template + // source between slotted nodes. Collapse to single spaces. + expect((testNode.children[0] as HTMLInputElement).innerText.replace(/\s+/g, ' ').trim()).to.deep.equal( + `beep / beep` + ) }) it('inserts into nested elements', function () { @@ -1410,7 +1415,7 @@ describe('Components: Component binding', function () { if (isHappyDom()) return ctx.skip('happy-dom: innerText whitespace rendering differs from real browsers') applyBindings(outerViewModel, testNode) - expect((testNode.children[0] as HTMLElement).innerText.trim()).to.deep.equal(`A. B. C.`) + expect((testNode.children[0] as HTMLElement).innerText.replace(/\s+/g, ' ').trim()).to.deep.equal(`A. B. C.`) const em = testNode.children[0].children[0].children[0] expect(em.tagName).to.deep.equal('EM') }) @@ -1432,7 +1437,7 @@ describe('Components: Component binding', function () { if (isHappyDom()) return ctx.skip('happy-dom: innerText whitespace rendering differs from real browsers') applyBindings(outerViewModel, testNode) - expect((testNode.children[0] as HTMLElement).innerText.trim()).to.deep.equal(`B. C. E.`) + expect((testNode.children[0] as HTMLElement).innerText.replace(/\s+/g, ' ').trim()).to.deep.equal(`B. C. E.`) const em = testNode.children[0].children[0].children[0] expect(em.tagName).to.deep.equal('EM') }) diff --git a/packages/binding.foreach/spec/eachBehavior.ts b/packages/binding.foreach/spec/eachBehavior.ts index 77e2e04e..d56a729f 100644 --- a/packages/binding.foreach/spec/eachBehavior.ts +++ b/packages/binding.foreach/spec/eachBehavior.ts @@ -1010,6 +1010,9 @@ describe('observable array changes', function () { describe('focus', function () { let $target + // foreach schedules processQueue via requestAnimationFrame when setSync(false). + // Waiting one frame is enough to flush the DOM re-order + focus-preservation pass. + const nextFrame = () => new Promise(resolve => requestAnimationFrame(() => resolve(null))) beforeEach(function () { $target = $("
" + '' + '
').appendTo(document.body) ForEachBinding.setSync(false) @@ -1023,7 +1026,7 @@ describe('focus', function () { const list = ['a', 'b', 'c'] $target.find(':input').focus() applyBindings(list, $target[0]) - await new Promise(resolve => setTimeout(resolve, 1000)) + await nextFrame() assert.strictEqual(document.activeElement, document.body) }) @@ -1036,7 +1039,7 @@ describe('focus', function () { list.remove('a') list.push('a') - await new Promise(resolve => setTimeout(resolve, 1000)) + await nextFrame() assert.strictEqual(document.activeElement, document.body) }) @@ -1050,7 +1053,7 @@ describe('focus', function () { list.remove(o0) list.push(o0) - await new Promise(resolve => setTimeout(resolve, 1000)) + await nextFrame() assert.strictEqual(document.activeElement, $target.find(':input')[2], 'o') }) @@ -1067,7 +1070,7 @@ describe('focus', function () { list.push(o0) list.push('y') - await new Promise(resolve => setTimeout(resolve, 1000)) + await nextFrame() assert.strictEqual(document.activeElement, $target.find(':input')[3], 'o') }) @@ -1083,7 +1086,7 @@ describe('focus', function () { list.push(o0) // focused list.push(o0) - await new Promise(resolve => setTimeout(resolve, 1000)) + await nextFrame() assert.strictEqual(document.activeElement, $target.find(':input')[2], 'o') }) }) diff --git a/packages/binding.if/spec/elseBehaviors.ts b/packages/binding.if/spec/elseBehaviors.ts index 95da1889..dd00a6ed 100644 --- a/packages/binding.if/spec/elseBehaviors.ts +++ b/packages/binding.if/spec/elseBehaviors.ts @@ -32,13 +32,19 @@ describe('else inside an if binding', function () { }) describe('as ', function () { + // innerText preserves whitespace from the HTML source in real + // browsers (happy-dom normalizes it, which is why that path + // skips). The source here intentionally includes a space + // around `` so the comment parses as a binding, + // so the selected branch keeps a trailing/leading space in its + // text node. Trim assertions instead of stripping the source. it('is ignored when the condition is true', function (ctx: any) { if (isHappyDom()) return ctx.skip('happy-dom: innerText whitespace rendering differs from real browsers') testNode.innerHTML = "" + 'abc def' + '' expect(testNode.childNodes[0].childNodes.length).to.equal(3) applyBindings({ x: true }, testNode) expect(testNode.childNodes[0].childNodes.length).to.equal(1) - expect(testNode.innerText).to.equal('abc') + expect(testNode.innerText.trim()).to.equal('abc') }) it('shows the else-block when the condition is false', function (ctx: any) { @@ -47,7 +53,7 @@ describe('else inside an if binding', function () { expect(testNode.childNodes[0].childNodes.length).to.equal(3) applyBindings({ x: false }, testNode) expect(testNode.childNodes[0].childNodes.length).to.equal(1) - expect(testNode.innerText).to.equal('def') + expect(testNode.innerText.trim()).to.equal('def') }) it('toggles between if/else on condition change', function (ctx: any) { @@ -57,9 +63,9 @@ describe('else inside an if binding', function () { expect(testNode.childNodes[0].childNodes.length).to.equal(3) applyBindings({ x: x }, testNode) expect(testNode.childNodes[0].childNodes.length).to.equal(1) - expect(testNode.innerText).to.equal('def') + expect(testNode.innerText.trim()).to.equal('def') x(true) - expect(testNode.innerText).to.equal('abc') + expect(testNode.innerText.trim()).to.equal('abc') }) }) }) diff --git a/packages/builder/spec/builderBehaviors.ts b/packages/builder/spec/builderBehaviors.ts index 5cd237be..8d6fc675 100644 --- a/packages/builder/spec/builderBehaviors.ts +++ b/packages/builder/spec/builderBehaviors.ts @@ -1,10 +1,23 @@ import { VirtualProvider } from '@tko/provider.virtual' import { bindings as ifBindings } from '@tko/binding.if' +import { options } from '@tko/utils' import { Builder } from '../dist' describe('Builder', () => { - it('creates a ko instance', () => { + it('creates a ko instance', function () { + // `new Builder({...})` mutates the shared `@tko/utils` options + // (filters + bindingProviderInstance). Under Vitest's per-file + // module isolation the mutation is invisible to later specs; + // under the browser /tests runner every spec shares module + // state, so an unrestored mutation here wipes `options.filters` + // (which at this point holds the punctuation filters from + // `@tko/filter.punches`) and breaks ~14 downstream filter- + // lookup tests. + // @ts-ignore — global helper from mocha-test-helpers.js + restoreAfter(options, 'filters') + // @ts-ignore — global helper from mocha-test-helpers.js + restoreAfter(options, 'bindingProviderInstance') // We're just testing that the builder constructs, here. const builder = new Builder({ filters: {}, provider: new VirtualProvider(), bindings: [ifBindings], options: {} }) }) diff --git a/packages/utils.parser/spec/identifierBehaviors.ts b/packages/utils.parser/spec/identifierBehaviors.ts index 8f66fd17..e76ab77c 100644 --- a/packages/utils.parser/spec/identifierBehaviors.ts +++ b/packages/utils.parser/spec/identifierBehaviors.ts @@ -173,6 +173,16 @@ describe('Identifier', function () { }) it('sets `this` of a top-level item to $data', function () { + // `restoreAfter` is a global cleanup-stack helper from + // builds/knockout/helpers/mocha-test-helpers.js. Vitest runs + // each spec file in isolated module state so this mutation is + // invisible to later specs; in the browser /tests runner all + // specs share one module graph, so without this restore the + // mutated `bindingGlobals` leaks into every subsequent spec + // that resolves `$parent` / `$customProp` / … against the + // global lookup, breaking 31 unrelated tests. + // @ts-ignore — global helper from mocha-test-helpers.js + restoreAfter(options, 'bindingGlobals') options.bindingGlobals = Object.create({ Ramanujan: '1729' }) const div = document.createElement('div'), context = { diff --git a/plans/browser-test-runner.md b/plans/browser-test-runner.md new file mode 100644 index 00000000..492c5e2a --- /dev/null +++ b/plans/browser-test-runner.md @@ -0,0 +1,123 @@ +# Plan: Live browser test runner at /tests + +## Context + +TKO specs run under Vitest + happy-dom in CI (`bun run test`). That +covers DOM semantics reachable from a JS implementation of the +DOM, but misses anything that depends on a real browser: system +focus routing, layout, rendering, intersection, CSSOM, and per- +engine DOM quirks across Chromium / WebKit / Firefox. + +A live browser runner at `/tests` on `tko.io` closes that gap by +running the same Mocha specs inside real iframes. It serves two +audiences: + +1. **Contributors** — open `/tests` in any browser, get real- + browser pass/fail without installing anything. Double as a + reproducible bug harness. +2. **Agents** — Playwright or similar hits `/tests`, parses the + stats chips, and knows whether a change holds up under real + layout/focus/rendering. Complements verified-behaviors.json + as an additional correctness signal. + +Test environments are additive: Vitest+happy-dom stays, browser +runner expands coverage. Neither replaces the other. + +## Approach + +Astro page at `tko.io/src/pages/tests.astro` drives two modes via +`?suite=`: + +- **source** (default) — run specs from the checkout, bundled + per-spec into ESM chunks. One iframe per spec. +- **build** — load a published `@tko/build.*` bundle from + jsDelivr, run all specs in one iframe against that CDN + artifact. Proves the shipped build is green. + +Each spec executes inside a `tests-frame.html` iframe harness +and posts pass/fail/pending/end events to the parent page. The +parent aggregates into a TKO-driven view model (suite tree, +stats chips, progress bar). + +## Architecture + +``` +┌───────────────────────────────────────────────────────────┐ +│ /tests page (TKO view model) │ +│ ┌─────────────────────────────────────────────────────┐ │ +│ │ suite tree (per-spec file, running/pass/fail state) │ │ +│ └─────────────────────────────────────────────────────┘ │ +│ │ +│ Phase 1: parallel (pool=4) hidden iframes │ +│ ┌─────┐ ┌─────┐ ┌─────┐ ┌─────┐ 1×1 offscreen │ +│ │spec │ │spec │ │spec │ │spec │ no focus needed │ +│ └─────┘ └─────┘ └─────┘ └─────┘ │ +│ │ +│ Phase 2: serial iframe in bottom-right workarea │ +│ ┌─────────────────┐ real layout box │ +│ │ focus-spec here │ sole system focus │ +│ └─────────────────┘ hasfocus/.focus/activeElement │ +│ │ +│ postMessage(pass/fail/pending/end) → parent │ +└───────────────────────────────────────────────────────────┘ +``` + +### Why two phases + +Chromium grants system focus to one iframe at a time and denies +it to off-screen / zero-area iframes. Specs that assert on +`hasfocus`, `.focus()`, `document.activeElement`, or `focusin` / +`focusout` can't run in parallel in hidden iframes — they need a +real visible layout box and sole focus. A `needsFocus` flag +(grep-based in the bundler) routes those specs serially through +a visible workarea. + +### Bundling + +`tko.io/scripts/bundle-tests.mjs` (esbuild) produces: + +- `public/tests/build-bundle.js` — all `builds/**/spec/*` specs + that only depend on `ko.*` globals, bundled into one script + for build mode. Specs with relative imports are excluded. +- `public/tests/source/setup.js` — classic-script IIFE that + exposes `globalThis.ko`, chai/sinon, punctuation filters, + mocha-test-helpers, and the focus/sinon polyfills. +- `public/tests/source/.js` — one ESM module per spec + for source mode. +- `public/tests/source/manifest.json` — per-spec metadata + (slug, suite name, needsFocus) the parent uses to schedule. + +`prebuild` runs the bundler so the dev server and production +build both serve fresh output. + +## Files + +| File | Role | +|------|------| +| `tko.io/src/pages/tests.astro` | `/tests` page — TKO VM, suite tree, schedulers | +| `tko.io/public/tests-frame.html` | Per-spec iframe harness | +| `tko.io/scripts/bundle-tests.mjs` | Discover specs, emit bundles + manifest | +| `builds/knockout/helpers/browser-setup.js` | Globals, focus polyfill, sinon restore, ctx-arg shim | +| `tko.io/package.json` | `prebuild` invokes the bundler | +| `tko.io/.gitignore` | Ignores generated `public/tests/` | + +## Verification + +1. `cd tko.io && bun run dev` +2. Visit `http://localhost:4321/tests` — source mode should + reach 2708/0/42 in ~5-6s. +3. Switch to `/tests?suite=build&pkg=knockout&ver=latest` — + build-mode suite runs against the published CDN bundle. +4. `?grep=` — filter specs by file/slug (both modes). +5. CI: `bun run verify` still green under Vitest+happy-dom; the + browser runner is an additive signal, not a replacement. + +## What's not there yet + +| Gap | What's needed | +|-----|---------------| +| **CI smoke** | Playwright job hitting `/tests` on every PR | +| **Firefox / WebKit coverage** | Playwright per-engine; today only Chromium is exercised interactively | +| **Flaky focus detection** | Track focus-phase retries; flag specs that pass only after retry | +| **Build-mode spec coverage** | 60+ specs currently excluded because they have relative imports — rewrite to use `ko.*` globals so build mode covers them | +| **Differential result** | Surface specs that pass under happy-dom but fail in a real browser (and vice versa) as a report | diff --git a/tko.io/.gitignore b/tko.io/.gitignore index 6cd54b46..78247e6d 100644 --- a/tko.io/.gitignore +++ b/tko.io/.gitignore @@ -4,3 +4,4 @@ _site/ dist/ package-lock.json public/lib/ +public/tests/ diff --git a/tko.io/package.json b/tko.io/package.json index b7c1ea47..897a22ee 100644 --- a/tko.io/package.json +++ b/tko.io/package.json @@ -5,7 +5,7 @@ "description": "TKO documentation site", "scripts": { "postinstall": "patch-package", - "prebuild": "node ./scripts/generate-verified-behaviors.mjs && mkdir -p public/lib && cd ../builds/knockout && bun run build && cp dist/browser.min.js ../../tko.io/public/lib/ko.js && cd ../reference && bun run build && cp dist/browser.min.js ../../tko.io/public/lib/tko.js", + "prebuild": "node ./scripts/generate-verified-behaviors.mjs && mkdir -p public/lib && cd ../builds/knockout && bun run build && cp dist/browser.min.js ../../tko.io/public/lib/ko.js && cd ../reference && bun run build && cp dist/browser.min.js ../../tko.io/public/lib/tko.js && cd ../../tko.io && node ./scripts/bundle-tests.mjs", "predev": "bun run prebuild", "dev": "ASTRO_TELEMETRY_DISABLED=1 astro dev", "build": "ASTRO_TELEMETRY_DISABLED=1 astro build --force", diff --git a/tko.io/public/tests-frame.html b/tko.io/public/tests-frame.html new file mode 100644 index 00000000..f40bba08 --- /dev/null +++ b/tko.io/public/tests-frame.html @@ -0,0 +1,89 @@ + + + + + TKO Spec Frame + + + + + +
+
+ + + + + + + + + diff --git a/tko.io/scripts/bundle-tests.mjs b/tko.io/scripts/bundle-tests.mjs new file mode 100644 index 00000000..2c7386ee --- /dev/null +++ b/tko.io/scripts/bundle-tests.mjs @@ -0,0 +1,310 @@ +// Bundles TKO specs for the in-browser `/tests` runner. +// +// Two outputs: +// +// public/tests/build-bundle.js Version-portable IIFE of +// `builds/{knockout,reference}/spec/`. +// Runs as a single Mocha run against +// whichever `@tko/build.*` the page +// loaded via + + + + + + +