Skip to content

Commit c5bd8d7

Browse files
Brian M HuntBrian M Hunt
authored andcommitted
tests: tighten browser-setup.js comments
- Fix dated claim that window.ko comes from /lib/ko.js <script>; it's set by the bundled IIFE in /tests/source/setup.js. - Drop "34 tests fail" specific count (historical, pre-polyfill). - Collapse duplicate phrasings ("isHappyDom is never true here" was in both the header and the body). - Shorten the punctuation-filter, sinon-restore, and ctx-arg shim comments to one concise paragraph each.
1 parent a66121f commit c5bd8d7

1 file changed

Lines changed: 40 additions & 97 deletions

File tree

builds/knockout/helpers/browser-setup.js

Lines changed: 40 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,33 @@
11
// Setup for running TKO specs in a real browser under Mocha.
2-
// Counterpart to vitest-setup.js — same concerns, different host.
2+
// Counterpart to vitest-setup.js. Loaded as the first import of
3+
// the test bundle (tko.io/scripts/bundle-tests.mjs).
34
//
4-
// Loaded as the first import of the test bundle (see
5-
// tko.io/scripts/bundle-tests.mjs). When this module runs, it
6-
// assumes:
7-
//
8-
// - mocha.setup('bdd') has already been called on the page,
9-
// so `before` / `after` / `beforeEach` / `afterEach` are
10-
// global Mocha hooks.
11-
// - window.ko was set by loading /lib/ko.js via a <script>
12-
// tag before the bundle.
13-
//
14-
// Responsibilities:
15-
// - Expose `chai`, `expect`, `sinon` as the globals the specs
16-
// and mocha-test-helpers.js expect.
17-
// - `isHappyDom` is never true here (real browser).
18-
// - Force `ko.options.jsxCleanBatchSize = 0` before the suite
19-
// runs so the 25ms JSX cleanup timer does not race test
20-
// teardown.
5+
// Assumes `mocha.setup('bdd')` already ran (so `before` / `after`
6+
// / `beforeEach` / `afterEach` are global) and `globalThis.ko`
7+
// was set by the bundled IIFE in /tests/source/setup.js, which
8+
// this module is imported from.
219

2210
import * as chai from 'chai'
2311
import sinon from 'sinon'
24-
// Register punctuation filters (`uppercase`, `lowercase`, `tail`, …)
25-
// on the shared `@tko/utils` options so `Parser` instances created
26-
// ad-hoc by specs (`new Parser().parse("x | tail")`) can resolve
27-
// them. The knockout builder ALSO registers these via
28-
// `builder.create({ filters })` at page startup, but it assigns to
29-
// a module-local `knockout.options` — not the same reference that
30-
// Parser reads from. Writing to the shared `options.filters` here
31-
// is safe: it's the same object the Builder later augments, and
32-
// earlier writes are idempotent for the punctuation set.
12+
// Register punctuation filters on shared `@tko/utils` options so
13+
// specs that construct Parsers directly (`new Parser().parse('x | tail')`)
14+
// can resolve them. The builder registers the same filters at page
15+
// startup but on a module-local options reference — not this one.
3316
import { filters as punctuationFilters } from '@tko/filter.punches'
3417
import { options as sharedOptions } from '@tko/utils'
3518

3619
globalThis.chai = chai
3720
globalThis.expect = chai.expect
3821
globalThis.sinon = sinon
22+
globalThis.isHappyDom = () => false
3923

4024
sharedOptions.filters = Object.assign(sharedOptions.filters || {}, punctuationFilters)
4125

42-
// A real browser is never happy-dom — specs that skip under
43-
// happy-dom run here.
44-
globalThis.isHappyDom = () => false
45-
46-
// Specs depend on helpers registered as globals by
47-
// mocha-test-helpers.js — `prepareTestNode`, `restoreAfter`,
48-
// `expectContainHtml`, etc. That module also overrides
49-
// `globalThis.after` to a cleanup-stack pusher (not a Mocha
50-
// suite hook) and wires root `beforeEach` / `afterEach` hooks
51-
// that flush the cleanup stack. Must be imported after Mocha's
52-
// `bdd` UI has been set up (the HTML page does
53-
// `mocha.setup('bdd')` before loading the bundle), otherwise
54-
// its `beforeEach(function () { … })` at module top throws.
26+
// mocha-test-helpers wires root beforeEach/afterEach hooks, so it
27+
// must be imported after `mocha.setup('bdd')` ran.
5528
import './mocha-test-helpers.js'
5629

