Skip to content

fix(e2e): repair the Cypress suite (36 failures → 0) and the reader bug it was catching - #268

Merged
HugoFara merged 1 commit into
developfrom
fix/e2e-suite-green
Aug 6, 2026
Merged

fix(e2e): repair the Cypress suite (36 failures → 0) and the reader bug it was catching#268
HugoFara merged 1 commit into
developfrom
fix/e2e-suite-green

Conversation

@HugoFara

@HugoFara HugoFara commented Aug 5, 2026

Copy link
Copy Markdown
Owner

The Cypress suite had 36 failures across 9 specs on develop. This takes it
to 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's data_hex. That was legitimate when
data_hex held a reversible hex encoding of the word — but #237 (shipped in
3.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:

before:  TEXT="e6967a5826fe441a 75e3090289e2955d"  SENT="NOTAS DEL TRANSCRIPTOR"
after:   TEXT="NOTAS DEL"                          SENT="{NOTAS DEL} TRANSCRIPTOR"

It now reads data_text (set on multi-word spans, whose own text content is a
word 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

Spec Reality the test had not caught up with
05-texts /texts/new is a two-step wizard; language comes from the navbar
07-admin /admin/statistics redirects to /profile/statistics
04-languages Creating a language hands off to the starter-vocabulary step
04-languages The "Quick Setup Wizard" modal is now inline on /languages/new
06-words wordListApp is registered without parentheses
10-auth Login/register gained a language switcher, so button[type="submit"] matches two elements

The 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/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 "handlers are bound" signal.

Worth stating explicitly: I initially took this for an x-model bug and started
replacing 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. The
specs used bare cy.request() and got 403 — one even asserted 405 and
"passed" on the 403 for years. 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 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 reset
also reparses.

Verification

  • Cypress 215/215, twice consecutively
  • tsc clean, eslint clean, vitest 4419 pass
  • No PHP touched

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.
@HugoFara
HugoFara merged commit 416e654 into develop Aug 6, 2026
14 checks passed
@HugoFara
HugoFara deleted the fix/e2e-suite-green branch August 6, 2026 12:08
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