Skip to content

fix(e2e): the decisions-leaf skip guard asks about the right app - #2058

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/decisions-leaf-skip-guard-names-the-right-app
Sep 9, 2026
Merged

fix(e2e): the decisions-leaf skip guard asks about the right app#2058
rubenvdlinde merged 1 commit into
developmentfrom
fix/decisions-leaf-skip-guard-names-the-right-app

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Found while fixing ConductionNL/.github#722, which is the cause of this one.

What was wrong

retired-surfaces.spec.ts' decisions-leaf test skipped on the absence of OpenRegister's integration registry, with the reason "the decision app is not installed on this instance". Those are two different apps. The registry is OpenRegister's, installed by its openregister-integration-global bundle on every page.

That bundle was never built in CI: openregister gitignores /js/, and the shared workflow cloned it and ran composer install without ever building its frontend. So the registry was absent on every run, this test skipped on every run, and it described openregister's packaging in the words of decidiq's absence.

A skip renders exactly like a pass. This read as covered for its whole life.

How it surfaced

#722 adds the missing build step. The moment it ran, this test started executing and failing — correctly, because decidiq really is not installed in dossiq's CI. Measured on #2053: of 381 tests, exactly two results changed, and this was one of them.

Error: decidesk-decisions leaf not registered; registry holds:
["files","notes","tags","tasks","audit-trail","version-history","shares",
 "calendar","contacts","contactmoment","email","talk","openproject",
 "xwiki","bookmarks","col…]

That list is also the proof the bundle now loads: a populated registry, where there was none.

The change

Probe OC.appswebroots, which is how Nextcloud itself resolves an installed app and is already this suite's probe for exactly this (dutch-value-l10n.spec.ts). Both ids are accepted — the fleet rename is in flight and lib/Support/FleetAppId.php maps decidiq => ['decidiq', 'decidesk'], so naming one would make the test skip on whichever side of the rename an instance sits.

The skip is now taken only on the fact that earns it: neither app is installed. On dossiq's CI that is still the branch taken, so this test still skips here — but it now says something true, and it will start asserting the moment the decisions app is present.

An absent registry is no longer skipped past. With the decisions app installed, it fails loudly, because it means every integration leaf in this app is silently rendering nothing. That is the condition that hid this test, and it should never again be able to hide anything.

Order

Independent of #722 and safe either way: without it this test keeps skipping (for a better-stated reason), with it the test skips instead of failing red.

Follow-up, not done here

dossiq's CI installs openregister and nothing else, so this test can never actually assert. Adding decidiq to e2e-additional-apps would turn it into a real check. That is a cost decision about CI minutes and belongs in its own PR.

The guard read the absence of OpenRegister's integration registry as
"the decision app is not installed". Those are two different apps. The
registry is OpenRegister's, installed by its
openregister-integration-global bundle on every page, and that bundle was
never built in CI — openregister gitignores /js/ and the shared workflow
cloned it without building it. So the registry was absent on every run,
this test skipped on every run, and it reported a fact about
openregister's packaging in the words of decidiq's absence.

A skip renders like a pass, so it read as covered for its whole life.

ConductionNL/.github#722 builds that bundle. The moment it did, this test
started running and failing, correctly: decidiq really is not installed
here. Measured on dossiq#2053, where it is the only test whose result
changed apart from the one #722 fixes.

Probe OC.appswebroots instead, which is how Nextcloud resolves an
installed app and is already this suite's probe for it in
dutch-value-l10n.spec.ts. Both ids are accepted because the fleet rename
is in flight and lib/Support/FleetAppId.php maps
decidiq => ['decidiq', 'decidesk'].

An absent registry is no longer skipped past. With the decisions app
installed it is now a loud failure, because it means every integration
leaf in this app is silently rendering nothing — the exact condition that
hid this test.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ 1c7e888

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue3-compile
test-l10n
format
check-schema-l10n
check-l10n-js
composer ✅ 106/106
npm ✅ 549/549
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-09-09 09:02 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit ef177d7 into development Sep 9, 2026
47 of 49 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/decisions-leaf-skip-guard-names-the-right-app branch September 9, 2026 09:14
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