Skip to content

test(e2e): the registry spec must report a diff, not stand down - #868

Merged
rubenvdlinde merged 5 commits into
developmentfrom
fix/registry-spec-must-assert-not-stand-down
Aug 24, 2026
Merged

test(e2e): the registry spec must report a diff, not stand down#868
rubenvdlinde merged 5 commits into
developmentfrom
fix/registry-spec-must-assert-not-stand-down

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What was hidden

integration-registry.spec.ts ran 12 of its 76 tests. The other 64 skipped,
and 46 said a version of "leaves PR not deployed yet" — a claim that cannot
be true in decidiq's own CI, where the app under test is the commit under
test.

The mechanism, verbatim from the file:

if (ids.length < EXPECTED_COUNT) {
  test.skip(true, `partial registry: ${ids.length}/${EXPECTED_COUNT} providers
    — leaves PR not deployed yet (have: …)`)
}
expect(ids).toHaveLength(EXPECTED_COUNT)
for (const id of EXPECTED_IDS) expect(ids).toContain(id)   // ← never ran

The stand-down sits directly above the loop that names exactly which
providers are missing
. So a registry at 27/29 skipped instead of naming the
two, and a registry that was 93% correct reported as "not deployed".

The two were hermiq-agent and sync-contract — both registered by another
app's
bundle. This CI installs exactly one additional app, openregister
(.github/workflows/code-quality.ymladditional-apps), so neither can ever
register here. Requiring them is what stood the whole spec down.

What replaces it

openregister's IntegrationsCapability::describe() already publishes
available — the server's own answer to "is this provider's backing
Nextcloud app installed?"
— and documents it as a public discovery field for
exactly this decision. So the spec now asks the instance instead of guessing:

  • providerCaps() reads id, enabled, requiredApp, available from OCS caps
  • absenceReason() returns a reason naming the absent app, or null
  • a skip is permitted only when the server gives such a reason; everything else asserts
test before after
registry list skip at 27/29 arrayContaining(EXPECTED_IDS) — a missing provider fails by name
cross-app leaves required, so always short asserted conditionally, by app
sidebar count compare 5 against 29, skip compare against what the server says is available (5 here)
per-id tab (×27) skip("not rendered — leaves PR not deployed yet") skip only on a server-given absence; otherwise the tab must render

Not toHaveLength: another app installed alongside decidiq legitimately adds
providers, and an exact total would make every such addition a failure — which
is the same trap in a new coat.

Expect this to surface things

This deliberately converts skips into assertions, so anything that was hiding
behind them will now show. That is the point. A provider the server reports
as available whose tab does not render is a defect, and until now it was
indistinguishable from a green run.

If CI goes red here, read it as the spec finally doing its job — I'll fix what
it finds, or give the skip an accurate, server-backed reason.

Verification

check result
prettier --check clean
playwright test --list parses; 202 tests across 39 files
execution CI — decidiq is not installed on this machine, and the dev container is down

Related: ConductionNL/.github#559, the gate that surfaced these 46.

integration-registry.spec.ts ran 12 of its 76 tests. The other 64 skipped, and
46 of those said a version of "leaves PR not deployed yet" — a claim that cannot
be true in decidiq's own CI, where the app under test IS the commit under test.

The mechanism, verbatim from the file:

    if (ids.length < EXPECTED_COUNT) {
      test.skip(true, `partial registry: ${ids.length}/${EXPECTED_COUNT}
        providers — leaves PR not deployed yet (have: …)`)
    }
    expect(ids).toHaveLength(EXPECTED_COUNT)
    for (const id of EXPECTED_IDS) expect(ids).toContain(id)

The stand-down sits DIRECTLY ABOVE the loop that names exactly which providers
are missing. So a registry at 27/29 skipped instead of naming the two, and a
registry that was 93% correct reported as "not deployed".

The two were `hermiq-agent` and `sync-contract` — both registered by ANOTHER
app's bundle. This CI installs exactly one additional app, openregister
(.github/workflows/code-quality.yml `additional-apps`), so neither can ever
register here. Requiring them is what stood the whole spec down.

WHAT REPLACES IT

openregister's IntegrationsCapability::describe() already publishes `available`
— the server's own answer to "is this provider's backing Nextcloud app
installed?" — and documents it as a public discovery field for exactly this
decision. So the spec now asks the instance instead of guessing:

  * `providerCaps()` reads id, enabled, requiredApp, available from OCS caps.
  * `absenceReason()` returns a reason NAMING THE ABSENT APP, or null.
  * A skip is permitted only when the server gives such a reason. Everything
    else asserts.

Concretely:
  - the registry test asserts arrayContaining(EXPECTED_IDS) — a missing
    provider now fails BY NAME. Not toHaveLength: another app installed
    alongside decidiq legitimately adds providers, and an exact total would
    make every such addition a failure.
  - cross-app mount leaves are asserted conditionally, by app.
  - the sidebar count compares against what the server says is AVAILABLE (5 on
    this CI), not against 29. The old code compared 5 against 29 and blamed a
    frontend deploy for a sidebar that was behaving correctly.
  - each per-id tab test skips only on a server-given absence, and otherwise
    REQUIRES the tab to render.

