Round 4 reviewed round 3's commit. Nothing in production behaviour was wrong; what was wrong was the round's own test and its comments, which is the failure class this loop keeps finding. - The new NavigationProvider case named after the `&& !channelViewRef.current` clause never reached it. With storage cleared and a bare URL the provider lands on "home" (E24's own default), not "feed", so asking for "feed" was an ordinary page change and the guard's first conjunct short-circuited. Deleting the clause left all 828 tests green — a dead nav control (the rail's own module stops closing an open channel page) with no red anywhere. The test now asserts the precondition and calls `setPage` with the page that IS current. - Both new Home cases rejected a promise and then asserted after one fixed macrotask. React-query notifies through `notifyManager`'s own `setTimeout(0)`, registered after any timer the test schedules, so the assertion raced it; two reviewers measured intermittent reds. They poll for a signal that the rejection LANDED instead — the reroll button releasing, which is also the honest one for the empty-answer case, where the empty state is already on screen while the press is in flight and asserting it early would pass either way. - `buryUnder`'s comment still opened with "one rule for both entry points" ten lines above the paragraph saying the rule is complete only for one of them, listed the non-reporting palette rows without `openChannel` — which has its own `replaceState` no-push branch and is reached from PlayerModal too — and pointed at a ROADMAP.md this repository does not contain. - `history.dom.test.ts`'s header claimed its layers close by genuine Backs only, which two of its own tests contradict; and `CommandPalette.ordering.dom.test.tsx` still claimed to be "the only assertion in the suite" doing what the file corrected in round 3 also does. Round 3's message said that pair was handled and only one side had been. The provider test was re-mutation-checked (dropping the clause now goes red), and both hardened Home tests still go red against their own reverts. Seven consecutive full runs green. Gate: ruff, ruff format, 3x tsc, eslint (0 errors), prettier, knip, vitest 828, pytest 1005.
112 lines
4.8 KiB
TypeScript
112 lines
4.8 KiB
TypeScript
// @vitest-environment jsdom
|
|
//
|
|
// `setPage`'s no-op contract — the PRODUCER side of the history-burial fix, which nothing rendered
|
|
// before this. `setPage` early-returns when the requested page is already current with no channel
|
|
// page over it, and reports that by returning `false`; `lib/history.buryUnder` reads the boolean to
|
|
// decide whether the overlay entries beneath a navigation are still reclaimable. Get it wrong and
|
|
// the round-2 bug is back: Shift+W over the player on Home buries an entry nothing pushed past, and
|
|
// the user's next browser Back does nothing.
|
|
//
|
|
// The type does NOT fence this. `NavigationActions` pins the SHAPE (`=> boolean`), so deleting one
|
|
// `return` fails tsc — but round 3 measured that swapping the two values (early return `true`, tail
|
|
// `false`) passes typecheck and all 822 tests. Hence assertions on the value AND on the effects the
|
|
// early return skips, which is what the boolean actually stands for.
|
|
import { describe, it, expect, afterEach, beforeEach, vi } from "vitest";
|
|
import { renderHook, act, cleanup } from "@testing-library/react";
|
|
import type { ReactNode } from "react";
|
|
|
|
// Partial: the provider reads `t` AND `i18n.language` (both feed the document-title effect), and the
|
|
// import graph pulls in `src/i18n`, which initialises against the real `initReactI18next` — so a
|
|
// wholesale mock throws before any test runs.
|
|
vi.mock("react-i18next", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("react-i18next")>();
|
|
return {
|
|
...actual,
|
|
useTranslation: () => ({ t: (k: string) => k, i18n: { language: "en" } }),
|
|
};
|
|
});
|
|
|
|
import { NavigationProvider, useNavigation, useNavigationActions } from "./NavigationProvider";
|
|
|
|
const wrapper = ({ children }: { children: ReactNode }) => (
|
|
<NavigationProvider>{children}</NavigationProvider>
|
|
);
|
|
const render = () =>
|
|
renderHook(() => ({ ...useNavigation(), ...useNavigationActions() }), { wrapper });
|
|
|
|
beforeEach(() => {
|
|
localStorage.clear();
|
|
sessionStorage.setItem("siftlode.activeAccount", "1"); // account-scoped page memory
|
|
window.history.replaceState({ sfPage: "feed" }, "", "/");
|
|
});
|
|
// Not automatic: vitest runs without `globals`, so RTL never registers its own afterEach — leaving a
|
|
// provider (and its popstate listener) mounted for the next case. Same reason Home.dom.test.tsx does.
|
|
afterEach(cleanup);
|
|
|
|
describe("NavigationProvider · setPage reports whether it navigated", () => {
|
|
it("returns true and pushes an entry for a real page change", () => {
|
|
const { result } = render();
|
|
const push = vi.spyOn(window.history, "pushState");
|
|
try {
|
|
let reported: boolean | undefined;
|
|
act(() => {
|
|
reported = result.current.setPage("plex");
|
|
});
|
|
expect(reported).toBe(true);
|
|
expect(push).toHaveBeenCalledTimes(1);
|
|
expect(result.current.page).toBe("plex");
|
|
} finally {
|
|
push.mockRestore();
|
|
}
|
|
});
|
|
|
|
it("returns false and pushes nothing when it is already that page", () => {
|
|
const { result } = render();
|
|
act(() => {
|
|
result.current.setPage("plex");
|
|
});
|
|
const push = vi.spyOn(window.history, "pushState");
|
|
try {
|
|
let reported: boolean | undefined;
|
|
act(() => {
|
|
reported = result.current.setPage("plex");
|
|
});
|
|
// Both halves matter: `false` is what `buryUnder` reads, and "pushed nothing" is what makes
|
|
// that answer true. A value-only assertion would pass against an inverted pair.
|
|
expect(reported).toBe(false);
|
|
expect(push).not.toHaveBeenCalled();
|
|
} finally {
|
|
push.mockRestore();
|
|
}
|
|
});
|
|
|
|
it("returns true even for the current page when a channel page is over it", () => {
|
|
// The nav rail's own module while a channel page is open over it is a real navigation: it closes
|
|
// the channel page. The `&& !channelViewRef.current` half of the guard is what makes it one.
|
|
const { result } = render();
|
|
// The page under test must be the one that is ALREADY current, or the guard's first conjunct is
|
|
// false and this never reaches the clause it is named for — the mistake round 4 caught here: a
|
|
// bare URL with cleared storage lands on "home" (navigationInit's E24 default), not "feed", so
|
|
// asking for "feed" was just an ordinary page change and deleting the clause left it green.
|
|
const current = result.current.page;
|
|
expect(current).toBe("home");
|
|
act(() => {
|
|
result.current.openChannel("UC1", "A channel");
|
|
});
|
|
expect(result.current.channelView).not.toBeNull();
|
|
|
|
const push = vi.spyOn(window.history, "pushState");
|
|
try {
|
|
let reported: boolean | undefined;
|
|
act(() => {
|
|
reported = result.current.setPage(current);
|
|
});
|
|
expect(reported).toBe(true);
|
|
expect(push).toHaveBeenCalledTimes(1);
|
|
expect(result.current.channelView).toBeNull();
|
|
} finally {
|
|
push.mockRestore();
|
|
}
|
|
});
|
|
});
|