Skip to content

Commit e81e6c8

Browse files
authored
fix(e2e): make WelcomePortal recovery navigation locale-independent (#590)
* fix(e2e): make WelcomePortal recovery navigation locale-independent Extracted from PR #583's already-converged fix for exactly this failure class -- see #589 for the reproduced blocker and #532 for the broader startup/navigation nondeterminism this belongs to. ensureWelcomePortalEntry()'s Factory-Reset recovery fallback (used when a pre-existing/leftover project causes a cold boot to land in the main shell instead of the WelcomePortal) drove Settings/Data/Factory-Reset navigation through English/German-only translated button-name regexes, after trying to force English via localStorage + reload. The Playwright accessibility snapshot from #589's failure proved that reload doesn't reliably take effect before the English-only lookup runs, so the recovery path fails deterministically whenever the rare landing-in-main- chrome race triggers with any other persisted locale (Spanish, in the observed case). Replaces the whole recovery flow with stable, locale-independent data-tour/data-testid anchors end to end: - clickSettingsNavItem() (helpers.ts) -- mobile-aware Settings navigation keyed on data-tour="nav-settings"/"nav-more", not translated text. - resolveStartupState() -- explicit WELCOME_PORTAL | MAIN_CHROME result instead of repeated isVisible().catch(() => false) boolean soup. - settings-nav- testid on SettingsView's NavButton, and factory-reset-button / factory-reset-confirm-button testids on DataSection / SettingsModals, so the recovery flow never depends on translated labels. - Sidebar.tsx gains the data-tour="nav-more" anchor the new helper needs -- traced as a required dependency not otherwise present on main. Adds a dedicated regression test (onboarding-entry-precondition.spec.ts) that deterministically reproduces the exact failure shape (persisted main-chrome project + non-English language, Mobile Chrome and desktop) instead of relying on the rare race to expose it. Deliberately excludes #583's unrelated handleFactoryReset error-toast refactor (hooks/useSettingsView.ts) and its tests -- orthogonal to this locale-independence fix, left for #583's own convergence. * docs: add QNBS-v3 annotations for the new E2E test-selector attributes Sidebar.tsx's dataTour prop and DataSection.tsx's factory-reset-button testid were extracted from #583 with a plain JSDoc / no comment; the repo convention requires a single-line QNBS-v3 annotation on non-trivial TS/TSX changes. No behavior change. * test(e2e): assert applied locale, not just the persisted seed The prior assertion only proved localStorage held 'es', which doesn't prove the app actually rendered in Spanish -- a broken addInitScript seed or a failed es bundle load could still pass this test vacuously in English, the exact vacuity the original comment claimed to prevent. document.documentElement.lang (set by I18nProvider/App.tsx on mount) is the real applied-locale authority; assert that instead, using Playwright's own auto-wait toHaveAttribute matcher rather than a bare evaluate() + expect(). * chore(ci): exclude tests/ from codecov patch coverage vitest.config.ts's own coverage.include list scopes measurement to application source (App.tsx, index.tsx, register-sw.ts, app/, components/, features/, hooks/, services/, packages/*/src/) -- tests/** was never instrumented, by design, since it's test code, not application source. No codecov.yml existed to tell Codecov the same thing, so any PR adding substantial new logic to an E2E helper file (as #590 does in tests/e2e/helpers.ts) got counted as uncovered diff lines it structurally cannot have coverage data for, producing a false codecov/patch failure regardless of how well the actual application-source changes in the same diff were covered. Mirrors vitest.config.ts's coverage.exclude entry for the same path. Pure YAML config -- rationale here in the commit message, not an inline comment, per repo convention. * fix(ci): correct codecov.yml ignore key to top-level per documented schema ignore: is a documented top-level codecov.yml key, not nested under coverage: -- confirmed against docs.codecov.com and codecov.io/validate (parses to the expected (?s:tests/.*)\Z regex). The previous shape was accepted by YAML parsing but wasn't the schema Codecov's config loader actually recognizes.
1 parent c1a728b commit e81e6c8

7 files changed

Lines changed: 76 additions & 29 deletions

File tree

codecov.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ignore:
2+
- "tests/**"

components/SettingsView.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,17 @@ import { ViewErrorBoundary } from './ui/ViewErrorBoundary';
3838

3939
// --- SUB-COMPONENTS ---
4040

41+
// QNBS-v3: stable data-testid lets E2E recovery navigation target a category without matching translated label text
4142
const NavButton: FC<{
43+
id: string;
4244
icon: React.ReactNode;
4345
label: string;
4446
isActive: boolean;
4547
onClick: () => void;
46-
}> = React.memo(({ icon, label, isActive, onClick }) => (
48+
}> = React.memo(({ id, icon, label, isActive, onClick }) => (
4749
<button
4850
type="button"
51+
data-testid={`settings-nav-${id}`}
4952
onClick={onClick}
5053
aria-current={isActive ? 'page' : undefined}
5154
className={`flex items-center flex-shrink-0 md:flex-shrink md:w-full px-3 py-2 text-left rounded-md transition-colors whitespace-nowrap md:whitespace-normal ${isActive ? 'bg-[var(--nav-background-active)] text-[var(--nav-text-active)]' : 'hover:bg-[var(--nav-background-hover)] text-[var(--sc-text-secondary)] hover:text-[var(--sc-text-primary)]'}`}
@@ -365,6 +368,7 @@ const SettingsViewUI: FC = () => {
365368
filteredNavCategories.map((cat) => (
366369
<NavButton
367370
key={cat.id}
371+
id={cat.id}
368372
icon={cat.icon}
369373
label={cat.label}
370374
isActive={activeCategory === cat.id}
@@ -379,6 +383,7 @@ const SettingsViewUI: FC = () => {
379383
.map((cat) => (
380384
<NavButton
381385
key={cat.id}
386+
id={cat.id}
382387
icon={cat.icon}
383388
label={cat.label}
384389
isActive={activeCategory === cat.id}
@@ -396,6 +401,7 @@ const SettingsViewUI: FC = () => {
396401
{groupCats.map((cat) => (
397402
<NavButton
398403
key={cat.id}
404+
id={cat.id}
399405
icon={cat.icon}
400406
label={cat.label}
401407
isActive={activeCategory === cat.id}

components/Sidebar.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ const BottomTabItem: React.FC<{
7777
isActive: boolean;
7878
onClick: () => void;
7979
sectionId?: string;
80-
}> = React.memo(({ icon, label, isActive, onClick, sectionId }) => {
80+
// QNBS-v3: stable data-tour anchor lets E2E recovery navigation find this button without matching translated label text
81+
dataTour?: string;
82+
}> = React.memo(({ icon, label, isActive, onClick, sectionId, dataTour }) => {
8183
// QNBS-v3: colored icon dot for mobile tab bar via section SSOT
8284
const sectionConfig = sectionId ? APP_SECTIONS[sectionId as keyof typeof APP_SECTIONS] : null;
8385
const iconColor = sectionConfig && !isActive ? sectionConfig.textColor : '';
@@ -86,6 +88,7 @@ const BottomTabItem: React.FC<{
8688
<button
8789
type="button"
8890
onClick={onClick}
91+
data-tour={dataTour}
8992
className={`relative flex flex-col items-center justify-center flex-1 min-h-[44px] py-2 transition-colors duration-200 touch-manipulation outline-none focus-visible:ring-2 focus-visible:ring-[var(--sc-ring-focus)] rounded-lg ${
9093
isActive ? 'text-[var(--nav-text-active)]' : 'text-[var(--sc-text-muted)]'
9194
}`}
@@ -201,6 +204,7 @@ export const Sidebar: React.FC<SidebarProps> = ({
201204
label={t('common.more')}
202205
isActive={isSidebarOpen || !isTabBarView}
203206
onClick={() => setIsSidebarOpen(!isSidebarOpen)}
207+
dataTour="nav-more"
204208
/>
205209
</nav>
206210

@@ -299,4 +303,4 @@ export const Sidebar: React.FC<SidebarProps> = ({
299303
</aside>
300304
</>
301305
);
302-
};
306+
};

components/settings/DataSection.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,9 +418,11 @@ export const DataSection: FC = () => {
418418
{t('settings.data.dangerZone.factoryReset.hint')}
419419
</p>
420420
</div>
421+
{/* QNBS-v3: stable data-testid lets E2E recovery navigation target this button without matching translated label text */}
421422
<Button
422423
variant="danger"
423424
size="sm"
425+
data-testid="factory-reset-button"
424426
onClick={() => setModal({ state: 'factoryReset', payload: {} })}
425427
className="shrink-0"
426428
>

components/settings/SettingsModals.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,12 @@ export const SettingsModals: FC = () => {
138138
<Button variant="secondary" onClick={() => setModal({ state: 'closed', payload: {} })}>
139139
{t('common.cancel')}
140140
</Button>
141-
<Button variant="danger" onClick={() => void handleFactoryReset()}>
141+
{/* QNBS-v3: stable data-testid lets E2E recovery navigation target this button without matching translated label text */}
142+
<Button
143+
variant="danger"
144+
onClick={() => void handleFactoryReset()}
145+
data-testid="factory-reset-confirm-button"
146+
>
142147
{t('settings.data.dangerZone.factoryReset.modalConfirm')}
143148
</Button>
144149
</div>

tests/e2e/helpers.ts

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ export async function clickNavItem(page: Page, name: RegExp): Promise<void> {
2929
await page.locator('#sidebar-mobile').getByRole('button', { name }).click();
3030
}
3131

32+
/** Locale-independent Settings navigation: same mobile-aware fallback as clickNavItem, keyed on the stable `data-tour="nav-settings"` anchor instead of translated visible text. */
33+
async function clickSettingsNavItem(page: Page): Promise<void> {
34+
const desktopBtn = page.locator('#sidebar [data-tour="nav-settings"]');
35+
if (await desktopBtn.isVisible({ timeout: 1500 }).catch(() => false)) {
36+
await desktopBtn.click();
37+
return;
38+
}
39+
// QNBS-v3: data-tour="nav-more" (not a translated /More/i label) so this stays locale-independent on a non-English mobile boot.
40+
const moreBtn = page.locator('[data-tour="nav-more"]');
41+
await expect(moreBtn).toBeVisible({ timeout: 8000 });
42+
await moreBtn.click();
43+
await page.locator('#sidebar-mobile').waitFor({ state: 'visible' });
44+
await page.locator('#sidebar-mobile [data-tour="nav-settings"]').click();
45+
}
46+
3247
// QNBS-v3: Stable Writer `#writer-section-select` + option handling avoids Playwright strict-mode / native-<option> visibility pitfalls that broke CI E2E.
3348

3449
/** Writer section `<Select>` — stable id to avoid picking tone/tool comboboxes elsewhere on the page. */
@@ -147,6 +162,19 @@ export async function waitForMainChrome(page: Page): Promise<void> {
147162
]);
148163
}
149164

165+
/** QNBS-v3: explicit discriminated startup state, not boolean soup — repeatedly asking "is the portal visible?" via isVisible().catch(()=>false) can't distinguish "main chrome" from "still loading" and silently swallows genuine errors as false. */
166+
export type StartupState = 'WELCOME_PORTAL' | 'MAIN_CHROME';
167+
168+
/** Resolves which of waitForSpaReady()'s two shapes the current document actually reached. */
169+
export async function resolveStartupState(page: Page): Promise<StartupState> {
170+
await waitForSpaReady(page);
171+
const portal = page.getByTestId('welcome-portal');
172+
if (await portal.isVisible().catch(() => false)) {
173+
return 'WELCOME_PORTAL';
174+
}
175+
return 'MAIN_CHROME';
176+
}
177+
150178
/** Language toggle on the welcome portal (EN must be active for English copy in assertions). */
151179
export async function selectEnglish(page: Page): Promise<void> {
152180
const enBtn = page.getByRole('button', { name: /^EN$/i }).first();
@@ -179,36 +207,20 @@ export async function ensureBlankProject(page: Page): Promise<void> {
179207
* waitForSpaReady()'s two success shapes the app actually booted into. A cold CI boot has landed
180208
* in an already-mounted main shell with a persisted project instead of the portal — a startup-
181209
* state precondition gap distinct from the (fixed) portal-activation auto-seed race.
182-
* Contract: guarantees the portal is reached, locale-independently — it does NOT guarantee
183-
* English. A caller needing English selects it itself (export.spec.ts already does this for the
184-
* fresh-boot case). Recovers via the real Settings → Data & Backups → Factory Reset flow when
185-
* main chrome is active so no React/Redux/storage internals are touched — only supported app
186-
* behavior.
210+
* Contract: guarantees the portal is reached, locale-independently. Recovers via the real
211+
* Settings → Data & Backups → Factory Reset flow when main chrome is active so no React/Redux/
212+
* storage internals are touched — only supported app behavior.
187213
*/
188214
export async function ensureWelcomePortalEntry(page: Page): Promise<void> {
189-
await waitForSpaReady(page);
190215
const portal = page.getByTestId('welcome-portal');
191-
if (await portal.isVisible({ timeout: 3000 }).catch(() => false)) {
216+
if ((await resolveStartupState(page)) === 'WELCOME_PORTAL') {
192217
return;
193218
}
194-
// QNBS-v3: force English before the locale-dependent recovery flow below, or a persisted non-EN/DE language would hang it.
195-
await page.evaluate(() => localStorage.setItem('worldscript-language', 'en'));
196-
await page.reload();
197-
await waitForSpaReady(page);
198-
// QNBS-v3: this reload can itself race a pending debounced autosave and land back in WelcomePortal instead of main chrome — accept either state again rather than assuming main chrome.
199-
if (await portal.isVisible({ timeout: 3000 }).catch(() => false)) {
200-
return;
201-
}
202-
await waitForMainChrome(page);
203-
await clickNavItem(page, /Settings/i);
204-
await page
205-
.getByRole('button', { name: /Data & Backups|Daten & Backups/i })
206-
.first()
207-
.click();
208-
await page.getByRole('button', { name: /Factory Reset|Werkseinstellungen/i }).click();
209-
await page
210-
.getByRole('button', { name: /Delete everything & restart|Alles löschen & neu starten/i })
211-
.click();
219+
// QNBS-v3: every step below uses a stable data-tour/data-testid anchor, never translated text — Playwright's own docs say addInitScript execution order across registrations is unspecified, so this cannot rely on forcing a language first.
220+
await clickSettingsNavItem(page);
221+
await page.getByTestId('settings-nav-data').click();
222+
await page.getByTestId('factory-reset-button').click();
223+
await page.getByTestId('factory-reset-confirm-button').click();
212224
await waitForSpaReady(page);
213225
await expect(portal).toBeVisible({ timeout: 15000 });
214226
}

tests/e2e/onboarding-entry-precondition.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,22 @@ test.describe('WelcomePortal entry precondition (CI-only)', () => {
4242
await expect(page.getByRole('button', { name: /Start a New Project/i })).toBeVisible();
4343
});
4444

45+
test('reaches the entry point via the recovery flow with a persisted non-English language, on Mobile Chrome and desktop alike', async ({
46+
page,
47+
}) => {
48+
// QNBS-v3: a fresh boot lands on the portal regardless of locale — this combines a persisted main-chrome project with a non-English language so a mobile "More"-button locale regression actually fails, on every project including Mobile Chrome.
49+
await page.goto('/');
50+
await ensureBlankProject(page);
51+
await expect(page.getByText(/All changes saved/i)).toBeVisible({ timeout: 10000 });
52+
await page.addInitScript(() => localStorage.setItem('worldscript-language', 'es'));
53+
await page.reload();
54+
await waitForMainChrome(page);
55+
// QNBS-v3: asserts the applied locale, not just the persisted seed — a broken addInitScript or a failed es bundle load could otherwise pass this test vacuously in English.
56+
await expect(page.locator('html')).toHaveAttribute('lang', 'es');
57+
await ensureWelcomePortalEntry(page);
58+
await expect(page.getByTestId('welcome-portal')).toBeVisible();
59+
});
60+
4561
test('reaches the entry point when its own internal reload can race a pending autosave', async ({
4662
page,
4763
}) => {

0 commit comments

Comments
 (0)