test(e2e): gate-19 round 2 — 176 → 151, twelve real tests each with a control - #92
Merged
Conversation
…, and the header-block tags moved onto the test that proves them
Closes 14 gate-19 findings; gate-19: 176 -> 162 (measured, unscoped, both
numbers observed on this tree).
NEW TEST — "a logged-in owner opening the share link gets the same
read-only page as a stranger" (tests/e2e/ci/public-share.spec.ts).
REQ-PSHR-006's fourth scenario is the only one of its four a browser can
decide, and it is the one with a real failure mode: the public view
noticing a session and upgrading to the editable workspace, so that a
link pasted into a group chat becomes an edit surface for every
colleague who is logged in. The test opens the share as the dashboard
OWNER — the caller with the most permission available — and requires
`.public-share-view` plus the read-only badge AND zero
`.launchpad-sidebar-toggle` / `.launchpad-grid-item`. The negative half
matters: asserting the badge alone would still pass if the editable
shell rendered around it.
It carries a CONTROL. Before opening the link it loads /apps/launchpad
and requires the workspace shell to be visible, which proves the shared
admin session is live. Without that, an expired session would make every
"no edit affordance" assertion pass for the wrong reason — read-only
because the visitor is anonymous, not because the view refuses to
upgrade.
THE THREE SIBLING SCENARIOS ARE EXCLUDED, AND THE REASON IS STRUCTURAL.
"Cannot create widget / edit / delete via public share" are guarded by
PublicShareContext, a REQUEST-scoped PHP marker set exactly once, inside
PublicShareController::renderShare(). No route mutates anything under
/s/{token}, so no request a browser can issue is both a public-share
bearer and a mutation: an anonymous PUT /api/dashboard/{uuid} is refused
as unauthenticated, and a test asserting 403 there would pass while
proving something else entirely. Recorded alongside: the endpoint the
first of those scenarios names, POST /api/dashboards/{uuid}/placements,
does not exist in appinfo/routes.php.
Remaining exclusions, all with claims that were checked before they were
written:
* REQ-PSHR-007 (3) — viewCount/lastViewedAt are DB columns no
anonymous response echoes back, plus distinct IPs or 60s+ waits.
* REQ-PSHR-009 (3) + the PSHR-005 throttle scenario (1) — the
IThrottler bucket is IP-global by the spec's own wording, and
playwright.config.ts runs workers:1 from one runner IP, so spending
it hands 429 to every later test. PublicShareControllerTest::
testUnlockReturns429WhenThrottled was opened and confirmed.
* REQ-PSHR-010 (2) + the PSHR-004 groupfolder scenario (1) —
GroupFolders is not installed on the fixture; code-quality.yml
`additional-apps` provisions only openregister and neither seed
script adds it.
HEADER-BLOCK TAGS MOVED (.github#343). public-share.spec.ts listed four
@e2e slugs above every test(). gate-19 resolves a tag's owner with
_TestDoc.owner(), which for a tag above all tests returns simply the
FIRST test in the file — so all four were credited to one body whatever
that body asserted, and a tag for a scenario nobody had tested would
have counted the same. These four happen to be genuinely proven by the
test they now sit on, but that was luck of authorship, not something the
gate checked. Every tag on this branch sits directly above its own
test().
--list: 77 tests in 19 files -> 78 in 19.
…d user CI never had Closes 11 gate-19 findings, every one of them with a test. gate-19: 162 -> 151 (measured on this tree). WHY THESE WERE OPEN. tests/e2e/dashboard-sharing.spec.ts drives the share sidebar and needs a pre-seeded `recipient` account (LAUNCHPAD_E2E_SHAREE) that tests/e2e/seed.sh does not create — it makes `e2e-grantee` and nothing else — so all four of its tests fail in the CI job and playwright.config.ts excludes the file. gate-19 counts nothing from a file no project runs, so eleven REQ-SHARE scenarios had no proof. Its four @e2e slugs would not have closed them anyway: it writes `owner-adds-a-user-share` where the spec heading slugifies to `owner-adds-a-share`. The new file does not fight that. It PROVISIONS what it needs — three throwaway accounts and a group, through the same OCS provisioning API tests/e2e/fixtures/secondary-user.ts uses — and asserts the sharing contract at the HTTP layer, then removes them in afterAll. The sidebar spec still owns the UI and is untouched. HTTP AND NOT DOM, DELIBERATELY. Every scenario in REQ-SHARE-001/002/ 004/006/009 is about what a SECOND user may see or do. The sidebar renders for the owner; the recipient's half of each scenario has no owner-visible UI, so a DOM assertion could only prove the half already covered. Same reasoning ci/public-share-lifecycle.spec.ts records, and the same conventions: `Authorization: Basic` rather than Playwright's reactive httpCredentials, and `storageState: undefined` on every context so no request is silently served as the admin from global-setup. EVERY TEST CARRIES A CONTROL, because most of these assertions are satisfiable by a build with no check at all: * owner-adds-a-share — the share list is asserted EMPTY first. * recipient-cannot-manage-shares — bob is given `full`, the most permissive level, and proven able to READ the dashboard before his POST is required to 403. * recipient-sees-a-shared-dashboard — bob is proven NOT to see it before the share exists, so an endpoint returning every dashboard on the instance cannot pass. * group-share-grants-visibility — dave, a non-member, is required NOT to gain visibility. * most-permissive-level-wins — carol is proven `view_only` with only the direct share, so the final `full` must come from the group. * owner-has-viewonly-recipient-has-full — the widget add is required to be REFUSED at view_only before it is required to succeed at full. Without that arm the success assertion would pass on a build with no permission check. * search-returns-matching-users-and-groups — a query matching nothing must return nothing, or the search is not filtering. * non-owner-is-denied / idempotent-re-PUT — the row set is compared before and after, ids and timestamps included. TWO PLACES WHERE THE SPEC PROSE NAMES A PATH THE APP DOES NOT SERVE, recorded in the file header rather than worked around silently: 1. REQ-SHARE-002 says the recipient's dashboards come from GET /api/dashboards. That route is dashboardApi#list -> getUserDashboards() -> findByUserId(), the caller's OWN rows only. The union that folds in shares is GET /api/dashboards/visible (getVisibleToUser() -> findSharedDashboards(), under a comment naming REQ-SHARE-002). Behaviour implemented, path stale. 2. REQ-SHARE-002 names a field `effectivePermissionLevel`. No response carries that key; /visible carries `source` + `isOwner` and the resolved level comes from GET /api/dashboard/{id} as `permissionLevel` with `isOwner` and `sharedBy`. The tests assert the keys the app actually sends. The idempotent-re-PUT scenario has two clauses. "No rows change" is asserted here, exactly. "No notifications published" is not observable from a browser, which is why REQ-SHARE-008 already carries its own @e2e exclude; the test says so where it sits. NEGATIVE CONTROL, run before this commit: removing this file whole takes gate-19 from 151 back to 162 and dashboard-sharing reappears at exactly 11. Restoring it returns 151. The gate is reading the tests, not the annotations. --list: 78 tests in 19 files -> 89 in 20.
… undefined
`storageState: undefined` and `storageState: { cookies: [], origins: [] }` are
not equivalent. Option merging treats an explicit `undefined` as "not
supplied", which falls back to the project default — and the project default
here is exactly the thing being guarded against: playwright.config.ts sets a
top-level `use.storageState` pointing at global-setup's admin session.
So the previous spelling risked being a no-op against the very inheritance it
documents, and the consequence is silent: a valid session cookie outranks a
later Authorization header, so every "bob is refused" assertion would have been
an assertion about an administrator. The measurement already in the file header
(run 31389746411, a non-admin create answering 201 with "createdBy":"admin")
is what that failure looks like.
The empty-jar literal cannot be read as "not supplied".
The same fix, in the same form, closed an identical false green in pipelinq,
where a context created to be anonymous read back ocs.data.id === "admin" — and
was only caught because that suite's identity guard had been changed to read
the resolved uid instead of an HTTP status.
No test behaviour changes; 3 call sites, one comment.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn and
remko48
as code owners
August 11, 2026 09:42
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 09:56 UTC
Download the full PDF report from the workflow artifacts.
…the share check — and the control was green for that same wrong reason
CI on this branch failed one test: "a full-level recipient may add a
widget to a dashboard whose own level is view_only", with
`{"error":"Access denied"}`. That body was read as canAddWidget()
refusing. It is not attributable that way.
`WidgetApiController::denyAddWidget()` has TWO independent 403 branches
and both return `ResponseHelper::forbidden()` — the byte-identical body
`{"error":"Access denied"}`:
1. PermissionService::canAddWidget() — the share check, which
is all REQ-SHARE-004
is about
2. RoleFeaturePermissionService::isWidgetAllowed() — the role-feature
widget allow-list,
about something else
The body cannot distinguish them, so the diagnosis had to come from the
fixture. tests/e2e/fixtures/role-feature-permissions.ts installs a
RESTRICTIVE `default` row — allowedWidgets: ['activity',
'recommendations'] — and TEN widget-adding specs call it in beforeAll.
playwright.config.ts runs workers:1, fullyParallel:false against one
instance, so that row was in force here whether this file asked for it
or not. isWidgetAllowed() short-circuits for Nextcloud admins; bob is a
plain provisioned account, so branch 2 refused `label` outright. That is
the failure, and it is a test-fixture collision, not a product bug.
THE PART THAT MATTERS MORE THAN THE FIX: the view_only CONTROL arm
PASSED THE WHOLE TIME. It expects 403 and branch 2 supplied one. So the
control was green while proving nothing whatever about share
permissions — a check that could not have failed for the reason it
names. It only surfaced because the arm NEXT to it could still fail.
Two changes remove the ambiguity instead of working around it:
* this file now calls ensureDefaultWidgetRestriction() in its own
beforeAll, so the allow-list state is KNOWN rather than inherited
from whichever spec happened to run first in the job;
* both arms now add `activity`, which that restriction ALLOWS, so
branch 2 is constant across them and the share level is the only
variable left. The refusal at view_only is attributable to the share
check precisely because the SAME widget id succeeds for the SAME
user at `full` — that arm is what rules branch 2 out.
Also added the splitting probe: after the upgrade POST the share row is
asserted to read `full` BEFORE the add is attempted, so any future
failure names its own cause — a row still at `view_only` means the
upgrade did not take, a row at `full` under a refused add means the
per-share level is ignored downstream.
Neither assertion was weakened and the control arm is unchanged in
strength; it is now merely able to fail for the right reason.
Also folds in the correction to the empty cookie jar: `storageState:
undefined` does NOT clear an inherited jar — option merging reads an
explicit `undefined` as "not supplied" and falls back to the project
default, which is the very admin session it was meant to drop. The
contexts spell it `{ cookies: [], origins: [] }`, and the stale header
line that still described the old form is corrected.
gate-19 unchanged at 151. --list unchanged at 89 tests in 20 files.
This was referenced Aug 11, 2026
…RCH-001..003
Closes 9 gate-19 findings, all with tests. gate-19: 151 -> 142.
A browser is the only place these can be proven. Every claim in these
three requirements is about the DOM or the keyboard: which element holds
focus after `/`, whether the browser's own Ctrl+K was prevented, whether
a non-matching tile is DIMMED rather than removed from the grid, whether
aria-activedescendant tracks the arrow keys. A component test cannot
reach the interesting half either, because that half is the bridge
between RuntimeShellSearch.vue (the combobox) and Views.vue (the grid) —
WorkspaceApp.vue joins them with a plain DOM query, by its own comment,
"because the grid DOM lives in a sibling component's tree".
The file seeds its own dashboard rather than guessing at the shared
fixture, which other specs in this serial job are free to change. Four
tiles are created through POST /api/dashboard/{id}/tile with titles
chosen to make the spec's ranking rule decidable — "Verlof aanvragen"
(prefix) against "Overzicht verlof" (mid-string) — and a per-run stamp
so a re-run against a warm instance cannot match a leftover tile.
Selectors are the component's own `data-test` hooks, not CSS classes.
THE ASSERTIONS THAT DISTINGUISH A REAL IMPLEMENTATION FROM A PLAUSIBLE
ONE, since most of these scenarios are satisfiable by something weaker:
* slash-focuses-the-bar also requires the input to be EMPTY. A handler
that focuses without preventing the default leaves a stray "/" in
the field, and the focus assertion alone would not see it.
* ctrlk-focuses-the-bar installs a listener and reads
`event.defaultPrevented` on the very event the app acted on.
Asserting focus alone passes on a handler that lets the browser's
own Ctrl+K fire as well.
* typing-filters-tiles-by-label counts the GRID before and after and
requires it unchanged. A filter implemented by unmounting cells
would satisfy every other assertion in that test. It also records
every /apps/launchpad/ request fired while typing and requires
none — the spec says the filter is entirely client-side.
* no-query-stored types a unique probe string and requires it in no
request URL, no request BODY, and no localStorage/sessionStorage
key or value. It waits for the result list to settle first, so it
cannot pass on a store that merely had not flushed yet.
* arrow-keys checks that the option named by aria-activedescendant is
the one carrying aria-selected="true", and that exactly ONE
non-colour marker is un-hidden — colour alone would leave zero,
which is the WCAG clause the scenario states.
* enter-opens-the-selected-tile records window.open and aborts the
route, so it asserts which target the app ASKS for rather than
depending on a page load — the technique image-widget.spec.ts
already uses for REQ-IMG-003.
Three tests carry an explicit CONTROL that focus/dimming is not already
in the asserted state before the key is pressed, so each assertion is a
change rather than a description of the resting state.
The four REQ-QSEARCH-004 fallback scenarios are NOT closed here. They
need `quicksearch_fallback_target` changed instance-wide and the shell
reloaded, and that setting is read from initial state; they are left
open rather than annotated.
--list: 89 tests in 20 files -> 98 in 21.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 10:07 UTC
Download the full PDF report from the workflow artifacts.
…ils SILENTLY
Self-review of the tile-quick-search file before trusting its CI result.
The de-emphasis class was written as
`.is-search-dimmed, .search-dimmed, [data-search-dimmed="true"]` — three
guesses, none of them real. The class
`WorkspaceApp.vue::applySearchDimming()` actually toggles is
`launchpad-grid-item--dimmed`.
A wrong selector here does not fail loudly, which is the reason this is
worth its own commit. It matches nothing, so it counts zero dimmed tiles
before Escape and zero after — and the Escape test was written as
`if (dimmedBefore > 0) { expect(...).toBe(0) }`, so the assertion was
skipped entirely. It would have gone green on every run, including on a
build where the undimming was broken or absent. A check that cannot fail
is indistinguishable from one that passed.
Fixed properly rather than patched:
* the class is named once, in a `DIMMED` constant, with a comment
saying why a wrong value is silent;
* the Escape test now REQUIRES a non-zero dim count before pressing
Escape, instead of tolerating zero. If the selector is ever wrong
again, that control fails loudly instead of skipping;
* the filter test gained the other half of its scenario — it already
proved the grid keeps every tile, and now also requires the
non-matching ones to actually carry the class, so "de-emphasised
rather than removed" is proven in both directions rather than one.
Also replaced the speculative `.launchpad-grid-item, .grid-stack-item`
pair with the real `.launchpad-grid-item` (Views.vue:143, and the
selector the app's own DOM bridge queries).
gate-19 unchanged at 142. --list unchanged at 98 tests in 21 files.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 10:11 UTC
Download the full PDF report from the workflow artifacts.
… never calls it Second self-review finding on the tile-quick-search file, and unlike the dimming selector this one would have failed CI loudly rather than silently — which is the only reason it is the less dangerous of the two. I wrote the activation test around recording `window.open`, by analogy with image-widget.spec.ts's REQ-IMG-003. The analogy does not hold. `WorkspaceApp.vue::activateSearchResult()` does not open a window: it finds the rendered cell by `data-placement-id`, scrolls it into view, and calls `.click()` on the cell's own `a[href]`. `__opened` would have stayed empty and the poll would have timed out. Rewritten around the mechanism the app actually uses. A capture-phase `click` listener records the clicked anchor's `href` and `target` and calls preventDefault, so the assertion is about the request the app makes and never touches the network — the seeded links point at the reserved `.invalid` TLD and must not be dialled. This also let the test grow the half of the scenario it was missing. "Enter opens the selected tile, honouring its configured link target (same-tab / new-tab)" has two clauses, and the href assertion only covers the first. `TileWidget.vue` renders `:target="tile.linkType === 'url' ? '_blank' : '_self'"`, the tile is seeded `linkType: 'url'`, so the test now also requires `_blank`. A regression that dropped the target — opening an external link inside the dashboard's own tab — would have passed on href alone. Both defects came from writing against a spec and a plausible mental model instead of against the source. The dimming one was found by checking a selector I had guessed; this one by reading the function the scenario names. Neither was found by the type checker or by `--list`, and the first could not have been found by CI at all. gate-19 unchanged at 142. --list unchanged at 98 tests in 21 files.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ❌ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 10:14 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 10:29 UTC
Download the full PDF report from the workflow artifacts.
… one click that left the page CI on 2ff919a: 7 failed, 94 passed — every failure in this one file. The 11 dashboard-sharing tests and the public-share test are green, so the rest of the branch stands. Two causes, not seven. CAUSE 1 — THE CLICK NAVIGATED AWAY (tests at :220 and :242). Both failed on their CONTROL with "element(s) not found" on the search input, which reads like a focus problem and is not one: the input was not on the page any more. `page.locator('body').click({ position: {x:5, y:5} })` measures from the BODY's box, whose origin is the top-left of the Nextcloud chrome, so the click landed on core's own header and left the app. Replaced with a `blurToGrid()` helper that focuses `.workspace-shell__grid` — the shell's own container, `tabindex="-1"` per WorkspaceApp.vue, and exactly where `focusGrid()` puts focus after Escape. In-page, representative of a real resting state, and not a typing target, which matters because `isSlashFocusShortcut()` ignores `/` while focus is in a text field. Worth recording: the reading that this was autofocus — that the bar already held focus, making the scenario unobservable as a state change — was offered and is wrong. The error was "element(s) not found", not "element is focused". The control was correct and was reporting accurately; only its setup step was broken. CAUSE 2 — THE WRONG DASHBOARD WAS ACTIVE (the other five). All five failed on an EMPTY result list. The seeding is sound — `PlacementService::addTileFromArray()` -> `applyTileConfig()` sets `tileType='custom'` and `tileTitle=$tileData['title']`, which is exactly what `WorkspaceApp.vue::tileSearchLabel()` searches — so by elimination the tiles being searched were not this suite's. There are two activation mechanisms and they are not interchangeable. `POST /api/dashboard/{id}/activate` sets the legacy id-based `is_active` column; `POST /api/dashboards/active` sets a per-user UUID PREFERENCE, and `getActive()` resolves the shell's dashboard through THAT — its own comment says the active dashboard "can now be a group/default (showcase) dashboard the user does not own (resolved via the last-used preference)". Only the first was being called, so a preference left behind by an earlier spec in the same serial job won. Both are now set. AND THE GATE THAT SHOULD HAVE CAUGHT IT. `openWorkspace()` required only `GRID_ITEM count >= TILES.length`. A dashboard carrying the default widget bundle clears that bar easily, so the wrong dashboard sailed through and five tests failed three steps downstream on a symptom that named nothing. The gate now requires THIS suite's own tiles, matched by the per-run stamp, and says in its message that a failure there means the shell resolved a different dashboard and nothing below is meaningful. That is the same defect shape as the two found by self-review before CI: a check whose passing did not depend on the thing it was there to establish. Also dropped an unused `STATUS` selector constant. gate-19 unchanged at 142. --list unchanged at 98 tests in 21 files.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 10:47 UTC
Download the full PDF report from the workflow artifacts.
…hich link broke CI on ea11d48: 2 failed, 99 passed — down from 7. Both remaining failures are in tile-quick-search. FAILURE 1 — `ReferenceError: CSS is not defined`, and it never reached an assertion, so it says nothing about the app. `CSS.escape` appears at two sites in this file and they run in DIFFERENT contexts: * inside `page.evaluate()` — BROWSER, where `CSS` exists. Correct, and left alone. * in the Node test body, building `page.locator('#' + CSS.escape(id))` — where `CSS` does not exist at all. Replaced with `page.locator('[id="…"]')`. An attribute selector needs no escaping and is exact, so this removes the dependency rather than reaching for a Node-side escaper. The consequence worth stating: because this test died before asserting, THE SELECTION MECHANISM WAS UNVERIFIED on that run — and the activation test below depends on the selection being sound. Fixing this one first is not just tidying; it is the prerequisite for reading the other failure at all. FAILURE 2 — "Enter must activate the selected tile", an empty activation list. That message names none of the four links in the chain that could have produced it, which is why the fix here is not another harness rewrite. Two splitting probes now sit before the Enter press: * PROBE 1 asserts the rendered cells carry an `a[href]`, found exactly the way `activateSearchResult()` finds them. That function clicks the cell's anchor and FALLS BACK to `el.focus()` when the anchor is absent — in which case no click is ever dispatched and no harness could record one. If this probe fails, the finding is about tiles seeded through `POST /api/dashboard/{id}/tile` not rendering a link, which is a product observation and will be filed as one rather than absorbed into the test. * PROBE 2 asserts the input still holds focus when Enter is pressed. The assertion itself is unchanged and unweakened — it still requires the tile's own href and its `_blank` target. Only the diagnosis is better: a red run now points at a link in the chain instead of at "nothing happened". gate-19 unchanged at 142. --list unchanged at 98 tests in 21 files.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 11:04 UTC
Download the full PDF report from the workflow artifacts.
…ding left OPEN, dashboards now cleaned up CI on cc5718f: 1 failed, 100 passed. The last failure is not a test defect. WHAT THE SPLITTING PROBES SETTLED. Both PASSED: the rendered cells do carry `a[href]`, and the input does still hold focus when Enter is pressed. The sibling test proving `aria-activedescendant` tracking now passes too, so the selection is sound. Anchor present, focus correct, selection correct, and still no click. `WorkspaceApp.vue::activateSearchResult()` explains it exactly: const placementId = item?.placement?.id // an INTEGER … placementId.replace(/"/g, '\\"') … // TypeError, always `Number.prototype.replace` does not exist. The truthiness guard above it does not help, and the throw is inside a Vue event handler, so nothing surfaces: Enter silently does nothing. A COMPETING HYPOTHESIS WAS OFFERED AND IS REFUTED BY EVIDENCE ALREADY IN HAND — that `this.$el.querySelector(...)` returns null because the grid lives in a sibling component's tree, making the method a silent no-op. It cannot be that. `applySearchDimming()` runs the SAME `this.$el.querySelectorAll('.launchpad-grid-item[data-placement-id]')` query, and the filtering test asserts `dimmed > 0` and PASSES. If `this.$el` could not see the grid, `items` would be empty, no class would be toggled, and that assertion would fail. The scope is fine; the crash happens before `querySelector` is even called, while building its argument. NO `@e2e exclude` IS ADDED. The scenario is browser-observable — the reason it has no passing test is that the feature is broken. An exclusion would record "a browser cannot see this", which is false. The finding stays OPEN against #95, which is the standing rule for a scenario that fails because the product is wrong rather than because a browser cannot reach it. gate-19 therefore goes 142 -> 143, and that increase is deliberate. A SECOND DEFECT FROM THE SAME ROOT, also filed in #95: the dimming assertion requires only `dimmed > 0`, and that is satisfied by "EVERY tile is dimmed" — which is what the app does, because `applySearchDimming()` compares a STRING `getAttribute(...)` against NUMERIC ids with `Array.includes`, so no tile ever matches and the matches get dimmed along with everything else. My own assertion therefore passes for the wrong reason. Tightening it to "matches are NOT dimmed" belongs with the fix, since it would be red on current `development`; it is recorded in the file and in the issue rather than quietly left. ALSO — TEARDOWN. dashboard-share-api.spec.ts created ten dashboards and deleted none, leaving them in a register every later spec reads, and `POST /api/dashboard` also moves the caller's ACTIVE dashboard. That is not hypothetical: tile-quick-search failed earlier on this very branch because a dashboard it did not create was active. Ids are now recorded inside `createDashboard()` at the moment of creation — not by the caller afterwards — so a test that throws still gets its dashboard removed, and `afterAll` deletes them tolerantly before the accounts. --list: 98 tests in 21 files -> 97 in 21. gate-19: 143 (measured).
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| composer | ✅ | ✅ 102/102 | |||
| npm | ✅ | ✅ 548/548 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 11:22 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.
gate-19 (e2e-coverage) round 2 — launchpad
Measured, unscoped, on this tree: 176 → 143. Round 1 left 176; this branch closes 33. CI is green — Playwright 100 passed, 0 failed (run 31485185999), whole Code Quality run
success.Baseline re-measured independently before any edit (
check_e2e_coverage.py .from the app dir, the same invocation the orchestrator makes unscoped) and confirmed at 176 — not taken on trust from the handover.What closed, and how
dashboard-public-sharedashboard-sharingtile-quick-search20 of 33 closed with a test that runs in CI. Playwright
--list: 77 tests in 19 files → 97 in 21; CI executed 100 passed (--listcounts test declarations, the runner counts the same suite — the difference is the threeci/specs' parameterised cases).Exclusion reason families (13, all
dashboard-public-share)Every claim was checked before it was written; every named PHPUnit method was opened and confirmed to exist.
PublicShareContextis a request-scoped PHP marker set exactly once, insidePublicShareController::renderShare(). No route mutates anything under/s/{token}, so no request is both a public-share bearer and a mutation; an anonymousPUT /api/dashboard/{uuid}is refused as unauthenticated, and a test asserting 403 there would pass while proving something else. Recorded alongside: the endpoint one of those scenarios names,POST /api/dashboards/{uuid}/placements, does not exist inappinfo/routes.php.IThrottlerbucket is IP-global by the spec's own wording;playwright.config.tsrunsworkers: 1from one runner IP, so exhausting it hands 429 to every later test.PublicShareControllerTest::testUnlockReturns429WhenThrottledcovers the path.code-quality.yml'sadditional-appsprovisions onlyConductionNL/openregister, and neither seed script adds it.viewCount/lastViewedAtare DB columns; the scenarios additionally need distinct client IPs or 60s+ waits against a 60s test timeout.Left open and filed, NOT excluded — 25 findings
@e2e excluderecords "a browser cannot observe this scenario". That is not what is true in these two cases; what is true is that the feature is not wired. Excluding them would have bought 24 findings with a false statement.dashboard-kiosk-mode(23 scenarios). The kiosk frontend does not exist.grep -rli kiosk src/ templates/ css/returns onlysrc/stores/kioskPlaylists.js— which has zero importers — and its own unit test. No.vue, no template, no CSS.KioskController::render()returns aDataResponse, not a page. The UI was removed, not never-built:888b121cshipped it,1fd075c5deleted it as orphaned, and the spec was never updated. The backend is still fully routed and live.tile-quick-search::enter-opens-the-selected-tile(1 scenario). Pressing Enter on a match never opens the tile.WorkspaceApp.vue::activateSearchResult()callsplacementId.replace(...)on an integer placement id —Number.prototype.replacedoes not exist, so it throws aTypeErrorinside a Vue event handler and Enter silently does nothing. Isolated by splitting probes that both passed: the cells do carrya[href], and the input does hold focus at Enter; the sibling selection test passes too. A competing hypothesis — thatthis.$el.querySelector(...)is mis-scoped because the grid lives in a sibling tree — is refuted by evidence already in hand:applySearchDimming()runs the samethis.$elquery and the filtering test'sdimmed > 0assertion passes, which it could not ifthis.$elsaw no grid items. Same issue records a second defect from the same root: dimming compares a stringgetAttributeagainst numeric ids withArray.includes, so every tile is dimmed including the matches — which is also why this branch's owndimmed > 0assertion passes for the wrong reason, recorded in the file rather than quietly left.runtime-shellREQ-SHELL-003 (1 scenario). Describes a Save Layout button in a toolbar removed in18d601a0;WorkspaceApp.vue:46records the decision where the markup was. Worth a second look: the requirement's other two scenarios already carry@e2e exclude, both phrased as "covered by Vitest/Newman", neither mentioning that the control they describe is gone. The gate was satisfied without anyone noticing the requirement had been overtaken.Negative control — the gate is reading tests, not annotations
Removing
tests/e2e/ci/dashboard-share-api.spec.tswhole and re-measuring:+11, exactly the number of scenarios that file anchors, all in the right capability.
Does Playwright actually run in CI here? Yes
code-quality.ymlsetsenable-playwright: truewithplaywright-test-path: tests/e2e, which holds no config of its own, so the workflow falls back to the rootplaywright.config.ts— the same file gate-19 reads. Round 1 established that; it still holds. Everything added here therefore has to pass, and the run list is the proof.Round 1's measured CI run: 77 passed in 8.0m against a
globalTimeoutof 38m, so there is headroom for the 21 added tests.Note the Newman job has no frontend-build step, so nothing served by a built bundle is observable there — which is why the DOM-level work in this PR is Playwright-only.
Two traps worth carrying to the next repo
1.
storageState: undefineddoes not clear an inherited cookie jar. Playwright's option merging reads an explicitundefinedas "not supplied" and falls back to the project default — hereuse.storageState, the admin session fromglobal-setup.ts. So a context written to be anonymous is served as admin, and every "bob is refused" assertion becomes an assertion about an administrator. The correct spelling is{ cookies: [], origins: [] }.tests/e2e/ci/dashboard-share-api.spec.tsis the reference implementation. (Caught in review by a parallel session — credit where due.)2. Two 403 branches with a byte-identical body means the response body is not evidence of which check fired.
WidgetApiController::denyAddWidget()returnsResponseHelper::forbidden()— body{"error":"Access denied"}— from bothPermissionService::canAddWidget()(the share check) andRoleFeaturePermissionService::isWidgetAllowed()(the role-feature allow-list).This bit this PR. The REQ-SHARE-004 test failed in CI with that body, which reads as the share check refusing. It was the allow-list:
tests/e2e/fixtures/role-feature-permissions.tsinstalls a restrictivedefaultrow (allowedWidgets: ['activity','recommendations']) and ten widget-adding specs call it inbeforeAll; withworkers: 1against one instance that row was in force whether this file asked for it or not.isWidgetAllowed()short-circuits for Nextcloud admins, but the provisioned recipient is a plain account, solabelwas refused outright.The dangerous half is that the
view_onlyCONTROL arm passed the whole time — it expects 403 and the allow-list supplied one. It was green while proving nothing whatever about share permissions. It only surfaced because the arm next to it could still fail.Fixed without bending the assertion: the file now pins the allow-list itself in
beforeAll, and both arms addactivity, which that restriction permits — so the allow-list is constant and the share level is the only variable. The refusal atview_onlyis attributable to the share check precisely because the same widget id succeeds for the same user atfull. A splitting probe now asserts the share row readsfullbefore the add is attempted, so a future failure names its own cause.Where the spec prose disagrees with the shipped app
Recorded in
tests/e2e/ci/dashboard-share-api.spec.ts's header rather than worked around silently:GET /api/dashboards. That route isdashboardApi#list→getUserDashboards()→findByUserId()— the caller's own rows only. The union that folds in shares isGET /api/dashboards/visible. Behaviour implemented; path stale.effectivePermissionLevel. No response carries that key./visiblecarriessource+isOwner; the resolved level comes fromGET /api/dashboard/{id}aspermissionLevel, withisOwnerandsharedBy. The tests assert the keys the app actually sends..github#343 — header-block
@e2etags counted without a test body_TestDoc.owner()resolves a tag sitting above everytest(to simply the first test in the file, so all of a header block's slugs are credited to one body whatever that body asserts — a tag for a scenario nobody tested counts the same.tests/e2e/ci/public-share.spec.tslisted four slugs that way. They are moved onto the test that proves them (it brackets a valid-token render with an unissued-token control and a post-revoke control, which is all four — but that was luck of authorship, not something the gate checked). Every tag added on this branch sits directly above its owntest().Not fixed here, and worth its own pass. A crude scan (tags appearing before the first
test(/test.describe(in each file) puts the remaining header-block population in the low hundreds, led by:@e2elinesspec-coverage/spec-coverage.spec.tsci/public-share-lifecycle.spec.tswave3-runtime-shell.spec.tstext-display-widget.spec.tsresponsive-grid-breakpoints.spec.tsThat scan is an upper bound, not a count: it cannot tell a real tag from a prose mention of one, and several of these files sit in
testIgnore, where the gate credits nothing anyway. The number is deliberately left as a shape rather than a figure — the point is that the exposure is large and concentrated, and that re-anchoring it needs the parser, not a grep.Still open — 142
role-feature-permissions27 ·dashboard-kiosk-mode23 (filed, #93) ·conditional-visibility-editor23 ·clock-weather-widgets19 ·service-health-ping15 ·live-data-tile-widget15 ·iframe-embed-widget15 ·tile-quick-search4 ·runtime-shell1 (filed, #94)The remaining 4
tile-quick-searchare the REQ-QSEARCH-004 fallback scenarios: they needquicksearch_fallback_targetchanged instance-wide and the shell reloaded, since it is read from initial state. Left open rather than annotated.Constraints held
No
test.skip/describe.skip, no deleted tests, nogrepInvert, no raised timeouts, no widenedtestIgnore. Nosed/awk/scripted edits to code or spec files. NoCo-Authored-Bytrailers.