fix(e2e): 26 skips carried their reason in a comment the report never saw - #516
Merged
Merged
Conversation
added 2 commits
August 28, 2026 15:18
… saw
The skip-discipline gate reports 39 V3 findings here — "a skip or fixme
with no reason recorded". Every one of those 26 addressed in this commit
already HAD a reason, and a good one. None of them reached the Playwright
report, which is the only place the gate (or anyone) can read it.
`test.skip('title', fn)` is the STATIC form: it records no reason at all.
The reasons were sitting in comments, some above the declaration and some
inside the body, saying things worth keeping:
"SPEC DRIFT, not an environment limitation. The requirement describes ONE
tabbed editor with Design (default) and Raw JSON as sibling tabs. The
shipped app has no such pair…"
"STUB BODY — needs real assertions. The buildiq#41 quarantine is stale:
#41 MERGED 2026-07-27 and the builder UI renders."
Each is lifted into `test.skip(true, '<reason>')`, the form the report
carries.
buildiq-runtime 11
nldesign-theme-selection 6
docudesk-document-templates 5
application-detail-overview 2
buildiq-rbac 2
NOTHING about what runs changes. The same 26 tests are excluded, for the
same reasons, and the comments stay where they are.
That property is the whole risk in this edit, so it is asserted rather
than hoped for. Converting `test.skip(title, fn)` to `test(title, fn)`
and then failing to insert the annotation SILENTLY STARTS RUNNING a test
that was excluded on purpose — and it still compiles, so `--list` cannot
catch it. The transform aborts unless every converted test gains an
annotation, and it did abort on four files whose reasons lived above the
declaration rather than inside it, until it was taught to look there.
Verified:
static `test.skip('…')` remaining 0
annotations added 26
diff lines that are neither a declaration nor an annotation: 0
npm ci rc=0 · prettier clean · lint rc=0
`playwright test --list` 272 tests across 55 files
Remaining here after this: 13 V3 in files using other forms, 2 V2, and
5 V1 spec files that execute nothing. Part of ConductionNL/.github#609.
The first pass converted 26 static `test.skip('title', fn)` calls. These
are the rest, which used forms that pass did not match:
test.describe / .skip(...) promoteDestructive 1 split across two
lines by prettier,
so searching for
`test.describe.skip(`
finds nothing
test.describe.skip(...) export-zip 1
test.fixme(title, fn) virtual-app-crud 2
schema-crud 1
INDENTED test.skip(...) bootstrap-buildiq 1 inside a describe,
chat-companion 1 so a column-0 anchor
missed them
Their reasons were as good as the first batch's, and equally invisible to
the report:
"MOVED TO NEWMAN: asserts on the manifest API response, not the UI. The
API/contract is covered by tests/integration/*.postman_collection.json"
"the call site that opens PromoteVersionDialog does not exist: every
scenario here reaches it through TODO_PROMOTE_BUTTON_SELECTOR
(promote-version-btn), which appears nowhere in src/"
Final state of the branch, with comments excluded from every count so a
comment mentioning `describe.skip` cannot be mistaken for a call:
reasonless static test.skip('…') 0
reasonless static test.fixme('…') 0
reasonless test.describe.skip(…) 0
attributed annotations 40
diff lines that are neither a declaration nor an annotation: 0
272 tests across 55 files, unchanged · lint rc=0 · prettier clean
NOT addressed, deliberately: the 5 V1 spec files that execute zero tests.
hydra-console is the clearest and is NOT a spec defect — its guard is a
real capability probe carrying a full reason, and it skips because
`hydra-cache` (register 2512) is not provisioned on the CI instance. That
is a seeding gap; closing it by editing the spec would be the wrong fix.
Contributor
Quality Report — ConductionNL/buildiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 642/642 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-28 13:25 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/buildiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 642/642 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-28 13:39 UTC
Download the full PDF report from the workflow artifacts.
Two tests in chat-companion-streaming.spec.ts stood down on:
Streaming surface not yet wired — see openspec/changes/ai-chat-companion-streaming/
That path did not exist. Not in this repo, not in .github, openregister or
hermiq, and a fleet-wide code search finds the string referenced only by
the skip message citing it. The tests were deferred to a tracker nobody
had created, so nothing could ever close them.
They also assert nothing: both bodies are empty, carrying a comment
describing the intended assertion and no code. Enabling them as they
stand would light up two green ticks for assertions never written.
The hydra skip-discipline gate calls both V2 — "assert it or drop the
test, do not stand down" — and it is right. This is the missing half: the
work written down, so the exclusion points at something real.
openspec/changes/ai-chat-companion-streaming/
proposal.md why, and what the empty bodies were meant to prove
specs/ai-chat-companion/spec.md
two requirements: incremental delivery (with a
non-streaming fallback scenario, since a provider
without generateStreamOfText must still answer), and a
heartbeat so a slow call is distinguishable from a hung
one
tasks.md including 1.1/1.2, which say plainly that the assumed
provider shape and the 15s/35s heartbeat pairing in the
old test comments are GUESSES to be measured, not
carried forward
The skip stays, and task 4.3 says why: it may only be removed once 4.1
and 4.2 give those bodies real assertions.
The reason is rewritten to describe the actual state. The old wording
tripped the gate's DEPLOY_STATE phrase "not yet wired"; the new one is
checked against that list and classifies as allowed — verified, because
rewriting a reason without checking would have left it a V2.
Verified: prettier clean; `playwright test --list` 272 tests across 55
files, unchanged.
Contributor
Quality Report — ConductionNL/buildiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 642/642 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-28 14:13 UTC
Download the full PDF report from the workflow artifacts.
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
1 participant
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.
The skip-discipline gate reports 39 V3 findings here — "a skip or fixme
with no reason recorded". Every one of those 26 addressed in this commit
already HAD a reason, and a good one. None of them reached the Playwright
report, which is the only place the gate (or anyone) can read it.
test.skip('title', fn)is the STATIC form: it records no reason at all.The reasons were sitting in comments, some above the declaration and some
inside the body, saying things worth keeping:
"SPEC DRIFT, not an environment limitation. The requirement describes ONE
tabbed editor with Design (default) and Raw JSON as sibling tabs. The
shipped app has no such pair…"
"STUB BODY — needs real assertions. The buildiq#41 quarantine is stale:
#41 MERGED 2026-07-27 and the builder UI renders."
Each is lifted into
test.skip(true, '<reason>'), the form the reportcarries.
buildiq-runtime 11
nldesign-theme-selection 6
docudesk-document-templates 5
application-detail-overview 2
buildiq-rbac 2
NOTHING about what runs changes. The same 26 tests are excluded, for the
same reasons, and the comments stay where they are.
That property is the whole risk in this edit, so it is asserted rather
than hoped for. Converting
test.skip(title, fn)totest(title, fn)and then failing to insert the annotation SILENTLY STARTS RUNNING a test
that was excluded on purpose — and it still compiles, so
--listcannotcatch it. The transform aborts unless every converted test gains an
annotation, and it did abort on four files whose reasons lived above the
declaration rather than inside it, until it was taught to look there.
Verified:
static
test.skip('…')remaining 0annotations added 26
diff lines that are neither a declaration nor an annotation: 0
npm ci rc=0 · prettier clean · lint rc=0
playwright test --list272 tests across 55 filesRemaining here after this: 13 V3 in files using other forms, 2 V2, and
5 V1 spec files that execute nothing. Part of ConductionNL/.github#609.