This will surface anything that was hiding behind the stand-downs. That is the
point: a provider the server reports as available whose tab does not render is
a defect, and until now it was indistinguishable from a green run.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidiq @ d765193

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

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

Download the full PDF report from the workflow artifacts.

Correction to the previous commit, found by the CI run it produced.

The per-id tab loop and the sidebar count both ran over EXPECTED_IDS, which
includes `decidesk-decisions` — a `renderMode: 'mount'` leaf. Mount leaves
expose mount()/unmount() INSTEAD of a tab + widget component, as this file's
own MOUNT_IDS docblock says and as its parity test asserts. Demanding a tab for
one is wrong.

The old code had the same error; it was invisible because it skipped when the
tab was (correctly) absent. Removing the stand-down turned it into a failure,
which is the spec being wrong rather than the app.

Adds TAB_IDS — builtin + external + component leaves, no mount leaves — and
points both the per-id loop and the count expectation at it.

WHAT THE RUN ALSO FOUND, AND IT IS REAL

The remaining failures are not spec error. On decidiq run 32702211376:

  * the JS registry carries every expected provider           (test passed)
  * every component leaf carries a tab + widget component     (test passed)
  * OCS caps and the JS registry agree, no drift              (test passed)
  * the server reports 10 providers available
  * the sidebar renders 5 tabs

Five is exactly the number of hard-coded built-ins in CnObjectSidebar's
BACKWARDS-COMPATIBLE branch (files, notes, tags, tasks, audit-trail), which is
what renders when `useRegistry` is false. src/manifest.json sets
`config.sidebar.useRegistry: true` on MeetingIntegrations
(/meetings/:id/integrations), so the flag is declared and is not reaching
CnObjectSidebar at runtime — App.vue binds
`:useRegistry="objectSidebarState.useRegistry"`, whose default is false.

So ADR-019's whole integration surface is inert on those pages, and the
registry-mode assertions in this spec have never actually run against registry
mode.

The old guard predicted this in so many words — 'registry sidebar mode not
active — check use-registry forwarding' — but it only fired at count === 0, and
the five fallback tabs kept the count non-zero, so control fell through to the
'partial sidebar … leaves PR not deployed yet' skip and blamed a deploy for a
forwarding bug.

This commit does NOT fix that. It removes the one assertion that was mine to
fix, so what remains red is the app.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

CI ran it, and it found two things — one mine, one real

Mine, now fixed (c229b68)

The per-id tab loop and the sidebar count ran over EXPECTED_IDS, which
includes decidesk-decisions — a renderMode: 'mount' leaf. Mount leaves
expose mount()/unmount() instead of a tab, as this file's own
MOUNT_IDS docblock says. Demanding a tab for one is wrong.

The old code had the same error; it was invisible because it skipped when the
tab was (correctly) absent. Added TAB_IDS (builtin + external + component
leaves) and pointed both assertions at it.

Real, and not fixed here

check result
JS registry carries every expected provider passed
every component leaf carries a tab + widget component passed
OCS caps and JS registry agree, no drift passed
server reports providers available 10
sidebar renders tabs 5

Five is exactly the number of hard-coded built-ins in CnObjectSidebar's
backwards-compatible branchfiles, notes, tags, tasks, audit-trail
which is what renders when useRegistry is false.

src/manifest.json sets config.sidebar.useRegistry: true on
MeetingIntegrations (/meetings/:id/integrations). So the flag is
declared
and is not reaching CnObjectSidebar at runtime: App.vue binds
:useRegistry="objectSidebarState.useRegistry", whose default is false.

ADR-019's integration surface is inert on those pages, and the
registry-mode assertions in this spec have never run against registry mode.

The part worth keeping

The old guard predicted this almost word for word —

'registry sidebar mode not active — check use-registry forwarding'

— but it only fired at count === 0, and the five fallback tabs kept the count
non-zero. Control fell through to the "partial sidebar … leaves PR not
deployed yet"
skip, which blamed a deployment for a forwarding bug. The
diagnosis was already written down; the stand-down is what stopped anyone
reading it.

Where this leaves the PR

Still red, and the red is now the app rather than the spec. This should not
merge until the useRegistry forwarding is fixed — merging a correct spec
against a broken surface would just turn development red.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidiq @ 3b4bbe9

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

Quality workflow — 2026-08-24 10:55 UTC

Download the full PDF report from the workflow artifacts.

