Skip to content

fix(e2e): the organisation step is seeded now - #1503

Closed
rubenvdlinde wants to merge 1 commit into
developmentfrom
fix/organisation-step-is-seeded-now
Closed

fix(e2e): the organisation step is seeded now#1503
rubenvdlinde wants to merge 1 commit into
developmentfrom
fix/organisation-step-is-seeded-now

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Fixes the E2E failure on development (1 failed, 3 flaky, 303 passed).

The failure

First-time setup contract › setup status reports every step, and only currency gates completion
  no organisation name is configured in CI
  Expected: false
  Received: true

Cause — CI changed on purpose, the assertion did not

ci-seed.sh step 3b now sets receipt_company_name, added in #1480 at 08:57 today. Its own comment says why:

CnAppRoot opens the non-gating wizard when ANY optional step that is not info or summary is reported outstanding — not just demo-data. Steps 1-3 clear provision, currency and demo-data, and leave organisation behind … So the wizard could still auto-open as a modal mask in every fresh browser context.

SetupController derives organisation.done from receipt_company_name !== '', so the seed closing that step is exactly what flipped this assertion. The test still described the old world.

The change

The assertion follows the seed, and the reason is recorded next to it so the next reader does not read it as a weakened test.

What I am flagging rather than hiding

This spec used to witness "completion is true DESPITE an outstanding optional step"organisation was the one deliberately-undone step that made the point. With every optional step now satisfied by the seed, that reading is vacuous here.

The property is real and still worth covering. It needs a test that owns the config it depends on instead of reading CI's ambient state — the same lesson as any test that inherits state rather than establishing it. I have not written that test in this PR because it is a different piece of work and would want its own decision about whether to mutate config mid-suite (clearing receipt_company_name re-opens the wizard mask, which is precisely what #1480 was closing).

The ⚠️ comment in the file says this, so the gap is visible in the code rather than only here.

Verified: node --experimental-strip-types --check clean; pushed file byte-identical.

Surfaced once development runs started completing again — ConductionNL/.github#597, #625.

E2E fails on development:

  First-time setup contract — setup status reports every step, and only
  currency gates completion
  no organisation name is configured in CI
  Expected: false   Received: true

That is no longer true of CI. ci-seed.sh step 3b sets
receipt_company_name deliberately (#1480, 08:57 today): CnAppRoot opens
the non-gating wizard when ANY optional step is outstanding, so leaving
organisation undone covered the shell with a modal mask in every fresh
browser context, and the seed had to close it.

The assertion follows the seed. Recorded in the same breath: with every
optional step now done, this spec no longer witnesses completed is true
DESPITE an outstanding optional step. That property is real and needs a
test owning its own config rather than reading CI state.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/pipelinq @ bad7446

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue-demi
test-l10n
format
check-schema-l10n
check-l10n-js
composer ✅ 106/106
npm ✅ 645/645
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright 🚨 NO VERDICT — enabled but never ran
Hydra gates

Quality workflow — 2026-08-28 13:28 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Closing as superseded — and by a better fix than this one.

pipelinq#1502 "an optional step is not a gate, so stop pinning its value" landed on development at 13:25, while this PR was in CI.

Same diagnosis: ADR-111 demo-data generation (#1480) made ci-seed.sh set receipt_company_name, so organisation.done flipped to true and the old .toBe(false) assertion went stale.

Different, better remedy. This PR re-pinned the assertion to true, and I flagged in its body that doing so left the spec no longer witnessing "completed is true despite an outstanding optional step" — it would just track whatever CI happens to seed. #1502 stops pinning the value at all:

expect(
    typeof res.json.steps.organisation.done,
    'the organisation step reports a boolean either way',
).toBe('boolean')

That asserts the actual contract — an optional step reports its own state and does not gate completion, whichever way it lands — and removes the dependence on ambient CI state that made this brittle in the first place. It is the fix I described as needed and did not write.

GitHub also reported this branch dirty against the new base, so merging it would only have replayed the same change through a conflict.

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