fix(e2e): the same ambiguous locator, one file over - #645
Merged
rubenvdlinde merged 1 commit intoSep 7, 2026
Merged
Conversation
#642 fixed this strict-mode violation in `tests/e2e/workflows/vault-unlock.spec.ts` and left the identical locator in `tests/e2e/spec-coverage/lock-screen.spec.ts` untouched, so keepiq's development E2E stayed red on the same defect: strict mode violation: locator('.lock-screen') .getByText(/Wrong master password|decryption failed/i) resolved to 2 elements: 1) <p role="alert" class="lock-screen__sr-live"> 2) <p class="input-field__helper-text-message"> I reported keepiq as fixed after #642 merged. It was not: I had fixed the spec that failed rather than the LOCATOR that was wrong, and the second copy failed the very next run on a tree containing the fix. Same treatment as #642, so the two files now agree. Each surface is asserted on its own: the helper text is what a sighted reader sees, the visually-hidden role="alert" is what a screen reader announces, and it was added deliberately so a rejected credential interrupts rather than going unspoken. Narrowing to either one alone would let the other be removed with no test noticing, which for the live region means losing the announcement silently. The two remaining uses in `tests/e2e/workflows/_workflow-helpers.ts` are `.count()` and `.first().textContent()`. Neither is strict-mode sensitive, so they are correct as they stand and are left alone. prettier clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/keepiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 111/111 | |||
| npm | ✅ | ✅ 543/543 | |||
| 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-07 18:28 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
deleted the
fix/the-lock-screen-spec-has-the-same-ambiguity
branch
September 7, 2026 18:34
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.
keepiq's development E2E is still red on the strict-mode violation #642 was supposed to have fixed.
What I got wrong
#642 fixed the ambiguous locator in
tests/e2e/workflows/vault-unlock.spec.ts. The identical locator also lives intests/e2e/spec-coverage/lock-screen.spec.ts, and I did not touch it. I then reported keepiq as fixed. It was not: the run at6c09423, on a tree that contains #642, failed with the same message one file over.I fixed the spec that failed rather than the locator that was wrong.
The fix
Same treatment as #642, so the two files now agree. Each surface is asserted on its own:
role="alert"is what a screen reader announces, added deliberately so a rejected credential interrupts rather than going unspokenNarrowing to either one alone would let the other be removed with no test noticing, which for the live region means losing the announcement silently — the exact failure the region exists to prevent.
What I checked before assuming
tests/e2e/workflows/_workflow-helpers.tshas two more copies of the same regex. They are.count()and.first().textContent(), neither of which is strict-mode sensitive, so they are correct as they stand and are left alone.Verification
prettier clean. The E2E job is
skippingon a pull request and runs only on the development push, so this is proven on the merge or not at all.🤖 Generated with Claude Code