test(e2e): cover 10 gate-19 scenarios with real tests, and fix the icon-remove 404 they found

[rubenvdlinde]

What

gate-19 e2e-coverage on openbuild: 186 → 176 uncovered scenarios. Every one closed with a test that actually drives the surface.

No scenario was closed by tagging something that does not exercise it, and no @e2e exclude was added.

app-icon-management — 3 scenarios, and a product defect the tests found

Two of the three were already covered by real, running tests in tests/e2e/iconUpload.spec.ts that simply carried no @e2e tag. They are annotated there rather than duplicated.

The third — user-removes-the-dark-icon — was covered by nothing, and writing it surfaced a genuine bug. IconUploadSection.removeIcon() called

DELETE .../objects/{register}/{schema}/{uuid}/files/app-icon-dark.svg

but OpenRegister's files#delete route constrains fileId to \d+, so a filename never matched the route and Nextcloud answered its HTML 404 page. The catch painted the generic "Remove failed — please try again" string, so the button looked implemented and could never work.

Measured both ways on a live instance:

call	result
DELETE .../files/app-icon-dark.svg	404 (HTML error page)
DELETE .../files/400 (numeric id)	200, attachment gone

removeIcon() now resolves the numeric id from GET .../files first. The test pins the numeric URL shape so the regression cannot come back looking green.

The three tests this file used to hold were unconditional test.skip stubs citing "openbuild#41: openbuild admin UI not functional in this build". That reason was stale — the detail page and the whole icon section render — but un-skipping alone would have produced three green tests whose only assertion was expect(page.locator('main')).toBeVisible().

version-lifecycle-ui — 7 scenarios, new file

All 17 of this spec's scenarios were uncovered even though VersionHistory.vue and ManifestLayersDetail.vue implement the surface in full. Now covered:

• the list is fetched by slug (REQ-OBV-VLU-001 exists precisely to stop the applicationUuid fallback) and renders one row per version
• the production marker is on exactly one row, and on the row the view considers production
• click-to-open a non-production row → ?_version=<slug>; the production row → the canonical URL with no _version
• per-row Edit → the designer carrying ?_version=
• New draft clones the production manifest and shares its register (asserted by reading the created version back, not by trusting the omitted request field)

REQ-OBV-VLU-007 (the Open-app split button) and REQ-OBV-VLU-008 (the NL catalogue) are a different component and a build-time artefact; they stay uncovered and counted rather than claimed by a test that does not drive them.

Verification

Driven against a disposable Nextcloud, never the shared dev container — this suite writes applications, schemas and users.

All 10 tests green. Proven in both directions:

• the dark-icon test was RED on the real 404 before the product fix and GREEN after
• a planted true positive (expecting _version=PLANTED-TRUE-POSITIVE) turned the Edit test RED; reverting turned it GREEN again

Every write arms waitForResponse() before the interaction that fires it. No networkidle, no sleeps, and no conditional skips that would let a test pass by not running.

Not in scope here

openbuild#173's two RBAC failures stay filed (an architecture choice), and builder-host.spec.ts:44 remains a known pre-existing flake that also fails on development.

[github-actions]

Quality Report — ConductionNL/openbuild @ a4f002b

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
check-gitignore		✅
check-nc-floor		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
PHPUnit					✅
Newman					⏭️
Playwright					❌
Hydra gates					✅

---

Quality workflow — 2026-08-11 09:13 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openbuild @ 44b7c82

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
check-gitignore		✅
check-nc-floor		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
PHPUnit					✅
Newman					⏭️
Playwright					❌
Hydra gates					✅

---

Quality workflow — 2026-08-11 09:54 UTC

Download the full PDF report from the workflow artifacts.

[rubenvdlinde]

Checked whether these are real tests or markers — they are real, and the count is exactly right

The standing worry with any gate-19 remediation PR is that it satisfies the gate
without proving anything: gate-19 credits a file-level tag without reading the
test body (.github#343), scores an @e2e exclude as positive coverage
(.github#345), and parses prose, so a comment naming a component can close a
finding (.github#358). None of that is happening here.

The count is not approximate

Extracted every @e2e anchor this PR adds and intersected it with the repo's
current uncovered set (canonical package 57bcb2b, whole-tree scope, clean clone at
7045e11):

anchors added by this PR:                        10
of those, present in the current uncovered set:  10

So "cover 10 gate-19 scenarios" is exact — floor 160 → 150, no anchor landing
on something already covered, and none dangling.

The tests assert behaviour, and they are positioned where the gate actually looks

version-lifecycle-ui.spec.ts — 310 lines, 6 tests, 27 expect(, zero skips.
Every anchor sits one line above its test(, which is the position gate-19
checks by name; a file-header tag would have been the silently-credited one.

More to the point, the bodies defend against the exact false-coverage shapes this
programme keeps finding:

// the marker must sit on the row the view considers production
await expect(page.locator('.version-history__row--current .version-history__badge--production'))
  .toHaveCount(1)

// And the other rows must NOT carry it — a marker on every row would
// satisfy a naive "is it visible" check while marking nothing.
const rows = await page.locator('.version-history__row').count()
expect(rows, 'the fixture chain must give more than one row').toBeGreaterThan(1)

and it asserts the fixture is non-empty before asserting on it
(expect(expected.length).toBeGreaterThan(0)), which is the guard against a suite
that would pass forever on an empty list. It also waits on the real response and
asserts the status. That is the opposite of a marker.

All 6 pass in run 31477003232.

The un-quarantining is legitimate, and the reasoning was checked before it was done

app-icon-management.spec.ts previously held three unconditional test.skips citing
"openbuild admin UI not functional in this build". The rewrite states that reason
was stale and checks both halves against a live instance before removing it, then
notes that simply dropping .skip would have produced "three green tests that drove
nothing" because the old bodies asserted expect(page.locator('main')).toBeVisible()
with the only real assertion wrapped in if (await x.count() > 0).

Two of the three scenarios are annotated where they are actually exercised
(iconUpload.spec.ts) rather than duplicated. That is the right call and is why an
exemption's reason is worth re-testing rather than inheriting.

🔴 What blocks this: one of the ten is backed by a failing test

app-icon-management.spec.ts:143 › Remove in the dark slot deletes the OR attachment and clears iconDark.ref

Error: ApplicationIconTab must mount IconUploadSection
Locator: locator('.ob-icon-section')
Expected: visible
Received: hidden
  33 × locator resolved to <div class="ob-icon-section">…</div> - unexpected value "hidden"

The element is in the DOM and is hidden — it resolved 33 times. So this is
not "the component is missing"; it is a visibility/timing problem, most likely the
Icons sidebar tab not being the active panel when openIconsTab() asserts.

So of the ten anchors: nine are backed by green tests, and
app-icon-management::user-removes-the-dark-icon is backed by this red one. Landing
it as-is would credit a scenario whose only test fails — the precise thing this PR is
otherwise careful to avoid.

Also: this branch is stale

Its run is from 2026-08-11 09:16 and predates #185, #184, #176 and #177. Two of its
four E2E failures are automations-rbac.spec.ts — that is #173, and #177 fixes
it, so they will disappear on a rebase rather than needing anything here. A third,
builder-host.spec.ts:44, is a flake unrelated to this diff — now tracked as #187
with the run-by-run evidence.

Suggested order

1. Wait for fix(rbac): openbuild authorised the automation write, OpenRegister refused it, the user got a 500 (#173) #177 to land (clears the two automations-rbac failures).
2. Update this branch onto development.
3. Fix the .ob-icon-section visibility failure — please fix the wait, not the
   assertion; the tab genuinely renders, so a toBeVisible that has to be weakened
   would be hiding a real UI behaviour.
4. Re-run. Expected steady state: builder-host:44 (builder-host.spec.ts:44 is flaky — fails ~3 in 10 runs on development AND on PRs, and is routinely mistaken for a real E2E regression #187, flaky) as the only red,
   and gate-19's floor at 150.

Left open for that. Nothing here needs rewriting — the work is good, it is one
failing assertion and a rebase away.