30+
// Disable the 25ms JSX cleanup timer so it can't race test teardown.
5731
before(() => {
5832
if (globalThis.ko?.options) {
5933
globalThis.ko.options.jsxCleanBatchSize = 0
@@ -62,42 +36,24 @@ before(() => {
6236

6337
// Iframe focus-event polyfill.
6438
//
65-
// Each spec runs inside a hidden iframe spawned by
66-
// `tko.io/src/pages/tests.astro` via `tests-frame.html`. Chromium
67-
// refuses to grant programmatic `iframe.contentWindow.focus()`
68-
// true system focus from a parent that already has focus — the
69-
// iframe's window never passes `document.hasFocus() === true`, so
70-
// the browser suppresses `focusin` / `focusout` events when specs
71-
// call `element.focus()` / `element.blur()` inside. The
72-
// `hasfocus` binding observes those events (not
73-
// `document.activeElement`); 34 tests fail as a result, both
74-
// under Playwright and under a real Chrome tab.
39+
// Chromium refuses to grant programmatic `iframe.contentWindow.focus()`
40+
// true system focus from a parent that already holds focus — the
41+
// iframe never passes `document.hasFocus() === true`, so `focusin`
42+
// / `focusout` are suppressed when specs call `element.focus()`
43+
// inside. The `hasfocus` binding observes those events (not
44+
// `document.activeElement`), so without this patch those specs
45+
// fail under both Playwright and a real Chrome tab.
46+
//
47+
// Wrap `focus`/`blur` to dispatch the missing events synchronously
48+
// after the native call. If the browser DOES regain system focus
49+
// and fires them too, observers see duplicates — harmless for
50+
// these specs (state-checking, not call-count).
7551
//
76-
// Fix: wrap `HTMLElement.prototype.focus` and `.blur` to ALSO
77-
// dispatch the `focus` / `focusin` / `blur` / `focusout` events
78-
// synchronously after the native call. If the browser ALSO fires
79-
// them (whenever it does regain system focus), observers see
80-
// duplicates — harmless for the specs in this suite (they observe
81-
// state, not call count). Scope-guarded to the iframe context
82-
// (`window.parent !== window`) so the parent page is never
83-
// patched.
52+
// Scope-guarded to iframes (`window.parent !== window`) so the
53+
// parent page is never patched.
8454
//
85-
// References:
86-
// - https://github.com/testing-library/user-event/issues/553
87-
// `.focus()` does not fire focus events if the window is not
88-
// focused. Confirmed across browsers; the same gate applies
89-
// to unfocused iframes.
90-
// - https://github.com/cypress-io/cypress/issues/8111
91-
// iframe elements that focus are blurred immediately when
92-
// they lack system focus.
93-
// - https://github.com/jsdom/jsdom/pull/2996
94-
// Reference implementation of synthetic focusin/focusout
95-
// dispatch in jsdom; same shape as the wrap below.
96-
// - https://html.spec.whatwg.org/multipage/interaction.html#focusing-elements
97-
// The spec allows `.focus()` to succeed programmatically even
98-
// when the owner isn't "being rendered"; event delivery,
99-
// however, is gated on system focus of the top-level browsing
100-
// context — that's the Chromium behaviour this patch bridges.
55+
// Refs: https://github.com/jsdom/jsdom/pull/2996 (same shape as
56+
// this wrap), https://html.spec.whatwg.org/multipage/interaction.html#focusing-elements.
10157
if (window.parent !== window && !HTMLElement.prototype.__tkoFocusPatched) {
10258
const HE = HTMLElement.prototype
10359
HE.__tkoFocusPatched = true
@@ -121,35 +77,22 @@ if (window.parent !== window && !HTMLElement.prototype.__tkoFocusPatched) {
12177
}
12278
}
12379

124-
// Spies, stubs, and fake timers installed via the non-sandboxed
125-
// `sinon.spy(obj, 'method')` / `sinon.stub(...)` / `sinon.useFakeTimers()`
126-
// APIs remain wrapped on their targets until explicitly restored.
127-
// Specs that forget to restore (or whose `this.after(...)` cleanup
128-
// entry ran out of order) leak state into the next test, producing
129-
// `+0 to deeply equal N` on call-count assertions or
130-
// `Can't install fake timers twice` when the next spec re-installs.
131-
// `sinon.restore()` is a no-op for sandbox-scoped fakes, so scoped
132-
// specs are unaffected — only the pathological globals get reset.
133-
// This hook lives only in the browser runner; Vitest's
134-
// per-file module isolation shields it from the problem.
80+
// Unscoped sinon fakes (`sinon.spy(obj,'m')`, `sinon.useFakeTimers()`)
81+
// leak across specs if not restored, producing bogus call-count
82+
// diffs or "Can't install fake timers twice". `sinon.restore()` is
83+
// a no-op for sandbox-scoped fakes. Vitest isolates per-file so
84+
// doesn't need this hook.
13585
afterEach(() => {
13686
if (globalThis.sinon?.restore) globalThis.sinon.restore()
13787
})
13888

13989
// Vitest-style context-arg shim.
14090
//
141-
// Some specs are written for Vitest and take the test context as a
142-
// single arg, e.g. `function (ctx: any) { if (isHappyDom()) return
143-
// ctx.skip('...') }`. Mocha inspects `fn.length` to decide whether
144-
// the test expects a `done` callback; a 1-arg function is treated as
145-
// async-with-done, and since these specs never call done(), Mocha
146-
// times them out (~10s each).
147-
//
148-
// Fix: wrap `it` so that 1-arg specs that look like ctx-style (use
149-
// `.skip(...)` and never call `done(...)`) are invoked with a fake
150-
// ctx `{ skip }` and the wrapper's arity is hidden from Mocha.
151-
// Genuine Mocha done-callback specs (identified by a `done(` call
152-
// in the source) pass through unchanged.
91+
// Specs written `function (ctx) { if (isHappyDom()) return ctx.skip(…) }`
92+
// look like Mocha done-callback specs (`fn.length === 1`) and time
93+
// out after ~10s because they never call done. Wrap `it` to detect
94+
// the ctx shape (uses `.skip(...)` and never calls `done(`) and
95+
// invoke with a synthetic `{ skip }` while hiding arity from Mocha.
15396
{
15497
const wrap = orig =>
15598
function (name, fn) {

0 commit comments

Comments
 (0)