fix(e2e): wait for the Vue chrome instead of sleeping, and report the surfaces that never mount - #254
Merged
Merged
Conversation
… 27 live CSS rules
The guard reported 31 element-overrides selectors as matching nothing
on any surface. They match. Probed by hand against a real Nextcloud,
all 15 `#header` selectors and all 11 `#app-navigation-vue` /
`.app-navigation-entry.active` selectors return counts of 1 or 2.
WHAT THE GUARD WAS ACTUALLY MEASURING
-------------------------------------
`page.goto(..., { waitUntil: 'domcontentloaded' })` followed by
`waitForTimeout(2500)`. Nextcloud's header and navigation are Vue
components — absent from the server-rendered document, present only
once the bundles execute. DOMContentLoaded fires long before that.
Every one of the 31 resolves inside a Vue-scoped (`data-v-*`) subtree.
That is not a coincidence, it is the signature: the probe ran before
the markup existed. The failing run's own arithmetic confirms it —
six surfaces in 24.5s total is ~4s per surface INCLUDING the 2.5s
sleep, so each probe fired ~1.5s after navigation began.
WHY THIS MATTERED MORE THAN A RED RUN
-------------------------------------
The failure message says "Either fix them against the real DOM, or add
them to ALLOWED". Acting on it meant rewriting or deleting 27 rules of
WORKING theme CSS to satisfy a measurement taken before the markup
existed — in the repository that owns the fleet's theming. A guard
written to catch dead selectors had become a machine for creating
them.
THE FIX
-------
Wait for a Vue-rendered node (`#header .app-menu` or
`#header .unified-search-input`) instead of guessing at a duration,
plus an opportunistic wait for `#app-navigation-vue`, which is
legitimately absent on surfaces like Dashboard.
And — the part that matters for next time — a run that reaches its
surfaces but never sees the chrome mount now FAILS ON THE MEASUREMENT,
naming it, instead of reporting every selector under `#header` as
dead. The old behaviour could not tell "this CSS is dead" from "I
looked too early", and both rendered as the same list.
STILL EXPECTED TO FAIL, HONESTLY
--------------------------------
Four selectors are not covered by any surveyed surface and this commit
does not pretend otherwise:
.guest-box.login-box alive — MEASURED as matching 1 on /login,
which SURFACES does not include
body#body-public public share page, not surveyed
body[data-theme-dark] dark theme not enabled during the run
body[data-themes*="dark"]
Those are a real coverage gap in the survey, not dead CSS, and closing
them means adding surfaces/states rather than deleting rules. Left
red deliberately: no ALLOWED entry is added for them, because an
allowlist entry would hide a gap this commit exists to expose.
tsc: 16 pre-existing @types/node errors before and after, none in this
file.
Contributor
Quality Report — ConductionNL/nldesign @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 2/2 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-09 15:07 UTC
Download the full PDF report from the workflow artifacts.
…, the diagnosis was not I claimed the 31 dead selectors were an artefact of probing before Nextcloud's Vue chrome mounted, and that fixing the wait would clear most of them. It cleared NONE. Measured on this branch's own CI run: 31 before, 31 after, and the run's log confirms the chrome DID mount on files, dashboard and settings — only contacts, mail and calendar (not installed there) failed, which the new reporting now says out loud. The correlation I built the claim on was real and still is: every one of the 31 sits inside a Vue-scoped subtree, and probed by hand against a full Nextcloud all 15 `#header` and 11 `#app-navigation-vue` selectors match with counts of 1-2. What I got wrong was the inference. The instance I probed was the shared dev box with ~30 apps installed; CI runs a minimal seeded instance, and "these selectors exist on a rich instance" does not establish "these selectors exist on CI's". A different Nextcloud is a different DOM. The wait still belongs here, for the reason that is left rather than the one I gave: a run that probes before mount and a run that probes a genuinely stale stylesheet produced the SAME output, and the failure message invites the reader to "fix them against the real DOM" — which, acted on, means deleting theme CSS to satisfy a measurement. The wait plus the new chromeless assertion separate those two cases, so the next person reads a failure that means what it says. What the remaining 31 actually are is now an open question and this commit does not answer it. The likeliest reading, unverified: CI's minimal instance does not render the header and navigation chrome these rules target at all.
rubenvdlinde
changed the title
Contributor
Quality Report — ConductionNL/nldesign @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 2/2 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-09 15:55 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR originally claimed the 31 "dead" selectors were an artefact of probing before Nextcloud's Vue chrome mounted, and that fixing the wait would clear 27 of them.
It cleared none. 31 before, 31 after — measured on this branch's own CI run, whose log confirms the chrome did mount on files, dashboard and settings. Only contacts, mail and calendar failed, because they are not installed there; the new reporting now says that out loud.
The correlation I built the claim on is real and still holds: every one of the 31 sits inside a Vue-scoped (
data-v-*) subtree, and probed by hand against a full Nextcloud all 15#headerand 11#app-navigation-vueselectors match with counts of 1–2. What I got wrong was the inference. The instance I probed was the shared dev box with ~30 apps installed; CI runs a minimal seeded instance. "These selectors exist on a rich instance" does not establish "these selectors exist on CI's" — a different Nextcloud is a different DOM.What this PR still does, and why it is still worth landing
Replaces
waitForTimeout(2500)after adomcontentloadedgoto with an explicit wait for a Vue-rendered node, and adds an assertion that fails on the measurement, naming it, when the chrome never mounts.The reason is the failure mode, not the count. Before this, a run that probed before mount and a run that probed a genuinely stale stylesheet produced the same output — and that output invites the reader to "fix them against the real DOM", which, acted on, means deleting theme CSS to satisfy a measurement. Those two cases are now distinguishable.
It also produced new, true information on its first run:
Three of the six surveyed surfaces contribute nothing, and nobody knew.
What is still open
What the remaining 31 actually are. This PR does not answer it. The likeliest reading, explicitly unverified: CI's minimal instance does not render the header and navigation chrome these rules target at all — which would make this a fixture problem (the survey's instance is not representative), not dead CSS and not timing.
The four I can characterise:
.guest-box.login-box/login, whichSURFACESdoes not includebody#body-publicbody[data-theme-dark]/body[data-themes*="dark"]No
ALLOWEDentry is added for any of them — an allowlist entry would hide the gap rather than close it, and I would be hiding a gap I have not diagnosed.This PR does not make the E2E job green, and the E2E job was already red on
developmentfor this same test.