Skip to content

Cover in the browser what jest was covering alone, and drop those cases - #2240

Merged
umputun merged 2 commits into
masterfrom
e2e-covers-more
Sep 9, 2026
Merged

umputun merged 2 commits into
masterfrom
e2e-covers-more

Conversation

@paskal

@paskal paskal commented Aug 30, 2026 •

Copy link
Copy Markdown
Collaborator

#2239 removed the jest cases the browser suite already asserted. These are the ones it could have asserted and did not: the states behind a held-open request, the actions an ordinary reader is offered, an order nothing read, and the values inside elements the suite only ever waited for.

Eighteen jest cases go, 340 to 322. Two commits on master: the cases, and the Back step below.

The rule, and why coverage is not it

Each removed case is released by a named browser assertion, and each of those is proven by a mutant that makes it fail at the assertion it names. Two cases can execute identical lines and assert different things, so a coverage diff cannot tell a real replacement from a coincidental one.

Where a trap can be tripped again, the warning sits beside the code that would trip it: the read-only case says why it posts a comment first, the profile case why its route is a regexp, the countdown why its bound allows a second, and the vote case why the assertion that looks like it belongs there is not there.

What was added

browser work releases
TestComment_ActionsDependOnWhoseCommentItIs 3
TestVote_BothButtonsAreDisabledWhileTheVoteIsInFlight 2
TestTelegramSub_AFailed{Check,Unsubscribe}IsClearedByASuccessfulOne 2
TestProfile_LoadingAndFailureStates 2
Copy reporting back, folded into the action-order case 2
TestComment_AdminActionsKeepTheirOrder 1
TestComment_EditCountdownCountsDown 1
telegram control absent where notifications are off 1
email control absent where notifications are off 1
Reply absent on a read-only thread 1
the verification icon before anyone verifies 1
Edit becoming Cancel while editing 1
TestTelegramSub_ThePanelSaysItIsWorking —
the profile comment count —
TestSubscribe_BackFromTheTokenStepKeepsThePanelOpen —

The first two of those add coverage and release nothing: every jest case touching them also asserts a call count or another thing the browser does not.

The two notification-flag assertions cost no stack time. The main instance runs NOTIFY_USERS=email and the Telegram instance the inverse, so each is already the other's negative case, and both were folded into cases that load those instances anyway.

Four assertions that proved nothing

The interesting half. Each of these passed, and would have shipped as a proof:

  • Delete absent on another reader's comment held with the ownership test removed, because editDeadline is supplied only for the reader's own comment, so the weakened condition is equivalent on the comment under test.
  • Reply absent on a read-only thread held because that case runs on a thread with no comments in it. It now posts one and asserts Reply is offered before the thread is disabled.
  • The profile states passed with the route never matching: **/api/v1/comments?user=** is a glob, and ? is a single-character wildcard, so the request was never intercepted and simply succeeded. It uses a regexp now.
  • A cast downvote staying disabled held with its guard removed, because the in-flight guard disables that button anyway. This one could not be fixed, so the assertion was withdrawn and its jest case kept.

Three were fixable. One was not. A coverage check would have been green for all four.

Two code changes

Counter gains a class outside the CSS modules. It carried only a hashed module name and a data-testid the production bundle strips, which is what kept the count out of reach of any browser case.

setEmailStep no longer defers its step change behind a zero timeout. That deferral existed to dodge the dropdown closing on the click: the rerender removed the Back button before the click reached the document's bubble-phase listener, which found no target inside the panel and closed it over the email form. With that listener in the capture phase the deferral does nothing, so it is gone, and Back on the token step is now the one flow in the widget where a click detaches its own control inside the click's task.

TestSubscribe_BackFromTheTokenStepKeepsThePanelOpen pins the phase on that flow: submit an address, click Back, and the panel has to stay open on the email step. With the listener moved back to the bubble phase and the image rebuilt, it fails at that assertion with both clicks passing first, which the earlier synthetic case could show only by removing the target itself.

Review

Both reviewers went over this twice. Every finding is fixed; the ones worth knowing about are the assertions that could not fail, because each looked like a check and was not.

Could not fail, and were rewritten or removed

  • the post-release wait in the in-flight vote case was true on both sides of the release: the clicked button is disabled first by the in-flight guard and then by the cast vote, so the poll returned immediately. It waits on the score now.
  • two preloader-absence checks sat behind a state that already implied them, since the telegram panel renders the preloader and the settled state from the same flag. Removed.
  • the profile's negative assertions passed against any mutant, because the heading lives inside commentsJSX's comments?.length branch and never renders while the list is out anyway. They are now proven by rendering the heading unconditionally, and by keeping the preloader alive through the failure.
  • the countdown had an upper bound and no lower one, so a timer starting at 2s satisfied it. A unit-scaling mutant now fails it: "the countdown started 14s below the edit window, having spent 0s getting there".

