e2e: add missing .mtime marker to 4 magic-time wdio7 specs - #750
Open
IvanTheGeek wants to merge 1 commit into
Open
e2e: add missing .mtime marker to 4 magic-time wdio7 specs#750IvanTheGeek wants to merge 1 commit into
IvanTheGeek wants to merge 1 commit into
Conversation
The tyd runner groups specs for parallel vs serial runs by filename: specs named '.mtime' get run with --not-parallel, everything else is eligible for --parallel N (s/impl/tyd-e2e-tests.ts). But these four specs advance the server-global test clock — they call adminArea.review.playTimePastUndo(), which does server.playTimeSeconds(ReviewDecisionUndoTimoutSeconds + 10) — without carrying the marker. Under --parallel, they can shift time underneath unrelated concurrent specs: sessions expire, review-undo windows get skipped, notification timing skews. This is the failure mode the TODO in tyd-e2e-tests.ts (line 241) warns about: "Don't run magic time tests in parallel — they mess up the time for each other." Renamed (adding .mtime before .e2e.ts): alias-anons-approve-review.2br.f.e2e.ts bookmarks-basic.2br.f.e2e.ts modn-ban-spammer.2br.f.e2e.ts modn-review-specific-user.3br.f.e2e.ts Also updated: the describe() titles (which mirror the filenames), docs/tests-map.txt, the --only args in s/run-e2e-tests.sh (one used the full filename and would silently have stopped matching), and a spec-name comment in ModerationController.scala. The wdio6 suite needs no changes: its 25 clock-moving specs are exactly the 25 .mtime-named ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 tyd e2e runner decides parallel-vs-serial purely from the
.mtimefilename marker: marked specs run with--not-parallel, everything else is eligible for the parallel batch under--parallel N(s/impl/tyd-e2e-tests.ts, grouping around lines 273–285).Four wdio7 specs advance the server-global test clock without the marker — each calls
adminArea.review.playTimePastUndo(), i.e.server.playTimeSeconds(c.ReviewDecisionUndoTimoutSeconds + 10)(ty-e2e-test-browser.ts:10293):alias-anons-approve-review.2br.f.e2e.tsbookmarks-basic.2br.f.e2e.tsmodn-ban-spammer.2br.f.e2e.tsmodn-review-specific-user.3br.f.e2e.tsUnder
--parallelthey land in the parallel batch and can jump time underneath unrelated concurrent specs — expired sessions, skipped review-undo windows, skewed notification timing; scheduling-dependent flakiness. It's the failure mode the existing TODO warns about (tyd-e2e-tests.ts:241: "Don't run magic time tests in parallel — they mess up the time for each other").This PR just renames the four files to carry
.mtime, and updates the references so nothing dangles:describe()titles (they mirror the filenames),docs/tests-map.txt(8 lines),--onlyargs ins/run-e2e-tests.sh— one of them used the full filename (modn-ban-spammer.2br.f.e2e.ts) and would silently have stopped matching,ModerationController.scala.No code changes —
git show --statis 4 renames (99% similarity) + 17 reference lines.For contrast I checked the wdio6 suite too: its 25 clock-moving specs are exactly the 25
.mtime-named ones, zero drift (verified by grepping spec bodies forplayTime/pauseServerAndBrowsersand diffing against filenames). A longer-term hardening idea: have the runner grep spec bodies forplayTimeinstead of trusting filenames, so future drift can't reintroduce this.Found while analyzing which e2e specs can run in parallel — write-up: https://forum.ivanthegeek.com/-206
🤖 Generated with Claude Code