rubenvdlinde added a commit to ConductionNL/nextcloud-vue that referenced this pull request Aug 24, 2026
…thing (#754)

`config.sidebar.useRegistry` reaches `resolvedSidebar()` intact (the Object
branch returns `{show, enabled, ...cfg}`) and survives `mergeSidebarSources()`,
which spreads `resolved` before its own field list. It was then never published
onto the shared `objectSidebarState` channel: `assignSidebarState()` writes
active/open/objectType/objectId/object/schemaObject/title/subtitle/register/
schema/hiddenTabs/tabs, and nothing else.

So the host's CnObjectSidebar always saw the prop default `false` and rendered
its BACKWARDS-COMPATIBLE branch — the five hard-coded built-in tabs (files,
notes, tags, tasks, audit-trail) — instead of one tab per registered provider.
Every app that opted a page into the registry sidebar has been getting the
built-ins, and the declaration did nothing at all.

The word `useRegistry` did not appear anywhere in CnDetailPage.vue. Decidiq's
App.vue carries the comment "Set by CnDetailPage when its manifest
config.sidebar.useRegistry is true", describing a write that was never
implemented.

MEASURED, decidiq run 32702211376, whose MeetingIntegrations page
(/meetings/:id/integrations) sets `config.sidebar.useRegistry: true`:

    JS registry carries every expected provider        passed
    every component leaf has a tab + widget component  passed
    OCS caps and the JS registry agree, no drift       passed
    providers the server reports available                10
    tabs the sidebar rendered                              5

Five, exactly — the fallback set. ADR-019's whole integration surface was inert
on those pages, and decidiq's registry-mode e2e assertions had never once run
against registry mode.

`excludeIntegrations` rides the same channel and was dropped the same way;
without it a page cannot suppress a provider it does not want.

Opt-in is strict `=== true`, so a truthy-but-not-boolean manifest value cannot
switch a host into registry mode by accident.

Four tests in CnDetailPageSidebarConfig.spec.js, two of them negative controls.
Verified must-fail: with this commit's src change reverted, the two positive
tests fail and the two controls still pass. Full suite 6557 tests / 552 suites
green, eslint clean.

Found because decidiq's e2e stopped skipping (ConductionNL/decidiq#868). The
old guard had predicted it in words — "registry sidebar mode not active — check
use-registry forwarding" — but only fired at zero tabs, and the five fallback
tabs kept the count non-zero, so it fell through to a skip that blamed an
undeployed dependency.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidiq @ f1028ea

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

Quality workflow — 2026-08-24 18:12 UTC

Download the full PDF report from the workflow artifacts.

…gistry

This branch's registry-sidebar spec asserts the gap instead of standing down,
and it has been failing honestly: 7 tests, all reporting that a
`config.sidebar.useRegistry: true` page renders 5 tabs rather than one per
registered provider.

The cause was never in this app. `CnDetailPage.syncSidebarState()` never
included `useRegistry` in the field list it hands to `assignSidebarState()`,
and that helper only writes the keys it is given — so the flag reached
`resolvedSidebar` intact, survived `mergeSidebarSources()`, and was dropped on
the last hop. The host sidebar therefore saw the prop default `false` and
rendered its five hard-coded built-in tabs.

ConductionNL/nextcloud-vue#754 fixes it. This lockfile was pinned to 2.15.0,
which predates it — 2.16.0 is the first release that carries it.

Verified against the published tarball rather than the repo, because a green
source branch says nothing about what consumers install:

  useRegistry publish line in 2.16.0 ... present
  CnEditSupportModal export ........... present
  supportButton in the v2 schema ...... present

The spec should now pass on its own. If it does not, the remaining failure is
a real one in this app and worth reading rather than re-pinning.
Last failure on this PR:

    clicking tab-button-shares activates it + mounts the panel
    Locator: aside.app-sidebar [role="tabpanel"]:not([hidden]),
             aside.app-sidebar .app-sidebar__tab   .first()
    Expected: visible   Received: hidden

The tab itself was fine — `aria-selected=true` passed on the line above. The
selector was a UNION whose second branch, `.app-sidebar__tab`, did not exclude
hidden elements, and the sidebar renders one such element per tab with all but
the active one hidden. `.first()` returns the first match in DOM ORDER, not the
first visible one, so it kept landing on a hidden sibling of the panel that had
just opened correctly.

`:not([hidden])` could not have saved it: these panels are hidden by CSS, not by
the `hidden` attribute, so that guard never applied to the branch that mattered.

Both branches now carry `:visible`.

WHERE THIS PR NOW STANDS

nextcloud-vue 2.16.0 (this branch's dependency bump) carries the useRegistry
forwarding fix, and the sidebar is finally in registry mode. Measured across the
two runs:

    before   135 passed   8 failed   46 skipped
    after    141 passed   1 failed   46 skipped

Seven of the eight were the one nextcloud-vue defect. This commit is the eighth,
and it was mine.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidiq @ 211a6ed

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

Quality workflow — 2026-08-24 22:42 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidiq @ 51926b6

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

Quality workflow — 2026-08-24 23:20 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit caf3dc7 into development Aug 24, 2026
87 checks passed
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