Correctness and coverage

  • the profile replacement had dropped the heading and Load more absences the removed jest cases asserted. Both restored, in both states.
  • TextContent() inside an eventually closure carried playwright's 30s default instead of pollText's 1s, overshooting the loop's budget and reporting the wrong failure.
  • the countdown's floor was a fixed window - 2, which is scheduler-sensitive at -parallel 4. It now derives from time actually spent since the comment was posted.
  • the Copied! comment claimed the assertion separates a working copy from a silent no-op. It does not: copyComment sets isCopied outside its own catch.

A CI failure this introduced

The read-only case posted a comment containing the words "read-only", and text=Read-only matches case-insensitively, so the status locator matched both the status and the comment. Two matches is a strict-mode failure. It passed locally because the comment rendered below the fold, where the IntersectionObserver leaves the article empty, and failed in CI where it did not. The text can no longer collide and the status is matched exactly.

Tidying: an empty describe('verification') husk, an orphaned comment this same commit falsified, a comment detached from the statement it explained, the Selectors list missing the two new hooks, and the backlog work-list still presenting as open the items this change completes.

Codecov

codecov/project passes. Removing jest cases lowers the frontend number while the widget coverage that replaces it is measured by make e2e-cover and never uploaded, so the threshold added in #2239 gives the project status half a percent of slack. Uploading the widget lcov from the e2e workflow under its own flag, so codecov unions it with jest on the same commit, remains a workflow change this PR deliberately does not make.

Copilot AI lite review requested due to automatic review settings August 30, 2026 23:18
@paskal
paskal requested a review from umputun as a code owner August 30, 2026 23:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.01%. Comparing base (34fe625) to head (1169a73).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2240      +/-   ##
==========================================
- Coverage   74.10%   74.01%   -0.09%     
==========================================
  Files         129      129              
  Lines        3746     3745       -1     
  Branches      829      840      +11     
==========================================
- Hits         2776     2772       -4     
- Misses        964      967       +3     
  Partials        6        6              
Flag Coverage Δ
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

size-limit report 📦

Path Size
public/embed.mjs 2.05 KB (0%)
public/remark.mjs 53.82 KB (-0.01% 🔽)
public/remark.css 7 KB (-0.03% 🔽)
public/last-comments.mjs 17.37 KB (+0.01% 🔺)
public/last-comments.css 3.22 KB (0%)
public/deleteme.mjs 2.98 KB (0%)
public/counter.mjs 731 B (0%)

@paskal
paskal force-pushed the e2e-covers-more branch 6 times, most recently from 95a9d6e to 9c3ecf0 Compare September 4, 2026 09:26
@paskal
paskal force-pushed the e2e-covers-more branch 2 times, most recently from 064bec1 to 4256323 Compare September 7, 2026 21:45
The previous change removed the jest cases the browser suite already asserted.
These are the ones it could have asserted and did not: the states behind a
held-open request, the actions an ordinary reader is offered, an order nothing
read, and the values inside elements the suite only waited for.

Eighteen jest cases go. Each is released by a named browser assertion, and each
of those is proven by a mutant that makes it fail at the assertion it names,
recorded in the pull request with the mutant and the expected failure point. Line
coverage is not the check here: two cases can run identical lines and assert
different things.

That discipline caught four assertions of mine that proved nothing. Delete
being absent on another reader's comment held with the ownership test removed,
because editDeadline is supplied only for the reader's own comment and the
weaker condition is equivalent. Reply being absent on a read-only thread held
because the thread had no comments in it. The profile states passed with the
route glob never matching, since a question mark is a single-character wildcard
in playwright's url matching. And a cast downvote staying disabled held with
its guard removed, because the in-flight guard covers it either way; that one
could not be fixed, so the assertion was withdrawn and its jest case kept.

Where one of those traps can be tripped again, the warning sits beside the code
that would trip it: the read-only case says why it posts a comment first, the
profile case says why its route is a regexp, the countdown says why its bound
allows a second, and the downvote case says why the assertion that looks like
it belongs there is not there.

The counter gets a class outside the css modules. It carried only a hashed name
and a data-testid the production bundle strips, which is what kept the count
out of reach of a browser case.
setEmailStep deferred its step change behind a zero timeout. That existed
to dodge the dropdown closing on the click: the rerender removed the Back
button before the click reached the document's bubble-phase listener,
which then found no target inside the panel and treated the click as
outside. With that listener in the capture phase the deferral does nothing,
so it goes, and the flow becomes the one place in the widget where a click
detaches its own control inside the click's task.

That is the flow the capture listener exists for, so it is now the browser
case that pins it: submit an address, click Back on the token step, and the
panel has to stay open on the email step. With the listener moved back to
the bubble phase the case fails at that assertion, with the two clicks
before it passing.
@umputun
umputun merged commit d9254d2 into master Sep 9, 2026
15 checks passed
@umputun
umputun deleted the e2e-covers-more branch September 9, 2026 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants