fix(e2e): seed the current settings version in the theme spec - #277
Open
thetechjon wants to merge 1 commit into
Open
fix(e2e): seed the current settings version in the theme spec#277thetechjon wants to merge 1 commit into
thetechjon wants to merge 1 commit into
Conversation
The `folder-tree` boot assert in editor-selection-themes.spec.ts is not a timing flake. The spec seeds `noteser-settings` with `version: 2` while the store persists at 4, so every run replays the migration ladder, and v2 -> v3 with no legacy `pinnedPanels` falls back to a single calendar-only group (settingsStore.ts:1100). The Files panel -- and `folder-tree` with it -- is destroyed the moment persist rehydrates. Whether the test passes comes down to whether Playwright's first poll lands before rehydration, while the DEFAULTS two-group layout is still painted. That is why it fails on a different theme each run, passes locally, and survived the expect-timeout bump in #275. Confirmed locally with a scratch probe (removed) seeding each version and counting `folder-tree` after a 3s settle: version=2 -> 0 elements, groups=[{tabs:["calendar"]}] version=3 -> 1 element, groups=[calendar/plugins/search, files/...] version=4 -> 1 element, groups=undefined (DEFAULTS) Export SETTINGS_PERSIST_VERSION from the store and stamp that, so the next migration cannot silently re-break the spec. general-settings and nav-and-pin already seed v3 plus an explicit sidebarGroups record, so they were never exposed. Also reverts the 15s expect timeout from #275: it was based on the wrong diagnosis, it fixed nothing, and its comment now misleads. The html reporter from that PR stays -- its uploaded screenshot is what identified the calendar-only sidebar. Ports #276 (already merged to dev) onto main, which still carries the bug. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Root cause
CI run 30167489426 failed on
e2e/editor-selection-themes.spec.ts—getByTestId('folder-tree')not found (not merely slow) after 15s.It is not a timing flake.
seedSettingswritesnoteser-settingswithversion: 2, but the store persists at 4. So every run replays the migration ladder, andv2 → v3with no legacypinnedPanelshits the fallback atsrc/stores/settingsStore.ts:1100:The Files panel — and
folder-treewith it — is destroyed the moment persist rehydrates. The uploaded failure screenshot (thanks to the html reporter added in #275) shows exactly that: a sidebar with one Calendar group and no file tree.Whether the test passed came down to whether Playwright's first poll landed before rehydration, while the
DEFAULTStwo-group layout was still painted. That explains why it failed on a different theme each time ("sepia" here, "default" in 30158105758), passed locally, and survived the expect-timeout bump.Confirmed locally with a scratch probe (removed before commit) seeding each version and counting
folder-treeafter a 3s settle:The fix
SETTINGS_PERSIST_VERSIONfromsettingsStore.tsand have the spec stamp that instead of a literal, so the next migration bump cannot silently re-break the spec.expecttimeout from test(e2e): stop the folder-tree boot assert flaking CI #275 in both playwright configs. It was based on the wrong diagnosis, it fixed nothing, and its comment now misleads. The html reporter from that PR stays — its screenshot is what identified the calendar-only sidebar.e2e/general-settings.spec.tsande2e/nav-and-pin.spec.tsalready seed v3 plus an explicitsidebarGroupsrecord, so they were never exposed.Note on provenance: this is a port of #276, which fixed the same bug on
dev(CI green at 584b299).mainnever received it and still carries both the broken seed and the ineffective timeout, so the same flake is live onmain's gate. Refs #276, #275.Verification
npm run typechecknpm run lintnpm testnpm run e2e:ci(full suite, local)The 2 local e2e failures are
toHaveScreenshotdiffs on thedefaultandsolarized-darkthemes at ratio 0.03 vs a 0.02 threshold — a font-metric delta between this machine and the Ubuntu runner (the diff image is glyph-width shift on identical text). They fail identically before and after this change and both pass on CI. Thefolder-treeboot assert now clears on all four themes, and reverting the timeout produced no boot-assert flakes across the full local suite.Opened by the automated autofix bot. Awaiting human review — not merged by the bot.