Skip to content

fix(e2e): resolve the 29 'dead' lasuite selectors — 2 real defects, 27 a blind survey - #259

Merged
rubenvdlinde merged 3 commits into
developmentfrom
fix/e2e-playwright
Aug 10, 2026
Merged

fix(e2e): resolve the 29 'dead' lasuite selectors — 2 real defects, 27 a blind survey#259
rubenvdlinde merged 3 commits into
developmentfrom
fix/e2e-playwright

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What was red

development's Code Quality push run 31381545314 failed one real job — E2E Tests (Playwright) — plus Quality Report, a pure aggregator red only because E2E was. 27 jobs, 110 tests passed, 1 failed:

selector-liveness.spec.ts › every element-overrides selector matches something on at least one surface, reporting 29 selectors in css/systems/lasuite/element-overrides.css as matching nothing anywhere.

That message invites the reader to "fix them against the real DOM" — which, acted on, means deleting theme CSS to satisfy a measurement. The previous round already recorded in the file that a timing fix cleared none of them and that the honest reading was still open. So the first job was to find out which of the 29 are really dead.

Measured, not reasoned about

Two disposable Nextcloud instances (32.0.12 and 34.0.2, official images, lasuite active, nldesign installed) probed with document.querySelectorAll for every one of the 29. Every claim below is a count from a live DOM.

# selectors verdict
10 #app-navigation-vue .app-navigation-entry.active… and the .active.active variants live on 32 and 34 (count 1 each) — survey probed too early
1 #header .notifications-button__icon svg live on 32 and 34 — but its app is absent from CI's fixture
12 .app-menu__waffle, .app-menu__current-app-*, .unified-search-input*, .header-start … live on 34, absent on 32 — NC33+ markup
1 .guest-box.login-box live on /login — a surface the survey never visits
1 body#body-public live on a public share page — likewise
2 body[data-theme-dark], body[data-themes*="dark"] live with dark theme on — a state the survey never sets
2 .navigation-list__link.active, …::before genuinely dead on both versions

So 2 of 29 were a product defect. The other 27 were the survey judging a stylesheet it could not see.

The product fix

.navigation-list__link.active is deleted. Opened on the settings dialog on both 32 and 34, nc-vue emits navigation-list__link navigation-list__link--active (1 node) and navigation-list__link (5 nodes) — never the bare .active. The block carried both spellings on the general principle that this file had been bitten four times by naming one of two live spellings; sound principle, wrong application — that component has only ever had one spelling. --active, which is observed, stays.

The measurement fixes

Wait for the item, not the container. The survey waited on CHROME_READY (proves only the header app mounted) and then waitForSelector('#app-navigation-vue').catch(() => {}) — a container check whose failure was swallowed. The nav is a different bundle on a different clock, and #app-navigation-vue is attached long before the router marks an entry .active. The CI failure list corroborates this exactly: the bare .app-navigation-entry selector was not reported dead, only the .active variants — the entries had rendered, the active state had not. It now waits for #app-navigation-vue .app-navigation-entry.active itself, and an anchor that never arrives is a named, fatal measurement failure saying "do not fix the CSS on this".

Three surfaces the union was missing/login, a public link share, and a dark-theme pass over Files. The share is minted in beforeAll and dropped in afterAll; dark goes on immediately before its surface and off immediately after.

The anonymous surfaces are genuinely anonymous. use.storageState is inherited by every context created inside a test, so a plain browser.newContext() would have carried admin's cookies, /login would have redirected to the dashboard, and the survey would have measured an app page while reporting it had measured the login screen — green, about the wrong page. Contexts are built with storageState: undefined, and the login surface asserts document.body.id === 'body-login' so a regression cannot pass unnoticed.

Two deferrals that expire by themselves, kept separate from ALLOWED precisely because an allowance does not expire:

  • SINCE — markup a newer Nextcloud renders. Deferred only while the surveyed major is below since, and an explicit assertion refuses to defer anything once the survey reaches the app's declared max-version (34). Raising nextcloud-test-refs therefore cannot silently carry the exemption forward.
  • REQUIRES_APP — markup owned by an app the fixture does not install. notifications-button__icon appears nowhere in a Nextcloud release except apps/notifications/, which lives in its own repository and is absent from the nextcloud/server checkout CI builds from. The check is against OC.appswebroots, the instance's own list of enabled apps, so installing the app puts the selector straight back under the main assertion.

Counts are printed. Nextcloud major, distinct selectors parsed, selectors proven live, per-surface live/probed, deferrals, unreachable and chromeless surfaces. There are hard floors on "selectors parsed" and "selectors proven live" so a survey that measured empty pages cannot report PASS — this is the repo where thirteen a11y gates once passed planted true positives because every one of them globbed src/**/*.vue and nldesign ships zero.

