Skip to content

test: pin the public review-aggregate wire contract and prove five page surfaces render - #470

Merged
rubenvdlinde merged 5 commits into
developmentfrom
fix/contract-and-visual-coverage
Aug 9, 2026
Merged

test: pin the public review-aggregate wire contract and prove five page surfaces render#470
rubenvdlinde merged 5 commits into
developmentfrom
fix/contract-and-visual-coverage

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

gate-25 contract-coverage 1 → PASS, gate-26 visual-coverage 6 → PASS. hydra-gates 48c88ba1e0d049f8f38538c33e790d3e603c55d0, measured with --scope-to-diff --base origin/beta — the scope a push to development actually runs.

gate-25 — GET /api/reviews/aggregate had no automated proof

review#aggregate is #[PublicPage]. An anonymous visitor on a module or dienst detail page reads it, so its response shape and its status codes are part of the app's public surface — not an implementation detail. ReviewControllerContractTest pins them:

  • success is exactly {average, count, items} with HTTP 200;
  • the service's internal ok/reason bookkeeping never reaches the wire;
  • a subject with no approved reviews is a 200 with a null average, not a 404 — a consumer rendering a star widget has to tell "nothing approved yet" from "bad request", and that difference is the contract;
  • a rejected request is a 400 carrying exactly message;
  • subjectType/subjectId reach the service unaltered.

gate-26 — five page components with no browser-level proof

tests/e2e/spec-coverage/page-surfaces.spec.ts drives the real UI: it clicks the app's own navigation the way a user does, then asserts the page's own content region rendered and that the app logged no console error and returned no 5xx.

component surface
FacetedCatalogIndexView.vue Applications and Services
suites/SuitesIndexView.vue Suites
organisaties/PortfolioReport.vue Portfolio rationalization
settings/sections/EolSyncSettings.vue admin settings section

Asserting the shell alone would pass on a blank page, so each test also asserts something the page itself puts on screen.

KwetsbaarhedenView and LicensePostureView already had real behavioural specs the gate could not attribute to them; those two specs now name their page component in the docblock. No new assertion was invented for them and none was needed — the coverage already existed, only the link to the component was missing. This is the one case in this PR where the fix is a reference rather than a test, and it is a reference to a test that genuinely drives that page.

Visual baselines were deliberately not added: toHaveScreenshot baselines are platform-specific pixels, and generating them on this machine to satisfy a gate would commit screenshots that CI's renderer disagrees with — a red gate traded for a flaky one. The e2e route the gate offers is the honest one here.

Can-fail proof

Removing page-surfaces.spec.ts and ReviewControllerContractTest.php and reverting the two docblocks puts both counts back exactly:

[gate-25] contract-coverage: FAIL — 1 new public endpoint(s) missing a contract test
[gate-26] visual-coverage:   FAIL — 6 new page component(s) missing a visual baseline

And the contract tests themselves can fail: mutating ReviewController::aggregate to put 'ok' => $result['ok'] on the wire turns 2 of the 5 red —

1) testAggregateDoesNotLeakInternalBookkeepingKeys
   Failed asserting that an array does not have the key 'ok'.
2) testAggregateSuccessBodyCarriesExactlyTheContractKeys
   Failed asserting that two arrays are identical.

Other measurements

  • Unit suite 524 tests, 1844 assertions, 0 failures (25 pre-existing skips).
  • phpcs lib/ unchanged at 0 errors / 87 warnings.
  • tsc --noEmit on the new spec: clean.
  • Gate count vs origin/beta: 14 → 12 failing.

@rubenvdlinde
test: pin the public review-aggregate wire contract and prove five pa…
e6f5319 
…ge surfaces render

gate-25 contract-coverage 1 -> PASS, gate-26 visual-coverage 6 -> PASS, measured
with hydra-gates 48c88ba against origin/beta — the scope a push to development
actually runs.

