Skip to content

fix(e2e): the seeds create draft case types, and two stale assertions - #1944

Merged
rubenvdlinde merged 3 commits into
developmentfrom
fix/the-e2e-seeds-create-draft-case-types
Sep 8, 2026
Merged

fix(e2e): the seeds create draft case types, and two stale assertions#1944
rubenvdlinde merged 3 commits into
developmentfrom
fix/the-e2e-seeds-create-draft-case-types

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Ten of the twenty-three failing E2E tests on development. None of these is an
application defect: two behaviours changed on purpose and the tests had not
caught up, and one fixture was invalidated by a shipped schema change.

The other thirteen are separate. Three are the transition 500 in #1941, and ten
still need work, listed at the end.

Eight: the seeded case types are drafts

#1918 added x-relation-filter: {isDraft: false} to case.caseType, and the
caseType schema defaults isDraft to true. So every fixture that creates
a case type without naming the field creates a draft, and the New case picker
correctly refuses to offer it:

Locator: getByRole('option').filter({ hasText: 'E2EZAAK-mtsnt8sg-7325 Subsidie' })
Error: element(s) not found

That reads as a missing option, not as a draft, which is why it looked like
eight unrelated breakages.

Three seeding sites gain isDraft: false: case-create-form, case-identity,
and the shared ensureCaseType helper. The natural experiment is already in
the tree: every spec that already passes the flag has no missing-option
failure, namely seedStateMachine, dashboard-tiles, checklist-per-status
and case-types-tabs.

Excluding drafts from the picker is exactly what #1918 set out to do, and the
schema default means a real user's unpublished case type is correctly hidden
too. So the fixtures are what was wrong.

One: three elements now say Tags

tags became facetable and gained a column, so the index sidebar renders all
of these at once:

element what it is
.cn-index-sidebar__filter-label the filter label, what the test means
label.select__label that filter's own NcSelect label
.checkbox-content__text the Columns picker checkbox

All three are correct, and a bare getByText(/^(Tags|Labels)$/) is a strict
mode violation reporting "resolved to 3 elements". The assertion now names the
filter label class that CnIndexSidebar.vue renders, which is what the comment
above it always said it meant.

One: the guard refusal moved ahead of the POST, as the spec always asked

Scenario: Required document guard evaluation has always read:

THEN the transition button SHALL be disabled
AND the system SHALL display: "Vereist document ontbreekt: Besluit"

The test was clicking the button, confirming the dialog, and reading the error
out of the failed response. That is the behaviour the spec did not describe,
so the application is now right and the test was asserting the old shape.

It now asserts the disabled button and the reason beside it, and still asserts
the case has not moved, because a disabled button that transitioned anyway
would pass everything else.

That test also anchored at #a-failed-guard-keeps-the-case-where-it-is, which
matches no scenario in that spec. A broken anchor looks exactly like coverage.
It now points at the scenario that exists.

Verification

eslint and prettier are clean on all four files, and all four parse. The suite
itself needs CI: E2E runs only on the development push.

Still open, not in this PR

  • case-documents.spec.ts 503, 544, 611: an uploaded row never appears, with
    no server-side error anywhere in nextcloud.log.
  • case-list-lenses.spec.ts 281, 300, 320, 334: rows not assigned to the
    current user never show on the Cases index. The Mine lenses pass.
  • case-documents.spec.ts:697 and case-identity.spec.ts:231: two more
    application defects, a Generate document 500 and a case number overwritten on
    edit.

One process point worth deciding separately: 15 of the last 20 development
runs never reached E2E
, because the next merge cancelled them. Every cause
above shipped through that gap.

Ten of the twenty-three failing E2E tests on development, none of which is an
application defect.

EIGHT: the seeded case types are drafts. #1918 added
`x-relation-filter: {isDraft: false}` to `case.caseType`, and the caseType
schema defaults `isDraft` to TRUE. So every fixture that creates a case type
without naming the field creates a draft, and the New case picker correctly
refuses to offer it. The failure reads as a missing option rather than as a
draft, which is why it looked like eight unrelated breakages.

