diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 04760e1f..2c41202d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -543,6 +543,10 @@ jobs: done - name: Run Playwright E2E tests (fail-fast) + env: + # On master pushes every suite records the frontend coverage of the + # pages it opens into .nyc_output/ (#2203). Off on PRs to keep them fast. + NODE_OPTIONS: ${{ github.event_name == 'push' && '--require ./scripts/e2e-coverage-hook.js' || '' }} run: | # suite [VAR=value ...]: runs tests/e2e/ with # exactly those variables, but only on its own shard. The shard numbers @@ -720,29 +724,26 @@ jobs: done echo "3 passed" - # Shard 2 runs test-e2e-playwright.js, the one suite that writes - # .nyc_output, so the coverage collection and its badge live there too. + # An extra coverage source on top of the suites: a tour of the main + # page groups. Its files land in .nyc_output/ beside the suites' files. - name: Collect frontend coverage (parallel) if: success() && github.event_name == 'push' && matrix.shard == 2 run: | BASE_URL=http://localhost:13581 node scripts/collect-frontend-coverage.js 2>&1 | tee fe-coverage-output.txt || true - - name: Generate frontend coverage badge - if: success() + # One file per suite is about 6 MB, mostly the same maps, so the shard + # merges them before upload. e2e-test merges the three shards and makes + # the frontend coverage badge. + # Coverage never decides the outcome: a failure here must not skip the + # shard upload below (if: success()) and so fail the E2E gate. + - name: Merge this shard's frontend coverage + if: success() && github.event_name == 'push' + continue-on-error: true run: | - # .badges/ is tracked on master, so the checkout already holds the - # old badge files; the shard upload would ship them as if this run - # had produced them (#2136). Start empty. - rm -rf .badges && mkdir .badges - if [ -f .nyc_output/frontend-coverage.json ] || [ -f .nyc_output/e2e-coverage.json ]; then - npx nyc report --reporter=text-summary --reporter=text 2>&1 | tee fe-report.txt - FE_COVERAGE=$(grep 'Statements' fe-report.txt | head -1 | grep -oP '[\d.]+(?=%)' || echo "0") - FE_COVERAGE=${FE_COVERAGE:-0} - FE_COLOR="red" - [ "$(echo "$FE_COVERAGE > 50" | bc -l 2>/dev/null)" = "1" ] && FE_COLOR="yellow" - [ "$(echo "$FE_COVERAGE > 80" | bc -l 2>/dev/null)" = "1" ] && FE_COLOR="brightgreen" - echo "{\"schemaVersion\":1,\"label\":\"frontend coverage\",\"message\":\"${FE_COVERAGE}%\",\"color\":\"${FE_COLOR}\"}" > .badges/frontend-coverage.json - echo "## Frontend: ${FE_COVERAGE}% coverage" >> $GITHUB_STEP_SUMMARY + if ls .nyc_output/*.json >/dev/null 2>&1; then + npx nyc merge .nyc_output coverage-shard.json + else + echo "::warning::no frontend coverage was recorded on shard ${{ matrix.shard }}" fi - name: Stop test server @@ -764,7 +765,7 @@ jobs: name: e2e-shard-${{ matrix.shard }} path: | e2e-output.txt - .badges/ + coverage-shard.json retention-days: 1 if-no-files-found: ignore include-hidden-files: true @@ -803,10 +804,9 @@ jobs: ls shards [ "$(ls -d shards/e2e-shard-* | wc -l)" -eq 3 ] || { echo "::error::expected results from 3 shards"; exit 1; } cat shards/e2e-shard-*/e2e-output.txt > e2e-output.txt - # Start empty, as in the shards: the checkout holds master's old - # .badges/ files, which would otherwise go out in e2e-badges (#2136). + # Start empty: the checkout holds master's old .badges/ files, + # which would otherwise go out in e2e-badges (#2136). rm -rf .badges && mkdir .badges - cp shards/e2e-shard-*/.badges/*.json .badges/ 2>/dev/null || true # Aggregate per-suite PASS/FAIL across every test-*-e2e.js summary. # The previous regex (grep -oP '[0-9]+(?=/)' | tail -1) caught a # stray digits-before-slash like the '2' in '2/3 tests passed' from @@ -824,6 +824,40 @@ jobs: echo "{\"schemaVersion\":1,\"label\":\"e2e tests\",\"message\":\"${E2E_MSG}\",\"color\":\"${E2E_COLOR}\"}" > .badges/e2e-tests.json echo "E2E: ${E2E_MSG} ($(grep -c '^=== E2E SUITE' e2e-output.txt) suite runs)" | tee -a "$GITHUB_STEP_SUMMARY" + - name: Set up Node.js 22 + if: github.event_name == 'push' + continue-on-error: true + uses: actions/setup-node@v5 + with: + node-version: '22' + + # Coverage from every suite on every shard (#2203), merged into one + # number. Only master pushes record it, and it never fails the gate. + - name: Generate frontend coverage badge + if: github.event_name == 'push' + continue-on-error: true + run: | + mkdir -p .nyc_output + for f in shards/e2e-shard-*/coverage-shard.json; do + [ -f "$f" ] && cp "$f" ".nyc_output/$(basename "$(dirname "$f")").json" + done + if ! ls .nyc_output/*.json >/dev/null 2>&1; then + echo "::warning::no shard recorded frontend coverage; the badge keeps its previous value" + exit 0 + fi + echo "Frontend coverage from: $(ls .nyc_output | paste -sd ' ' -)" + npx --yes nyc@18.0.0 report --reporter=text-summary --reporter=text 2>&1 | tee fe-report.txt + FE_COVERAGE=$(grep 'Statements' fe-report.txt | head -1 | grep -oP '[\d.]+(?=%)' || true) + if [ -z "$FE_COVERAGE" ]; then + echo "::warning::nyc report gave no statement coverage; the badge keeps its previous value" + exit 0 + fi + FE_COLOR="red" + [ "$(echo "$FE_COVERAGE > 50" | bc -l 2>/dev/null)" = "1" ] && FE_COLOR="yellow" + [ "$(echo "$FE_COVERAGE > 80" | bc -l 2>/dev/null)" = "1" ] && FE_COLOR="brightgreen" + echo "{\"schemaVersion\":1,\"label\":\"frontend coverage\",\"message\":\"${FE_COVERAGE}%\",\"color\":\"${FE_COLOR}\"}" > .badges/frontend-coverage.json + echo "## Frontend: ${FE_COVERAGE}% coverage" >> "$GITHUB_STEP_SUMMARY" + - name: Upload E2E badges uses: actions/upload-artifact@v6 with: diff --git a/scripts/e2e-coverage-hook.js b/scripts/e2e-coverage-hook.js new file mode 100644 index 00000000..dee310a3 --- /dev/null +++ b/scripts/e2e-coverage-hook.js @@ -0,0 +1,18 @@ +'use strict'; +// Preload for the E2E shards (#2203): `NODE_OPTIONS=--require ./scripts/e2e-coverage-hook.js`. +// Every Chromium the suite launches is tracked by scripts/e2e-coverage.js, and +// the summed coverage is written to .nyc_output/ when the process exits. A +// suite that never loads an instrumented page writes nothing. + +const fs = require('fs'); +const path = require('path'); +const { chromium } = require('playwright'); +const { createCollector, writeCoverage } = require('./e2e-coverage'); + +const collector = createCollector(); +collector.wrapLaunch(chromium); + +process.on('exit', () => { + writeCoverage(fs, path, collector.store, path.resolve(__dirname, '..', '.nyc_output'), + path.basename(process.argv[1] || 'node', '.js'), process.pid); +}); diff --git a/scripts/e2e-coverage.js b/scripts/e2e-coverage.js new file mode 100644 index 00000000..1115986b --- /dev/null +++ b/scripts/e2e-coverage.js @@ -0,0 +1,154 @@ +'use strict'; +// Collects window.__coverage__ from every page a Playwright E2E suite opens +// (#2203). Loaded through scripts/e2e-coverage-hook.js; kept free of +// Playwright and fs so tests/unit/test-e2e-coverage.js can drive it with fakes. +// +// A page's counters live until its document is replaced, so they are read +// (and zeroed) before every call that can replace or end the document. A +// full load the suite does not start through one of these calls, such as a +// click on a link that leaves the SPA, is not counted. + +const PAGE_CALLS = ['goto', 'reload', 'setContent', 'goBack', 'goForward', 'close']; + +// A read must never hold up the call it runs before: a page stuck in a busy +// loop would otherwise keep browser.close() in a suite's cleanup waiting for +// as long as the loop runs. Past this, the read is given up. +const READ_TIMEOUT_MS = 5000; + +// Runs inside the page. Returns the hit counts since the last read and zeroes +// them, so a document read twice is not counted twice. A file the caller has +// not seen yet comes back whole (with its statement, function and branch maps). +// The result is a JSON string: Playwright serialises a returned object node by +// node, which took about 10 s for the roughly 6 MB of maps of the first read. +function readAndReset(known) { + const cov = window.__coverage__; + if (!cov) return null; + const seen = new Set(known); + const out = {}; + for (const file of Object.keys(cov)) { + const fc = cov[file]; + out[file] = seen.has(file) ? { s: fc.s, f: fc.f, b: fc.b } : fc; + } + const json = JSON.stringify(out); + for (const file of Object.keys(cov)) { + const fc = cov[file]; + for (const k in fc.s) fc.s[k] = 0; + for (const k in fc.f) fc.f[k] = 0; + for (const k in fc.b) fc.b[k].fill(0); + } + return json; +} + +// Adds a snapshot from readAndReset to store, summing the hit counts. +function mergeInto(store, snapshot) { + for (const file of Object.keys(snapshot)) { + const add = snapshot[file]; + const into = store[file]; + if (!into) { + store[file] = add; + continue; + } + for (const k in add.s) into.s[k] = (into.s[k] || 0) + add.s[k]; + for (const k in add.f) into.f[k] = (into.f[k] || 0) + add.f[k]; + for (const k in add.b) { + const sum = into.b[k] || add.b[k].map(() => 0); + add.b[k].forEach((n, i) => { sum[i] = (sum[i] || 0) + n; }); + into.b[k] = sum; + } + } +} + +function createCollector({ readTimeoutMs = READ_TIMEOUT_MS } = {}) { + const store = {}; + const pages = new WeakSet(); + const contexts = new WeakSet(); + + async function collect(page) { + if (page.isClosed()) return; + let timer; + const timeout = new Promise(resolve => { timer = setTimeout(resolve, readTimeoutMs, null); }); + let snapshot; + try { + snapshot = await Promise.race([page.evaluate(readAndReset, Object.keys(store)), timeout]); + } catch { + return; // page crashed or navigated away mid-read: nothing to add + } finally { + clearTimeout(timer); + } + if (snapshot) mergeInto(store, JSON.parse(snapshot)); + } + + function readFirst(obj, names, read) { + for (const name of names) { + const orig = obj[name]; + if (typeof orig !== 'function') continue; + obj[name] = async function (...args) { + await read(); + return orig.apply(this, args); + }; + } + } + + function trackPage(page) { + if (pages.has(page)) return page; + pages.add(page); + readFirst(page, PAGE_CALLS, () => collect(page)); + return page; + } + + function trackContext(context) { + if (contexts.has(context)) return context; + contexts.add(context); + context.pages().forEach(trackPage); + context.on('page', trackPage); + readFirst(context, ['close'], () => Promise.all(context.pages().map(collect))); + const newPage = context.newPage; + context.newPage = async function (...args) { + return trackPage(await newPage.apply(this, args)); + }; + return context; + } + + function trackBrowser(browser) { + const newContext = browser.newContext; + browser.newContext = async function (...args) { + return trackContext(await newContext.apply(this, args)); + }; + const newPage = browser.newPage; + browser.newPage = async function (...args) { + const page = await newPage.apply(this, args); + trackContext(page.context()); + return trackPage(page); + }; + readFirst(browser, ['close'], () => + Promise.all(browser.contexts().flatMap(c => c.pages()).map(collect))); + return browser; + } + + function wrapLaunch(browserType) { + const launch = browserType.launch; + browserType.launch = async function (...args) { + return trackBrowser(await launch.apply(this, args)); + }; + } + + return { store, collect, trackPage, trackContext, trackBrowser, wrapLaunch }; +} + +// Writes store as one nyc input file, or nothing when it is empty. Never +// throws: it runs in an exit handler, where a throw would change the suite's +// exit code. Returns the file written, or null. +function writeCoverage(fs, path, store, outDir, suite, pid) { + if (Object.keys(store).length === 0) return null; + try { + fs.mkdirSync(outDir, { recursive: true }); + const file = path.join(outDir, `e2e-${suite}-${pid}.json`); + fs.writeFileSync(file, JSON.stringify(store)); + return file; + } catch (e) { + console.error(`e2e-coverage: not written: ${e.message}`); + return null; + } +} + +module.exports = { readAndReset, mergeInto, createCollector, writeCoverage }; diff --git a/test-all.sh b/test-all.sh index 2b17ee0a..2af3aa4b 100755 --- a/test-all.sh +++ b/test-all.sh @@ -48,6 +48,7 @@ node tests/unit/test-confidence-indicator.js node tests/unit/test-coverage-gate.js node tests/unit/test-customizer-v2.js node tests/unit/test-drag-manager.js +node tests/unit/test-e2e-coverage.js node tests/unit/test-direct-rf-heard-by.js node tests/unit/test-embed-mode-1369.js node tests/unit/test-fetch-all-nodes-pagination.js diff --git a/tests/e2e/test-e2e-playwright.js b/tests/e2e/test-e2e-playwright.js index 44984552..43a5a1dd 100644 --- a/tests/e2e/test-e2e-playwright.js +++ b/tests/e2e/test-e2e-playwright.js @@ -2603,19 +2603,6 @@ async function run() { // ─── End #1468 tests ────────────────────────────────────────────────────── - // Extract frontend coverage if instrumented server is running - try { - const coverage = await page.evaluate(() => window.__coverage__); - if (coverage) { - const fs = require('fs'); - const path = require('path'); - const outDir = path.join(REPO_ROOT, '.nyc_output'); - if (!fs.existsSync(outDir)) fs.mkdirSync(outDir, { recursive: true }); - fs.writeFileSync(path.join(outDir, 'e2e-coverage.json'), JSON.stringify(coverage)); - console.log(`Frontend coverage from E2E: ${Object.keys(coverage).length} files`); - } - } catch {} - // --- Group: Deep linking (#536) --- // Test: nodes tab deep link diff --git a/tests/unit/test-e2e-coverage.js b/tests/unit/test-e2e-coverage.js new file mode 100644 index 00000000..255d7c25 --- /dev/null +++ b/tests/unit/test-e2e-coverage.js @@ -0,0 +1,223 @@ +/* scripts/e2e-coverage.js (#2203): every E2E page's __coverage__ is read before + * its document is replaced or closed, summed, and never counted twice. Driven + * with fake pages, so no browser is needed. */ +'use strict'; +const REPO_ROOT = require('path').resolve(__dirname, '..', '..'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const os = require('os'); +const { readAndReset, mergeInto, createCollector, writeCoverage } = require('../../scripts/e2e-coverage'); + +// Playwright calls that replace a page's document. Listed here, not taken from +// the module, so dropping one there fails this test. +const NEW_DOCUMENT = ['goto', 'reload', 'setContent', 'goBack', 'goForward']; + +// One instrumented file as istanbul shapes it: statement, function and branch +// counters plus the maps that only need to travel once. +function freshFile() { + return { + path: 'public/app.js', + statementMap: { 0: {}, 1: {} }, fnMap: { 0: {} }, branchMap: { 0: {} }, + s: { 0: 0, 1: 0 }, f: { 0: 0 }, b: { 0: [0, 0] }, + }; +} + +// A fake Playwright page: evaluate runs the function against this page's +// window, and goto/reload/setContent/goBack/goForward load a new document +// (fresh counters), as a real full load does. +function fakePage(context) { + const page = { + win: { __coverage__: { 'public/app.js': freshFile() } }, + closed: false, + isClosed() { return this.closed; }, + async evaluate(fn, arg) { + const saved = global.window; + global.window = this.win; + try { return fn(arg); } finally { global.window = saved; } + }, + context() { return context; }, + hit(stmt, times) { this.win.__coverage__['public/app.js'].s[stmt] += times; }, + }; + for (const name of NEW_DOCUMENT) { + page[name] = async function () { + this.win = { __coverage__: { 'public/app.js': freshFile() } }; + }; + } + page.close = async function () { this.closed = true; }; + return page; +} + +function fakeContext() { + const listeners = []; + const ctx = { + list: [], + pages() { return this.list.filter(p => !p.closed); }, + on(event, fn) { if (event === 'page') listeners.push(fn); }, + // A page the context opens by itself (window.open): only the 'page' + // event announces it. + open() { + const p = fakePage(ctx); + this.list.push(p); + listeners.forEach(fn => fn(p)); + return p; + }, + async newPage() { return this.open(); }, + async close() { this.list.forEach(p => { p.closed = true; }); }, + }; + return ctx; +} + +function fakeBrowser() { + const browser = { + ctxs: [], + contexts() { return this.ctxs; }, + async newContext() { const c = fakeContext(); this.ctxs.push(c); return c; }, + async newPage() { const c = await this.newContext(); return c.newPage(); }, + async close() { this.ctxs.forEach(c => c.list.forEach(p => { p.closed = true; })); }, + }; + return browser; +} + +async function main() { + // readAndReset: a new file comes back whole, a known one as counts only, + // and the page's counters are zero afterwards. + { + global.window = { __coverage__: { 'public/app.js': freshFile() } }; + window.__coverage__['public/app.js'].s[0] = 3; + window.__coverage__['public/app.js'].b[0][1] = 2; + const first = JSON.parse(readAndReset([])); + assert.ok(first['public/app.js'], 'an instrumented file comes back'); + assert.strictEqual(first['public/app.js'].s[0], 3); + assert.deepStrictEqual(first['public/app.js'].b[0], [0, 2]); + assert.ok(first['public/app.js'].statementMap, 'an unseen file must carry its maps'); + assert.deepStrictEqual(window.__coverage__['public/app.js'].s, { 0: 0, 1: 0 }, 'statement counters are zeroed'); + assert.deepStrictEqual(window.__coverage__['public/app.js'].b[0], [0, 0], 'branch counters are zeroed'); + const again = JSON.parse(readAndReset(['public/app.js'])); + assert.strictEqual(again['public/app.js'].statementMap, undefined, 'a known file travels as counts only'); + assert.strictEqual(again['public/app.js'].s[0], 0, 'a second read of the same document adds nothing'); + global.window = {}; + assert.strictEqual(readAndReset([]), null, 'an uninstrumented page gives nothing'); + delete global.window; + } + + // mergeInto sums every counter and copies files it has not seen. + { + const store = {}; + const a = freshFile(); a.s[0] = 1; a.b[0] = [1, 0]; + mergeInto(store, { 'public/app.js': a }); + mergeInto(store, { 'public/app.js': { s: { 0: 2, 1: 5 }, f: { 0: 1 }, b: { 0: [0, 4] } } }); + assert.deepStrictEqual(store['public/app.js'].s, { 0: 3, 1: 5 }); + assert.deepStrictEqual(store['public/app.js'].f, { 0: 1 }); + assert.deepStrictEqual(store['public/app.js'].b, { 0: [1, 4] }); + } + + // Every document a page shows is counted once: before each call that + // replaces it, and before close. + for (const name of NEW_DOCUMENT) { + const c = createCollector(); + const browser = fakeBrowser(); + c.trackBrowser(browser); + const page = await browser.newPage(); + page.hit(0, 1); + await page[name](); + page.hit(0, 2); + page.hit(1, 1); + await page.close(); + assert.deepStrictEqual(c.store['public/app.js'].s, { 0: 3, 1: 1 }, name + ': both documents counted once'); + await browser.close(); + assert.deepStrictEqual(c.store['public/app.js'].s, { 0: 3, 1: 1 }, name + ': a closed page is not read again'); + } + + // Pages left open are read when their context or browser closes, including + // pages from browser.newContext() and ones the page itself opened. + { + const c = createCollector(); + const browser = fakeBrowser(); + c.trackBrowser(browser); + const ctx = await browser.newContext(); + const p1 = await ctx.newPage(); + p1.hit(0, 1); + await ctx.close(); + assert.strictEqual(c.store['public/app.js'].s[0], 1, 'context.close reads its pages'); + + const ctx2 = await browser.newContext(); + const popup = ctx2.open(); + popup.hit(1, 2); + await popup.goto(); + assert.strictEqual(c.store['public/app.js'].s[1], 2, 'a page opened by the page itself is tracked too'); + const p2 = await ctx2.newPage(); + p2.hit(1, 4); + await browser.close(); + assert.strictEqual(c.store['public/app.js'].s[1], 6, 'browser.close reads the pages still open'); + } + + // wrapLaunch tracks what launch returns. + { + const c = createCollector(); + const browserType = { async launch() { return fakeBrowser(); } }; + c.wrapLaunch(browserType); + const browser = await browserType.launch(); + const page = await browser.newPage(); + page.hit(0, 7); + await browser.close(); + assert.strictEqual(c.store['public/app.js'].s[0], 7); + } + + // A read that fails or hangs is given up: the call it runs before still + // happens, promptly, and nothing is added. + { + const c = createCollector({ readTimeoutMs: 50 }); + const browser = fakeBrowser(); + c.trackBrowser(browser); + const broken = await browser.newPage(); + broken.evaluate = async () => { throw new Error('Target crashed'); }; + await broken.close(); + assert.ok(broken.closed, 'close still runs after a failed read'); + + const busy = await browser.newPage(); + busy.evaluate = () => new Promise(() => {}); // a page stuck in a busy loop + const started = Date.now(); + await busy.close(); + assert.ok(busy.closed, 'close still runs after a read that never answers'); + assert.ok(Date.now() - started < 2000, 'the read is given up after the timeout, took ' + (Date.now() - started) + ' ms'); + assert.deepStrictEqual(c.store, {}, 'a failed or hung read adds nothing'); + } + + // writeCoverage: one file when there is coverage, none without, and never + // a throw (it runs in an exit handler). + { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'e2e-cov-')); + const out = path.join(dir, 'nyc'); + assert.strictEqual(writeCoverage(fs, path, {}, out, 'suite', 1), null, 'nothing to write'); + assert.ok(!fs.existsSync(out), 'no directory without coverage'); + const store = { 'public/app.js': freshFile() }; + const file = writeCoverage(fs, path, store, out, 'test-x-e2e', 42); + assert.strictEqual(path.basename(file), 'e2e-test-x-e2e-42.json'); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(file, 'utf8')), store); + const fullDisk = { mkdirSync() {}, writeFileSync() { throw new Error('ENOSPC'); } }; + const savedError = console.error; + console.error = () => {}; + try { + assert.strictEqual(writeCoverage(fullDisk, path, store, out, 'suite', 1), null, 'a failed write returns null'); + } finally { + console.error = savedError; + } + } + + finished = true; + console.log('test-e2e-coverage: ok'); +} + +// A read that waits forever leaves nothing for node to wait on, so the +// process would exit 0 half-way through the test without this check. +let finished = false; +process.on('exit', code => { + if (code === 0 && !finished) { + console.error('test-e2e-coverage: stopped before the end (a read never returned)'); + process.exitCode = 1; + } +}); + +main().catch(e => { console.error(e); process.exit(1); });