fix(e2e): repair the Cypress suite (36 failures → 0) and the reader bug it was catching - #268
Merged
Merged
Conversation
The suite had 36 failures across 9 specs on develop. Most were stale selectors, but two were real defects and one whole family was asserting nothing at all. Real bug: multi-word selection captured hashes, not words --------------------------------------------------------- getWordSurface() returned the span's data_hex. That was legitimate when data_hex held a reversible hex encoding of the word, but #237 (3.2.0) made it a SHA-256 derived identity token, and this call site was never updated. Selecting two words therefore created a term literally named "e6967a5826fe441a 75e3090289e2955d", and the sentence lost its {...} markers because the wrapper needs the surface to find the term. Now reads data_text (set on multi-word spans, whose own text is a count in "show all" mode) and falls back to the element's text nodes. Covered by a unit test that fails without the fix. Dead link: the review table's "edit term" button is fixed on the #266 branch; unrelated to this change. Tests that were asserting nothing --------------------------------- Six 04-languages tests were wrapped in `if (find('.language-card').length > 0)`. That class has not existed since the list became a table, so the guard was never true and the bodies never ran. Rewritten against the real markup, with the row selector defined once so the next redesign is a one-line fix rather than silent rot. Stale expectations corrected to match deliberate app changes ------------------------------------------------------------ - /texts/new is a two-step wizard and takes its language from the navbar - /admin/statistics redirects to /profile/statistics (per-user stats) - creating a language hands off to the starter-vocabulary step - the languages "Quick Setup Wizard" modal is now inline on /languages/new (the six modal tests are dropped; "Embedded Wizard" already covers the real UI, extended here with the end-to-end save the old tests had) - wordListApp is registered without parentheses - login/register pages gained a language switcher, so `button[type="submit"]` matches two elements — scoped to method="POST" Races, not bugs --------------- Several failures looked like broken Alpine bindings. They were not: the markup is server-rendered, so a click can land before Alpine binds, and @alpinejs/csp drops it silently with no console error. Waiting for x-cloak to be stripped (or for :disabled to have been applied) is the real readiness signal. Verified by reverting a speculative fix and confirming the original x-model markup passes with the gate. CSRF ---- State-changing API requests without a Bearer token require X-CSRF-TOKEN; the specs used bare cy.request() and got 403. Added cy.apiRequest(), which reads the token from <meta name="csrf-token"> exactly as @shared/api/client does. Re-runnability -------------- The multi-word tests created a term that broke their own precondition on the next run, so the suite passed at most once per fresh database. Deleting the term is not enough — the multi-word *span* survives in the text's parsed items with wordId=null — so the reset reparses too. Result: 215/215 passing, green on two consecutive full runs. Gates: tsc clean, eslint clean, vitest 4419 pass.
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.
The Cypress suite had 36 failures across 9 specs on
develop. This takes itto 215/215, green on two consecutive full runs.
Most were stale selectors, but two were real defects and one whole family was
asserting nothing at all.
A real bug: multi-word selection captured hashes instead of words
getWordSurface()returned the span'sdata_hex. That was legitimate whendata_hexheld a reversible hex encoding of the word — but #237 (shipped in3.2.0) made it a SHA-256 derived identity token, and this call site was never
updated.
Selecting two words in the reader therefore created a term literally named
e6967a5826fe441a 75e3090289e2955d, and the example sentence lost its{...}markers, because the wrapper needs the surface text to locate the term.
Confirmed against a running instance before and after:
It now reads
data_text(set on multi-word spans, whose own text content is aword count in "show all" mode) and falls back to the element's text nodes. A
unit test covers it and fails without the fix.
Six tests that asserted nothing
The 04-languages tests were wrapped in
if (find('.language-card').length > 0).That class has not existed since the list became a table, so the guard was never
true and the bodies never executed — they passed without testing anything, which
is worse than failing. Rewritten against the real markup, with the row selector
defined once so the next redesign is a one-line fix rather than silent rot.
Stale expectations corrected to match deliberate app changes
/texts/newis a two-step wizard; language comes from the navbar/admin/statisticsredirects to/profile/statistics/languages/newwordListAppis registered without parenthesesbutton[type="submit"]matches two elementsThe six obsolete wizard-modal tests are dropped: the existing "Embedded Wizard"
block already covers the real UI, and I extended it with the end-to-end
create-and-verify-persistence case the deleted tests provided.
Races, not broken bindings
Several failures looked like broken Alpine bindings — clicks that did nothing.
They were not. The markup is server-rendered, so a click can land before Alpine
binds the handler, and
@alpinejs/cspdrops it silently, with no consoleerror. Waiting for
x-cloakto be stripped (or for:disabledto have beenapplied) is the real "handlers are bound" signal.
Worth stating explicitly: I initially took this for an
x-modelbug and startedreplacing the install-demo page with a registered component. I then reverted
that and re-tested the original markup behind the readiness gate — it passes
untouched, so the app change was unnecessary and is not in this PR.
CSRF
State-changing API requests without a Bearer token require
X-CSRF-TOKEN. Thespecs used bare
cy.request()and got 403 — one even asserted405and"passed" on the 403 for years. Added
cy.apiRequest(), which reads the tokenfrom
<meta name="csrf-token">exactly as@shared/api/clientdoes.Re-runnability
The multi-word tests created a term that broke their own precondition on the
next run, so the suite could pass at most once per fresh database. Deleting the
term is not sufficient — the multi-word span survives in the text's parsed
items with
wordId=null, so the reader keeps rendering it — hence the resetalso reparses.
Verification
tscclean,eslintclean,vitest4419 pass