Both directions, one mutation at a time

Each mutation applied alone and reverted, against Nextcloud 32 — the version CI runs.

mutation result
put .navigation-list__link.active back RED — names that selector
drop the login surface RED.guest-box.login-box
drop the public-share surface REDbody#body-public
drop the files-dark surface RED — both dark selectors
enable notifications, add a dead notifications-button__icon-* selector RED — the app deferral does not swallow a genuinely dead selector once the app is present
let the anonymous context inherit the admin session RED — the body-login positive control
revert to the old container-only wait PASS — reported honestly below

Unmutated, on a CI-shaped fixture (notifications disabled): 2 passed, 179 selectors parsed, 123 proven live, 13 deferred and named, 0 unexplained.

Run on Nextcloud 34 the suite goes red on three different selectors — the NC32-only header classes — which is the mirror image working: the guard catches whatever is dead on the version it is actually looking at.

What I could not prove

The container-only wait mutation still passes locally. Reverting the readiness fix does not turn my local run red, because this box wins the race the CI runner loses. So the timing fix is justified by the CI evidence (bare .app-navigation-entry live, .active dead — the signature of probing mid-render) and by the failure mode it removes, not by a local mutation. If CI still reports those eleven dead, the new assertion now says so as a measurement failure and names the anchor, instead of accusing the stylesheet.

Left alone deliberately

The La Suite header treatment does not reach Nextcloud 32 at all — the twelve deferred selectors have no NC32 counterparts in this file, so on the version CI runs the header is unstyled by this theme. That is a real gap, it is recorded in the SINCE comment, and it wants its own change rather than being smuggled into a red-to-green fix.

…d survey

The selector-liveness sweep reported 29 selectors in element-overrides.css as
matching nothing. Probed against real Nextcloud 32.0.12 and 34.0.2 instances
with lasuite active, only 2 are dead:

  .navigation-list__link.active (+ ::before) — nc-vue's settings-dialog
  navigation emits only the BEM --active spelling, on both versions. Deleted.

The other 27 are the survey judging a stylesheet it could not see:

  11 live on both versions but probed before the nav/bell bundles rendered —
     the sweep waited on the header, then on the nav CONTAINER with its
     failure swallowed. It now waits for the ITEM, and an anchor that never
     arrives is a named, fatal MEASUREMENT failure.
  12 NC33+ header markup, absent on the stable32 CI surveys — deferred via
     SINCE, which refuses to defer anything once the survey reaches the app's
     declared max-version.
   1 owned by the notifications app, which is not in CI's fixture — deferred
     via REQUIRES_APP, checked against OC.appswebroots so installing the app
     re-arms the assertion.
   3 live only on /login, a public share page, and under dark theme — three
     surfaces the union was missing, now surveyed.

Anonymous surfaces build their context with storageState: undefined and the
login surface asserts its own body id, so an inherited admin session cannot
pass unnoticed. Counts are printed and floored so a survey over empty pages
cannot report PASS.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/nldesign @ d9c8fb2

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-10 15:39 UTC

Download the full PDF report from the workflow artifacts.

storageState: undefined is not an empty session. Playwright merges context
options over the config's use block and reads undefined as 'not specified',
so the root use.storageState (tests/e2e/.auth/admin.json) was inherited and
the 'anonymous' context browsed as admin. CI proved it: /login came back with
body id body-user.

It was caught only because the login surface asserts its own body id — the
selector verdict itself would have been green, since token gating does not
depend on the caller. Replaced with an explicit { cookies: [], origins: [] },
which cannot be merged away.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/nldesign @ b63d991

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-10 16:41 UTC

Download the full PDF report from the workflow artifacts.

…enders that state

CI's instance is a nextcloud/server stable32 checkout served by php -S. Its
Files navigation renders its entries — the bare .app-navigation-entry and
:hover rules are live there — but no entry is ever marked .active, on any
surface, even after waiting 60s. A released Nextcloud has one: probed at rest
on 32.0.12 and 34.0.2, #app-navigation-vue .app-navigation-entry.active
matches exactly 1 node.

So the ten active-row selectors are correct CSS this fixture cannot exercise.
REQUIRES_RENDERED defers them on that measured fact and nothing else: the
anchor is a selector the survey actively waits for and records, so the day the
navigation marks an active entry they are required live again. Proven in both
directions — with the anchor rendering, a planted dead nav-active selector
still fails the run.

A missing anchor is no longer fatal in itself; a missing PAGE still is, via
chromeless and the live-count floor.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/nldesign @ a0f591d

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-10 17:09 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 73cbb57 into development Aug 10, 2026
30 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/e2e-playwright branch August 10, 2026 17:10
@rubenvdlinde
rubenvdlinde restored the fix/e2e-playwright branch August 19, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant