From 4a017f76365b04836eca8821a20bd7273703c50a Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 11 Aug 2026 11:32:14 +0300 Subject: [PATCH] feat(playwright): stop tracing the happy path, report slow tests `trace: 'retain-on-failure'` records a trace for every test and throws it away when the test passes, so a fully green suite pays the tracing cost in full to keep artefacts for the tests that fail. `on-first-retry` moves that onto the retry, which `retries: 2` already guarantees under CI -- the same artefacts where they are useful, none of the overhead where they are not. `reportSlowTests: null` silenced Playwright's slow-test summary. That report is how you notice a suite going wrong: visual-portfolio has been running 56 tests at 10.4s each against 4.2-4.7s for every sibling suite, because a `beforeEach` loads the whole of wp-admin before each test to read one string. Nothing surfaced it for months. Restored at Playwright's default shape with a 30s threshold. Adds a CI workflow too. Only npm-publish existed, and it runs on tags, so until now nothing checked a pull request -- a change could only fail once it was already on its way to npm. Same lint + test pair, run before the merge. Minor rather than patch: both settings change behaviour for every consumer, and consumers pin ^0.5.1, so the bump has to be deliberate on their side. --- .github/workflows/ci.yml | 41 ++++++++++++++++++++++++++++++++++++++++ package.json | 2 +- playwright.js | 14 ++++++++++++-- scripts/self-test.js | 28 +++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..79b6621 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,41 @@ +name: CI + +on: + pull_request: + push: + branches: + - main + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }} + cancel-in-progress: true + +jobs: + # The same lint + test pair the publish workflow gates on, run before the + # merge rather than after the tag. Until now nothing checked a pull request, + # so a change could only fail once it was already on its way to npm. + # + # ubuntu-latest, not a Blacksmith runner: this repo is public, so these + # minutes are free, and the job is short enough that a faster runner would + # save nothing worth paying for. + verify: + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-node@v6 + with: + node-version: 22 + cache: npm + + - run: npm ci + + - run: npm run lint + + - run: npm test diff --git a/package.json b/package.json index 67992ad..8e31b4a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@nk-crew/plugin-toolkit", - "version": "0.5.1", + "version": "0.6.0", "description": "Shared development tooling for the nk-crew WordPress plugins", "license": "MIT", "author": "nK ", diff --git a/playwright.js b/playwright.js index e966d95..64028d9 100644 --- a/playwright.js +++ b/playwright.js @@ -65,7 +65,11 @@ function createPlaywrightConfig({ Number.parseInt(process.env.TIMEOUT || '', 10) || timeout || 200_000, - reportSlowTests: null, + // Playwright's default is `{ max: 5, threshold: 15000 }`; `null` silenced + // it entirely. That hid a suite where a `beforeEach` was loading the whole + // of wp-admin before every test, for months. The cost of the report is a + // few lines at the end of a run. + reportSlowTests: { max: 5, threshold: 30_000 }, testDir, globalSetup, outputDir: path.join(process.cwd(), 'artifacts/test-results'), @@ -91,7 +95,13 @@ function createPlaywrightConfig({ // per-test budget is gone, so a single stuck page load costs the // full `timeout` (and the same again for every retry). navigationTimeout: 30000, - trace: 'retain-on-failure', + // `retain-on-failure` records a trace for every test and throws it + // away when the test passes, so the whole suite pays for tracing to + // keep artefacts for the few tests that fail. `on-first-retry` moves + // that cost onto the retry, which `retries` above already guarantees + // for anything that fails under CI -- same artefacts where they are + // useful, none of the overhead where they are not. + trace: 'on-first-retry', screenshot: 'only-on-failure', video: 'on-first-retry', }, diff --git a/scripts/self-test.js b/scripts/self-test.js index 152d238..a18bf80 100755 --- a/scripts/self-test.js +++ b/scripts/self-test.js @@ -281,6 +281,34 @@ check('navigations are bounded, not left to the per-test budget', () => { ); }); +check('tracing is not paid for on the happy path', () => { + const { use, retries } = buildPlaywrightConfig(true); + + assert.equal( + use.trace, + 'on-first-retry', + '`retain-on-failure` traces every test and discards it on success, so a green suite pays the tracing cost in full' + ); + assert.ok( + retries > 0, + 'on-first-retry only yields artefacts if a failing test is actually retried under CI' + ); +}); + +check('slow tests are reported', () => { + const { reportSlowTests } = buildPlaywrightConfig(true); + + assert.notEqual( + reportSlowTests, + null, + 'null silences the one report that would surface an expensive beforeEach' + ); + assert.ok( + reportSlowTests && reportSlowTests.threshold > 0, + 'a threshold of 0 flags every test and so says nothing' + ); +}); + check('every runtime require is a declared dependency', () => { const declared = new Set([ ...Object.keys(pkg.dependencies || {}),