Skip to content

Modernize UI screenshot-testing npm stack → Node 24 - #24677

Merged
sgiehl merged 62 commits into
6.x-devfrom
dev-20341
Jul 24, 2026
Merged

Modernize UI screenshot-testing npm stack → Node 24#24677
sgiehl merged 62 commits into
6.x-devfrom
dev-20341

Conversation

@sgiehl

@sgiehl sgiehl commented Jun 22, 2026

Copy link
Copy Markdown
Member

Modernize the UI screenshot-testing npm stack → Node 24

Brings Matomo's UI screenshot tests (tests/lib/screenshot-testing/, a separate npm package) onto a modern, Node 24 toolchain — the last piece still pinned to Node 16. The blocker was puppeteer ^8.0.0 (Feb 2021), whose bundled Chromium can't be provisioned on modern Node.

Targets 6.x-dev. The Vue→Vite/Node 24 counterpart (#24668) has merged.

Note

Built up in sequential, individually-verified steps; the harness is verified locally in ddev (Node 24) and CI is driven green. Moving the UI jobs onto the new headless Chrome shifted a large number of expected screenshots — these have been reviewed old-vs-new and re-synced from CI artifacts (see below). New headless Chrome installs track the latest stable, so a rebase occasionally needs a further screenshot re-sync.

Plan / progress

  • Step 1 — Puppeteer 8 → 24 + harness API migration
    • puppeteer ^24, engines.node ">=24", lockfile regenerated to v3
    • API changes: createBrowserContext, page._client() (method), waitForTimeout shim (drop waitForTimeout/waitForXPath/$x from the proxy), JSHandle.evaluate() for console args
    • waitForNetworkIdle made tolerant of requests Chrome leaves stalled under request interception
    • in-repo spec fixes: OptOutJS (CDP), Overlay (popup)
  • Step 2 — replace deprecated request/request-promise with native fetch (Node 24 built-in); dropped request, request-promise, url-parse, unused chai-image-assert
  • Step 3 — dependency sweep: mocha 8 → 11, fs-extra 7 → 11, bumped @testomatio/reporter + mocha-multi-reporters; overrides trimmed to axios. (chai stays on 4 — chai 5/6 are ESM-only; deferred as tech debt.)
  • Step 4 — ddev Chromium provisioning (.ddev/initial-config/config.js — system Chromium on all arches)
  • Step 5 — flip CI UI jobs Node 16 → 24 (matomo-tests.yml, run-single-ui-test-suite.yml; JS job on Node 24; submodule plugins added to the UI matrix)
  • Step 6 — sync shifted expected screenshots from CI artifacts — core, core-plugin and submodule expected screenshots reviewed individually and re-synced. Each diff was checked as either new-Chrome rendering drift / legitimate 6.x content, or a genuinely-wrong render that was fixed at the test level instead of synced (see review below).

Chrome stabilization (surfaced by the new headless Chrome)

Moving to modern headless Chrome exposed several deterministic behaviour changes, all fixed:

  • harness waitForNetworkIdle tolerates a navigation destroying the execution context mid-evaluate (fixed both OneClick real-update tests)
  • scrollIntoViewIfNeeded bounded so a stuck IntersectionObserver can't hang clicks/screenshots to the mocha timeout
  • open modals forced to their final state (+ settled inline styles) before element screenshots (TagManager modals)
  • request interception only enabled when a port rewrite is needed; browser cache disabled in the harness
  • Goals: waitForXPathxpath/ query-handler prefix; ManageGoals forced full reload (hash-only goto no longer reloads under the new Chrome, so tests inherited the prior form/notification state)
  • UsersManager: password confirmations set the field value + dispatch input/change (reliable for the special-char superuser password across the several duplicated #currentUserPassword modals) instead of page.type; bulk-remove waits for the list reload before selecting; bulk_set_access waits for the change to apply before capturing; delete_bulk_confirm focuses the field before capture; the "give access to all websites" Materialize-select header height is settled before capturing (permissions_next/permissions_single_site_access)
  • per-spec fixes: TagManager view-user access + password-confirmation modal, Dashboard copy-name emoji typing, SegmentEditor apply-reload + archive timeout, Menus mobile admin accordion, CustomDimensions code-block render wait; sub-pixel comparison thresholds for UsersManager user-table shots

Review of the shifted screenshots

Every failing screenshot was compared old-vs-new before syncing. Several genuinely-wrong renders were caught and fixed at the test level rather than synced:

  • Menus mobile-admin highlighted the wrong item — racy category-header clicks under the new Chrome; clicks scoped to the collapsible headers
  • MarketingCampaignsReporting loaded_ecommerce — sticky DataTable header floating mid-capture on a tall element; viewport grown to fit the report
  • CustomAlerts alert-condition dropdown — stray :hover on an option (opened via evaluate, cursor left resting); mouse moved off before capture
  • UsersManager bulk_set_access — captured before the bulk change applied and the list refreshed; settle-wait added
  • config-file diagnostics and api_listing re-synced after each rebase (content unchanged / new API methods; new-Chrome font rendering)

Genuinely-wrong CI renders (blank Comparison widget, OneClick CSRF-error screen) were not synced.

Notes / follow-ups

AI usage checklist

  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules

@sgiehl sgiehl added this to the 6.0.0 milestone Jun 22, 2026

@sgiehl sgiehl left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

incorrectly updates screenshots

@sgiehl
sgiehl force-pushed the dev-17333 branch 2 times, most recently from e9573c5 to e23455a Compare July 6, 2026 05:56
@sgiehl
sgiehl force-pushed the dev-20341 branch 2 times, most recently from 3dd1874 to 81ddc6b Compare July 6, 2026 18:22
@sgiehl
sgiehl force-pushed the dev-17333 branch 3 times, most recently from accbb95 to cead076 Compare July 8, 2026 08:22
@sgiehl
sgiehl force-pushed the dev-17333 branch 4 times, most recently from 431b97a to aac7203 Compare July 16, 2026 09:21
Base automatically changed from dev-17333 to 6.x-dev July 16, 2026 11:43
This was referenced Jul 21, 2026
sgiehl added 19 commits July 24, 2026 08:08
The committed expected was a clipped capture from the earlier bulk rebase re-sync,
missing the '<- Back to Users' link at the top. The current CI render (consistent
across runs) includes the back-link and full heading; re-synced to it. Addresses the
reopened review comment about this screenshot looking off.
…icky header)

Advance the gitlink to the pushed dev-20341 commit that fixes the
ReportsByDimensionAddition ecommerce screenshot capture (position:sticky DataTable
header was floating mid-table under Puppeteer 24).
Advance the gitlink to the pushed dev-20341 commit that moves the mouse off the
alert-condition dropdown before capturing alert_condition_reloaded_site1/site2, so no
option keeps a stray :hover highlight under Puppeteer 24 / Chrome 149.
The permissions page renders several password-confirmation modals that all reuse the
#currentUserPassword id, so page.type by that id could hit the wrong (hidden) field or
append to a pre-filled one, sending an incorrect password (400 on CI). Target the visible
field in the open modal and set its value directly.
…lossary

- Comparison large-range widget: wait for the evolution graph (.piwik-graph) before
  capturing, matching the sibling compare-period test; it was captured blank otherwise.
- Widgetized glossary: wait for network idle (entries load asynchronously) instead of a
  fixed 200ms timeout.
The earlier attempt used an input[type=password]:visible selector that never matched and
timed out (cascading into later tests). Revert to the proven .confirm-password-modal.open
#currentUserPassword selector used by the sibling single-site test, and clear the field
first so page.type does not append to a pre-filled (autofilled) value and send a wrong
password (the 400 seen on CI).
The unconditional waitForSelector on the password modal timed out (30s) when the modal
did not appear and cascaded failures into the following access-change tests. Restore the
conditional handling (fill the modal only when it is shown) so a missing modal no longer
breaks later tests. The underlying CI-only failure of this one test remains under
investigation.
Rendering drift only (identical content) from the new base's headless Chrome.
…am-add

- TagManager (submodule bump): comparisonThreshold on publish_with_content for its
  intermittent ~0.3% whole-modal variance.
- SitesManager add-recommended-params: wait for the added parameters to render before
  capturing so the block height settles.
Root cause of the cascading UsersManager permission failures on CI: several
confirm-password modals share the #currentUserPassword id and the field retains a value,
so page.type appended and submitted a wrong password. The first affected test (setting
sites to Admin) therefore never applied the access, and every later test that filters or
depends on that access failed in turn. Clear the field before typing at each
confirm-password-modal fill.
The set-access (all-sites, single-site, change-access, bulk-admin) and set-superuser
confirmations submitted a wrong/empty password on CI - #currentUserPassword is duplicated
across the several confirm-password modals and page.type of the special-char superuser
password (and appending to a retained value) was unreliable. Route them through a helper
that sets the value on the visible open-modal field and dispatches input/change so the Vue
model updates, then confirms.
bulk_set_access was captured before the bulk change applied and the list refreshed (rows
still selected, roles not yet updated); wait for the list to settle (selection cleared)
before capturing. delete_bulk_confirm intermittently lost the password field's focus state
by capture time under the new headless Chrome; focus it right before the screenshot.
…firm screenshots

Benign new-headless-Chrome rendering drift (identical content), reviewed against the
previous expected.
…creenshots

api_listing grew with new API methods from the rebase; delete_bulk_confirm now captures
the focused password field (rendering drift only); permissions_next is a clean render
(identical content, back-link present) - all reviewed against the previous expected.
permissions_next and permissions_single_site_access flaked because the 'give access to
all websites' Materialize select reflows to a slightly taller height after render,
shifting the site table below it. Wait for that header's height to stop changing before
capturing so the table position is stable.
Config-file listing content is unchanged (section list + values identical); the diff is
new-headless-Chrome font rendering across the long monospace page.
sgiehl added 2 commits July 24, 2026 08:48
Rendering/spacing drift only (identical content), reviewed against the previous expected.
…shold

The settle wait reduces but does not fully eliminate the run-to-run header-height variance
on permissions_next/permissions_single_site_access, so allow a small comparison threshold
(matching the sibling user-table shots) instead of flaking.
@sgiehl
sgiehl requested a review from a team July 24, 2026 08:04
@sgiehl
sgiehl enabled auto-merge (squash) July 24, 2026 08:05
@sgiehl
sgiehl merged commit 7d893bc into 6.x-dev Jul 24, 2026
331 of 333 checks passed
@sgiehl
sgiehl deleted the dev-20341 branch July 24, 2026 08:39
tzi added a commit that referenced this pull request Jul 27, 2026
…-dev merge [DEV-20451]

The 6.x-dev merge resolved four conflicting expected screenshots to upstream's
Node-24 renders (#24677 re-rendered nearly every screenshot in the repo), which
dropped this branch's content from them. These three are restored from the CI
artifacts of build 30253818158:

- configfile: re-adds the record_accurate_page_view_time [Tracker] row
- Installation_db_existing: re-adds matomo_log_page_view_time to the table list
- Live action tooltip: time on page 6 min 36s -> 12 min 0s (accurate metric)

UsersManager_permissions_next.png needed no change; upstream's render already
matches, and its UI job passed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: Tests & QA For issues related to automated tests or making it easier to QA & test issues.

Development

Successfully merging this pull request may close these issues.

3 participants