gate-25 — GET /api/reviews/aggregate had no automated proof
  `review#aggregate` is `#[PublicPage]`: an anonymous visitor on a module or
  dienst detail page reads it, so its response shape and status codes are part
  of the app's public surface. ReviewControllerContractTest pins them:
  - success is exactly `{average, count, items}` with HTTP 200;
  - the service's internal `ok`/`reason` bookkeeping never reaches the wire;
  - a subject with no approved reviews is a 200 with a null average, not a 404
    — a consumer rendering a star widget has to tell "nothing approved yet"
    from "bad request";
  - a rejected request is a 400 carrying exactly `message`;
  - subjectType/subjectId reach the service unaltered.

gate-26 — five page components with no browser-level proof
  tests/e2e/spec-coverage/page-surfaces.spec.ts drives the REAL UI by clicking
  the app's own navigation, then asserts the page's own content rendered and
  that the app logged no console error and returned no 5xx:
  FacetedCatalogIndexView (Applications and Services), SuitesIndexView,
  PortfolioReport, and EolSyncSettings inside the admin settings shell.
  Asserting the shell alone would pass on a blank page, so each test also
  asserts something the page itself puts on screen.

  KwetsbaarhedenView and LicensePostureView already had real behavioural specs
  that the gate could not attribute to them; those two specs now name their
  page component in the docblock. No new assertion was invented for them and
  none was needed — the coverage already existed.

Can-fail proof: removing the new files and reverting the two docblocks puts
both counts back exactly — gate-25 1, gate-26 6. Mutating the controller to
leak `ok` onto the wire turns 2 of the 5 contract tests red.

Unit suite 524 tests green; phpcs lib/ unchanged at 0 errors / 87 warnings.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ f332730

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue-demi ✅
test-l10n ✅
composer ✅ ✅ 128/128
npm ✅ ✅ 718/718
PHPUnit ✅
Newman ⏭️
Playwright ✅
Hydra gates ✅

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
test: pin the public review-aggregate wire contract and prove five pa…
2f30d4b 
…ge surfaces render

gate-25 contract-coverage 1 -> PASS, gate-26 visual-coverage 6 -> PASS, measured
with hydra-gates 48c88ba against origin/beta — the scope a push to development
actually runs.

gate-25 — GET /api/reviews/aggregate had no automated proof
  `review#aggregate` is `#[PublicPage]`: an anonymous visitor on a module or
  dienst detail page reads it, so its response shape and status codes are part
  of the app's public surface. ReviewControllerContractTest pins them:
  - success is exactly `{average, count, items}` with HTTP 200;
  - the service's internal `ok`/`reason` bookkeeping never reaches the wire;
  - a subject with no approved reviews is a 200 with a null average, not a 404
    — a consumer rendering a star widget has to tell "nothing approved yet"
    from "bad request";
  - a rejected request is a 400 carrying exactly `message`;
  - subjectType/subjectId reach the service unaltered.

gate-26 — five page components with no browser-level proof
  tests/e2e/spec-coverage/page-surfaces.spec.ts drives the REAL UI by clicking
  the app's own navigation, then asserts the page's own content rendered and
  that the app logged no console error and returned no 5xx:
  FacetedCatalogIndexView (Applications and Services), SuitesIndexView,
  PortfolioReport, and EolSyncSettings inside the admin settings shell.
  Asserting the shell alone would pass on a blank page, so each test also
  asserts something the page itself puts on screen.

  KwetsbaarhedenView and LicensePostureView already had real behavioural specs
  that the gate could not attribute to them; those two specs now name their
  page component in the docblock. No new assertion was invented for them and
  none was needed — the coverage already existed.

Can-fail proof: removing the new files and reverting the two docblocks puts
both counts back exactly — gate-25 1, gate-26 6. Mutating the controller to
leak `ok` onto the wire turns 2 of the 5 contract tests red.

Unit suite 524 tests green; phpcs lib/ unchanged at 0 errors / 87 warnings.
@rubenvdlinde
test(e2e): assert on the ITEM, not the container, in the page-surface…
0d990cb 
… specs

