From c8d94a820349ee4750c7f09efa85ddf55178f313 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 19:33:56 -0400 Subject: [PATCH] test(e2e): cover configurable destinations; stop assuming a nav item The assertions are about the navigation, not about the setting: "the config was saved" is the plumbing, and #69 and #72 both shipped green under specs that measured exactly that. Four existing specs reached a view by clicking its nav item, which since this change is not guaranteed to exist -- Autotag is hidden by default and Downloads is absent without a download client -- so they timed out waiting for a locator that will never resolve. `navigateTo` dispatches the app's own `navigate` event, which is what every nav item, card and detail view dispatches, so it is the mechanism rather than a test-only door. Click the item when the nav is the subject. Closes #25 --- e2e/specs/page-header.spec.ts | 8 ++- e2e/specs/settings-reach.spec.ts | 6 +- e2e/specs/view-lifecycle.spec.ts | 9 ++- e2e/specs/view-visibility.spec.ts | 116 ++++++++++++++++++++++++++++++ e2e/support/fixtures.ts | 29 ++++++++ 5 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 e2e/specs/view-visibility.spec.ts diff --git a/e2e/specs/page-header.spec.ts b/e2e/specs/page-header.spec.ts index faf16ed..71ce42a 100644 --- a/e2e/specs/page-header.spec.ts +++ b/e2e/specs/page-header.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../support/fixtures.js'; +import { test, expect, navigateTo } from '../support/fixtures.js'; /** * H-19: Playlists, Downloads, Jobs, Settings and Home had a page @@ -58,8 +58,12 @@ const TAGS: Record = { test.describe('every primary view says what it is', () => { test('each one has the shared header, with a heading', async ({ app }) => { + // By event rather than by nav item: a destination is not + // guaranteed to have one any more (#25 — Downloads is absent + // without a download client), and every one of these is still a + // primary view with a header, which is what this spec is about. for (const [view, heading, hasCount] of VIEWS) { - await app.getByTestId(`nav-${view}`).click(); + await navigateTo(app, view); await expect(app.getByTestId('main-content')).toHaveAttribute( 'data-active-view', view, diff --git a/e2e/specs/settings-reach.spec.ts b/e2e/specs/settings-reach.spec.ts index 9e9273b..1ddddea 100644 --- a/e2e/specs/settings-reach.spec.ts +++ b/e2e/specs/settings-reach.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../support/fixtures.js'; +import { test, expect, navigateTo } from '../support/fixtures.js'; /** * Plan 007 phase 5: a11y.1 and a11y.2, frozen against the real app. @@ -56,7 +56,9 @@ test.describe('Settings is reachable without a mouse', () => { test.describe("Downloads' tabs are tabs", () => { test('arrow keys move the selection and swap the panel', async ({ app }) => { - await app.getByTestId('nav-downloads').click(); + // By event, not by nav item: with no download client configured + // there is no Downloads destination to click (#25). + await navigateTo(app, 'downloads'); const view = app.locator('downloads-view'); const requests = view.getByRole('tab', { name: 'Requests' }); diff --git a/e2e/specs/view-lifecycle.spec.ts b/e2e/specs/view-lifecycle.spec.ts index 7bd9d08..d42e764 100644 --- a/e2e/specs/view-lifecycle.spec.ts +++ b/e2e/specs/view-lifecycle.spec.ts @@ -4,6 +4,7 @@ import { eventNames, resetEvents, waitForEvent, + navigateTo, } from '../support/fixtures.js'; /** @@ -39,7 +40,9 @@ test.describe('view lifecycle', () => { test('a keypress on Settings does not reach the Autotag queue', async ({ app, }) => { - await app.getByTestId('nav-autotag').click(); + // By event, not by nav item: Autotag is hidden by default (#25) + // and a hidden view is still reachable. + await navigateTo(app, 'autotag'); await expect(app.getByTestId('main-content')).toHaveAttribute( 'data-active-view', 'autotag', @@ -83,7 +86,9 @@ test.describe('view lifecycle', () => { // The other half of the same bug (H-2): two document keydown handlers // with no arbitration meant `s` on this page skipped the album *and* // toggled shuffle. As a panel binding it can only mean one thing. - await app.getByTestId('nav-autotag').click(); + // By event, not by nav item: Autotag is hidden by default (#25) + // and a hidden view is still reachable. + await navigateTo(app, 'autotag'); await expect .poll(() => pendingCount(app)) .toMatch(/^Pending \(\d+\)$/); diff --git a/e2e/specs/view-visibility.spec.ts b/e2e/specs/view-visibility.spec.ts new file mode 100644 index 0000000..9a5b7e4 --- /dev/null +++ b/e2e/specs/view-visibility.spec.ts @@ -0,0 +1,116 @@ +import { test, expect, navigateTo } from '../support/fixtures.js'; + +/** + * Which destinations the navigation offers (#25). + * + * Eleven sidebar entries is more than most libraries need, so they are + * individually toggleable from Settings, Autotag is off until asked for + * and Downloads is absent until there is a client to download with. + * + * **The assertions are about the navigation, not about the setting.** + * "The config was saved" is the plumbing, and the two most recent bugs + * in this area — #69 and #72 — both shipped green under specs that + * measured exactly that. What a person sees is whether the item is in + * the accessibility tree, and whether the view is still reachable when + * it is not. + * + * This runs against the seeded app, whose config is defaults and whose + * download client list is empty, so the initial state below is what a + * fresh install looks like. + */ +type Page = import('@playwright/test').Page; + +const navItem = (page: Page, label: string) => + page.getByRole('button', { name: label, exact: true }); + +/** The Navigation section's checkbox for a destination. */ +const viewToggle = (page: Page, label: string) => + page.getByRole('checkbox', { name: `Show ${label} in the navigation` }); + +async function openNavigationSettings(page: Page): Promise { + await page.getByTestId('nav-settings').click(); + + const section = page.locator( + 'config-page config-section[heading="Navigation"] .header', + ); + + await expect(section).toBeVisible(); + + if ((await section.getAttribute('aria-expanded')) === 'false') { + await section.click(); + } + + await expect(section).toHaveAttribute('aria-expanded', 'true'); +} + +test.describe('configurable destinations', () => { + test('Autotag is off by default and Downloads needs a client', async ({ + app, + }) => { + await expect(app.getByTestId('nav-home')).toBeVisible(); + + await expect(app.getByTestId('nav-autotag')).toHaveCount(0); + await expect(app.getByTestId('nav-downloads')).toHaveCount(0); + }); + + /** + * Hiding takes the item away and nothing else. Detail views navigate + * into these and the launch page is one of them, so a destination + * with no nav item still has to open. + */ + test('a hidden destination is still reachable', async ({ app }) => { + await navigateTo(app, 'autotag'); + + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'autotag', + ); + + // And nothing is falsely lit while standing on it -- the same rule + // a detail view follows, with no special case for either. + await expect(navItem(app, 'Home')).toHaveAttribute('aria-current', 'false'); + }); + + test('switching Autotag on adds it to the sidebar', async ({ app }) => { + await openNavigationSettings(app); + + await viewToggle(app, 'Autotag').check(); + + await expect(app.getByTestId('nav-autotag')).toBeVisible(); + + // Clicking it is the point of having it. + await app.getByTestId('nav-autotag').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'autotag', + ); + + // Put it back, or the next spec against this app sees a library + // this one changed. + await openNavigationSettings(app); + await viewToggle(app, 'Autotag').uncheck(); + await expect(app.getByTestId('nav-autotag')).toHaveCount(0); + }); + + /** + * Settings has no toggle at all, rather than a toggle that refuses: + * a user who hides it cannot get back to unhide it. The backend + * refuses it too, because `config.toml` is hand-editable. + */ + test('Settings cannot be switched off', async ({ app }) => { + await openNavigationSettings(app); + + await expect(viewToggle(app, 'Settings')).toBeDisabled(); + await expect(app.getByTestId('nav-settings')).toBeVisible(); + }); + + /** + * The launch page is refused while it is the launch page, which is a + * state the user can leave by changing the launch page above it. + */ + test('the launch page cannot be switched off', async ({ app }) => { + await openNavigationSettings(app); + + await expect(viewToggle(app, 'Home')).toBeDisabled(); + }); +}); diff --git a/e2e/support/fixtures.ts b/e2e/support/fixtures.ts index db33658..72a0327 100644 --- a/e2e/support/fixtures.ts +++ b/e2e/support/fixtures.ts @@ -111,6 +111,35 @@ export async function bindingCalls(page: Page): Promise { return calls.map(nameOf); } +/** + * Go to a view without going through the navigation. + * + * `navigate` is the event the shell listens for and every nav item, card + * and detail view dispatches, so this is the app's own mechanism rather + * than a test-only door. It exists because a destination is not + * guaranteed to have a nav item any more (#25): Autotag is hidden until + * the user asks for it and Downloads until a client exists, and a spec + * about what a *view* does should not also be asserting that the + * sidebar offers it. + */ +export async function navigateTo(page: Page, view: string): Promise { + await page.evaluate( + (v) => + void document.dispatchEvent( + new CustomEvent('navigate', { + detail: { view: v }, + bubbles: true, + composed: true, + }), + ), + view, + ); + + await page + .getByTestId('main-content') + .waitFor({ state: 'attached' }); +} + /** Thin client for the dev-only /__test/ surface (backend/testctl). */ export class TestCtl { constructor(private readonly baseURL: string) {}