Three seeding sites gain `isDraft: false`: case-create-form, case-identity and
the shared `ensureCaseType` helper. The natural experiment is already in the
tree, because every spec that already passes the flag has no missing-option
failure: seedStateMachine, dashboard-tiles, checklist-per-status and
case-types-tabs.

ONE: three elements now say Tags. `tags` became facetable and gained a column,
so the index sidebar renders the filter label, that filter's own NcSelect
label, and the Columns picker checkbox. All three are correct and a bare
getByText is a strict mode violation. The assertion now names the filter label
class that CnIndexSidebar renders, which is what the surrounding comment always
said it meant.

ONE: the guard refusal moved ahead of the POST, and the spec always asked for
that. Scenario: Required document guard evaluation reads "the transition button
SHALL be disabled" and "SHALL display: Vereist document ontbreekt". The test
was clicking through to a dialog and reading the error out of a failed
response, which is the behaviour the spec did not describe. It now asserts the
disabled button and the reason beside it, and still asserts the case has not
moved, because a disabled button that transitioned anyway would pass
everything else.

That test also anchored at #a-failed-guard-keeps-the-case-where-it-is, which
matches no scenario in that spec. A broken anchor looks exactly like coverage.
It now points at the scenario that exists.

eslint and prettier clean on all four files.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ cb91457

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 ✅ 541/541
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 6f5026d into development Sep 8, 2026
46 of 48 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/the-e2e-seeds-create-draft-case-types branch September 8, 2026 15:01
rubenvdlinde added a commit that referenced this pull request Sep 8, 2026
`case.caseType` has carried `x-relation-filter: {isDraft: false}` since
#1918, and the caseType schema DEFAULTS `isDraft` to true. A spec that takes
`caseTypes[0]` blind can therefore hand a case a draft type, the relation
filters it out, and the case arrives without a usable case type.

That failure is invisible in the worst way HERE specifically: the cases list
on the contact page comes back empty, which reads as 'this person has no
cases' — the exact reading this spec exists to tell apart from a broken
filter. The assertion would have been green for the wrong reason on the
empty-person test and red without explanation on the seeded one.

#1944 fixed the fixtures that CREATE a case type. Six other specs still take
`caseTypes[0]` blind and are the same hazard; this is the one file this
change owns, and the failure message names the count it saw so the next
reader is not left guessing.
rubenvdlinde pushed a commit that referenced this pull request Sep 8, 2026
Completes the fix. Since #1918 `case.caseType` carries
`x-relation-filter: {isDraft: false}` and the caseType schema DEFAULTS
`isDraft` to true, so a case seeded against a draft type has no usable type:
the list comes back empty and reads as 'there is nothing here' rather than as
a bad fixture. #1944 fixed the fixtures that CREATE a type; the six specs that
ADOPT one were not covered.

Measured, not assumed: of 21 live case types on the dev rig 13 are drafts, so
blind [0] adoption picks one more often than not. The register import ships 6
of its 14 with the field ABSENT, which the schema default makes drafts.

The two filters had to compose or the change was a regression. Another
worker's seeded type is isDraft:false and would be a valid adoption; the
FIXTURE_PREFIX filter excludes it, and without this second half those six
specs would be pushed onto a DRAFT shipped type instead — making the
multi-worker case worse than the serial one it exists to enable.

`=== false` not `!== true`: absent counts as a draft, which is what the
schema default makes it.

The six assertion messages said 'must ship at least one case type', which this
filter makes misleading — an instance with 21 case types and no published one
would send a reader looking in the wrong place. They now name the filter.
rubenvdlinde added a commit that referenced this pull request Sep 8, 2026
…1984)

Seven sites take `listObjects(api, 'caseType')[0]` blind: `ensureCaseType`
plus six specs that inline the same adoption (case-communication,
case-detail-kpis-and-tabs, case-documents, case-objects, case-parties,
case-task-pane). Two things are wrong with that, and they compound.

