fix(e2e): the cleanup must restore the seed, not clear it - #1513
Merged
rubenvdlinde merged 1 commit intoAug 28, 2026
Merged
Conversation
The finally block clears receipt_company_name and asserts done === false, under the comment leave the instance exactly as found. It is the opposite: ci-seed.sh step 3b sets that key precisely so no optional step is outstanding, and its own error message says why -- An unmet optional step makes CnAppRoot cover the shell with the wizard in every fresh browser context. So this cleanup handed that modal mask to every spec running afterwards. In the run this test last failed in, dashboard.spec.ts was flaky three ways: renders the page, renders the KPI and chart widgets, offers the quick-create actions -- which is what a modal over the shell produces. Restore the seeded value instead, and assert DONE rather than not-done. There is no GET for the setup config, only /status which returns booleans, so the literal is coupled to the seed and documented as such.
Contributor
Quality Report — ConductionNL/pipelinq @
|
| 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 19:43 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
deleted the
fix/cleanup-must-restore-the-seed-not-clear-it
branch
August 28, 2026 19:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1509 (bba1a91), which fixed the precondition half of this test. This is the other half, still live on
development.The cleanup does the opposite of what it says
ci-seed.shstep 3b sets that key precisely so no optional step is outstanding, and its own error message states the consequence:So this
finallyhands that modal mask to every spec that runs afterwards — while assertingdone === false, i.e. asserting that it left the instance in the bad state.The evidence it is already costing something
In the run this test last failed in,
dashboard.spec.tswas flaky three ways:renders the dashboard page for the default routeevery required KPI and chart widget is renderedthe quick-create actions are offeredThree specs about rendering the shell, intermittently failing, in a suite where one test deliberately leaves a modal able to cover the shell. That is consistent enough to act on, and cheap to act on.
The change
Restore the seeded value rather than clearing it, and assert
done === true— the state ci-seed established.The literal, and why it is one
There is no
GETfor the setup config — only/api/setup/status, which returns booleans — so the original value cannot be read back and restored dynamically.'CI Test Organisation'is the stringci-seed.shwrites, and the comment says so: if the seed changes, this changes with it. A documented coupling is better than a cleanup that silently re-opens the wizard.Scope
I did not touch the flaky dashboard specs. If this diagnosis is right they settle on their own; if they do not, they are a real defect and deserve investigating as one rather than being pre-emptively patched.
Verified:
node --experimental-strip-types --checkclean; built on currentdevelopment(so it carries #1509's precondition fix rather than conflicting with it); pushed file byte-identical.