The first draft asserted the page's title text inside <main>. That passes on a
blank page and on the WRONG page: the shell renders <main> for every route and
the nav echoes the same label, so the assertion could not distinguish 'the page
rendered' from 'something rendered'.

Each test now asserts on markup only the component under test declares:
  - FacetedCatalogIndexView -> the CnFacetSidebar title plus all four GEMMA
    dimensions it builds from DIMENSION_LABELS (Reference component, Standard,
    Application service, Domain);
  - SuitesIndexView -> the 'New suite' wizard trigger in its own action slot;
  - PortfolioReport -> [data-testid=pr-summary] and its five declared column
    headers, read as columnheader roles;
  - EolSyncSettings -> the 'End-of-life feed sync' section name and its
    'Sync now' control.

These anchors are declared by the component rather than derived from rows, so
an empty seed makes them ABSENT rather than merely empty — which is what makes
them a check that can fail.
@rubenvdlinde
Merge remote-tracking branch 'origin/fix/contract-and-visual-coverage…
cc4b7c0 
…' into fix/contract-and-visual-coverage

# Conflicts:
#	tests/e2e/spec-coverage/page-surfaces.spec.ts
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ b427719

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue-demi ✅
test-l10n ✅
composer ✅ ✅ 128/128
npm ✅ ✅ 718/718
PHPUnit ✅
Newman ⏭️
Playwright ❌
Hydra gates ✅

Quality workflow — 2026-08-09 20:10 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
fix(facets): the GEMMA facet sidebar never rendered a single filter
a13f0b5 
Found by strengthening the gate-26 e2e assertions from the container to the
ITEM. The first draft asserted the page title inside <main>; it passed. Asserting
the four GEMMA dimensions the page claims to render failed immediately.

FacetedCatalogIndexView passed `:filters="facetDimensionFilters"` to
CnFacetSidebar. CnFacetSidebar declares no `filters` prop — its props are
`schema`, `facetData`, `activeFilters`, `loading`, `title`, `clearLabel`,
`userIsAdmin` — and it derives its own list with
`effectiveFilters() => filtersFromSchema(this.schema)`. Vue drops an undeclared
prop into `$attrs` silently, `schema` was never passed, and
`filtersFromSchema(null)` returns []. So the sidebar rendered its "GEMMA
filters" title over an empty body: no console error, no build error, no failing
test. Verified against the SHIPPED dist of @conduction/nextcloud-vue
1.0.0-beta.213, not only its src/.

The fix passes what the component actually declares. `buildFacetDimensionSchema`
(src/utils/facetSchema.js) builds the schema document `filtersFromSchema` reads
— `facetable: true` per property, `title` for the label, `order` for the
sequence — so the four dimensions become four selects whose options come from
the live facet counts this feature already fetches.

tests/vitest/facetSchema.spec.js pins that contract against the REAL
`filtersFromSchema` imported from the installed package, not a local copy of
its rules — a copied rule set is only as fresh as its last manual edit and
fails in both directions. One test is a positive control: it feeds the
pre-fix shape (the derived filter LIST, no `properties` key) to the real
function and asserts it yields [], which is the defect reproduced.

Also corrected in the e2e suite: the PortfolioReport assertion targeted
[data-testid=pr-summary], which sits behind `v-else-if="selectedOrg && report"`.
On an instance with no organisation selected the page correctly renders its
empty state, so that assertion was asserting on seed data rather than on the
page. It now asserts the unconditional "Refresh report" control plus whichever
of the page's two legitimate states is showing.

vitest 220/220 (5 new). check:manifest Ajv PASS. gates 25 and 26 PASS.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ 5f4aa92

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue-demi ✅
test-l10n ✅
composer ✅ ✅ 128/128
npm ✅ ✅ 718/718
PHPUnit ✅
Newman ⏭️
Playwright ✅
Hydra gates ❌

Quality workflow — 2026-08-09 20:33 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit f0bea90 into development Aug 9, 2026
27 of 29 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/contract-and-visual-coverage branch August 9, 2026 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@rubenvdlinde