DRAFTS. Since #1918 `case.caseType` carries
`x-relation-filter: {isDraft: false}`, and the caseType schema DEFAULTS
`isDraft` to true. A case seeded against a draft type has no usable type: the
list comes back empty and reads as 'there is nothing here' rather than as a bad
fixture. #1944 fixed the fixtures that CREATE a type; the six that ADOPT one
were not covered. Measured rather than assumed: of 21 live case types on the
dev rig 13 are drafts, so blind [0] picks an unusable one MORE OFTEN THAN NOT,
and the register import ships 6 of its 14 with the field absent.

FIXTURE-OWNED ROWS. A throwaway seeded by another run can be adopted as though
it were instance data (`seeded: false`, so it is never cleaned up) and then
deleted by its owner's teardown mid-test.

`adoptableCaseTypes()` excludes both. The conditions have to compose: filtering
only fixture-owned rows would push these specs onto a DRAFT shipped type
instead. `=== false` not `!== true`, because absent counts as a draft, which
is what the schema default makes it.

NOT 'let every caller seed its own type'. `case` is ARCHIVAL, so a seeded case
cannot be deleted in teardown, and a caseType that IS deleted therefore strands
permanent cases pointing at a type that is gone. Each of the six inline sites
already carries a comment saying exactly that.

Also: a per-call suffix on the two seeding paths, because RUN_PREFIX is unique
per PROCESS but not across two calls in one worker, so the second create reused
the first's identifier; and `isDraft: false` on seedStateMachine's caseType,
which #1944's message reports as already done but which only its
workflowTemplate had.

The six assertion messages said 'must ship at least one case type', which this
filter makes misleading: an instance with 21 case types and no published one
would send a reader looking in the wrong place. They now name the filter.

playwright.config.ts is COMMENT-ONLY. `workers: 1` is unchanged; its comment
described the adoption defect as the reason, which is no longer accurate, and
left as-is would invite someone to re-fix it. The new comment records that
raising it is unverified and what remains unmeasured.

Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
rubenvdlinde added a commit that referenced this pull request Sep 8, 2026
…st (#1986)

* fix(e2e): adopt a PUBLISHED case type, not whichever one sorts first

Seven sites take `listObjects(api, 'caseType')[0]` blind: `ensureCaseType`
plus six specs that inline the same adoption (case-communication,
case-detail-kpis-and-tabs, case-documents, case-objects, case-parties,
case-task-pane). Two things are wrong with that, and they compound.

DRAFTS. Since #1918 `case.caseType` carries
`x-relation-filter: {isDraft: false}`, and the caseType schema DEFAULTS
`isDraft` to true. A case seeded against a draft type has no usable type: the
list comes back empty and reads as 'there is nothing here' rather than as a bad
fixture. #1944 fixed the fixtures that CREATE a type; the six that ADOPT one
were not covered. Measured rather than assumed: of 21 live case types on the
dev rig 13 are drafts, so blind [0] picks an unusable one MORE OFTEN THAN NOT,
and the register import ships 6 of its 14 with the field absent.

FIXTURE-OWNED ROWS. A throwaway seeded by another run can be adopted as though
it were instance data (`seeded: false`, so it is never cleaned up) and then
deleted by its owner's teardown mid-test.

`adoptableCaseTypes()` excludes both. The conditions have to compose: filtering
only fixture-owned rows would push these specs onto a DRAFT shipped type
instead. `=== false` not `!== true`, because absent counts as a draft, which
is what the schema default makes it.

NOT 'let every caller seed its own type'. `case` is ARCHIVAL, so a seeded case
cannot be deleted in teardown, and a caseType that IS deleted therefore strands
permanent cases pointing at a type that is gone. Each of the six inline sites
already carries a comment saying exactly that.

Also: a per-call suffix on the two seeding paths, because RUN_PREFIX is unique
per PROCESS but not across two calls in one worker, so the second create reused
the first's identifier; and `isDraft: false` on seedStateMachine's caseType,
which #1944's message reports as already done but which only its
workflowTemplate had.

The six assertion messages said 'must ship at least one case type', which this
filter makes misleading: an instance with 21 case types and no published one
would send a reader looking in the wrong place. They now name the filter.

playwright.config.ts is COMMENT-ONLY. `workers: 1` is unchanged; its comment
described the adoption defect as the reason, which is no longer accurate, and
left as-is would invite someone to re-fix it. The new comment records that
raising it is unverified and what remains unmeasured.

* perf(e2e): run the suite in parallel, with instance-mutating specs last

The suite truncates at its 38 minute globalTimeout having produced a result for
111 of 371 tests. Fixing the red tests does not close that, and the arithmetic
is worth keeping because the opposite reading is intuitive and wrong. Measured
off the log timestamps of run 34244366521:

    111 tests produced a result in 37.6 min      20.3s each
    the 25 failures cost 18.5 min of that        22.2s x2 for the retry
    if every one became a ~5.1s pass             saves 16.4 min
    all 371 tests at the remaining rate          71 min SERIAL

So a fully green suite still runs at roughly twice the budget. Parallelism is
the only lever that closes the gap.

Also worth recording because it was circulating the other way: the cost is NOT
per-file setup. Of 37.6 minutes, 2.3 falls at spec-file boundaries and 35.2
between tests inside a file.

THREE WORKERS ON CI, ONE LOCALLY. fullyParallel stays false, so this
parallelises at FILE granularity: different files on different workers, tests
inside a file still in order on one worker. Several files build shared state in
beforeAll and read it across their tests, so the aggressive half of parallelism
is not what this suite needs. Three and not four because a SQLSTATE[53200]
out of shared memory was seen once under four; the extra six minutes buys
distance from a failure mode that gets blamed on the tests. One locally,
because a developer runs this against the shared dev instance.

The instance-mutating specs move to their own project that runs after the rest.
The hazard is asymmetric: a spec installing demo data or writing app settings
while empty-state assertions run elsewhere makes those fail and reads as a
product defect. The reverse costs nothing, so the list errs toward including a
spec.

The price is stated in the config rather than left to be discovered: Playwright
SKIPS a project whose dependency failed, so while anything in the parallel
project is red those 31 tests report as did not run.

Verified without touching the instance: the config loads, and --list resolves
371 tests in 67 files across 340 parallel and 31 serial, with all four
instance-mutating specs landing only in the serial project. eslint and prettier
clean. Whether the run COMPLETES is for CI, which is the standard this change
was asked to meet.

Stacked on #1984, whose adoptableCaseTypes() removes the cross-worker case-type
hazard that pinned this to one worker. This branch carries those commits, so CI
here exercises the combination.

* perf(e2e): let E2E_WORKERS override the worker count

Taken from the four-worker experiment on exp/e2e-four-workers, which had this
and is worth keeping whatever that run concludes: the next person to question
the number should not have to edit a config to answer it.

Default is unchanged, three on CI and one locally.

* perf(e2e): four workers, because the run says so

This started at three, to keep distance from a SQLSTATE[53200] out of shared
memory / max_locks_per_transaction that had been seen once under four. Both
counts have now been run against the same tree, so the caution can be settled
with a measurement instead of carried:

    workers   reached a verdict   passed   never ran   postgres locks
       1            144             108       227          -
       3            282             205        89          0
       4            309             230        60          0

Four reached more and the lock error did not reappear. One sighting against a
clean run is a weak argument, so the measurement wins.

The three runs are comparable, which matters because the decision rests on it:
each reports Running 371 tests, so the project split does not change what is
collected. The four-worker tally sums to 369 rather than 371 because two errors
fell outside any test, which both parallel runs report.

⚠️ FOUR WORKERS DOES NOT MAKE THE SUITE FIT and the config now says so. It still
truncates with about 60 tests unreached. The rest of the gap is the 44
failures, which cost roughly 32 of the 38 minutes because each is retried. That
closes as they are fixed, not by adding workers, and the next raise should come
behind a run rather than behind arithmetic.

---------

Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
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