peter
d832bb19ce
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.
44 lines
2.2 KiB
TypeScript
44 lines
2.2 KiB
TypeScript
// @vitest-environment jsdom
|
|
//
|
|
// ONE test, ONE file, for the same reason CommandPalette.ordering.dom.test.tsx has its own: it
|
|
// ASSERTS on a reconcile issuing a REAL, unstubbed programmatic history.back(), and history.ts's
|
|
// overlay bookkeeping is module-global. history.dom.test.ts keeps its shared state clean by never
|
|
// leaving a real back() in flight — its layers close by a genuine Back, a stubbed back(), or a
|
|
// consumed burial mark — and this test would break that.
|
|
import { describe, it, expect, vi } from "vitest";
|
|
import { renderHook, act } from "@testing-library/react";
|
|
import { navigateOverOverlays, useBackToClose } from "./history";
|
|
|
|
const settle = () =>
|
|
act(async () => {
|
|
await Promise.resolve();
|
|
});
|
|
|
|
describe("navigateOverOverlays · burying a navigation that never happened", () => {
|
|
// The round-2 defect. `navigateOverOverlays` marked every live overlay entry buried BEFORE it knew
|
|
// whether the navigation pushed anything, and `setPage` early-returns when the requested page is
|
|
// already current. Home is the default landing page, so Shift+W pressed while already on Home with
|
|
// the player open buried the player's entry under a push that never happened: its unmount then
|
|
// consumed the mark instead of reclaiming the entry with history.back(), leaving an orphan. The
|
|
// user's next browser Back popped onto an identical `sfPage` with an empty overlay stack — a press
|
|
// that changed nothing on screen, one more per play → Shift+W → close cycle.
|
|
it("leaves the overlay's entry reclaimable when the navigation reports it pushed nothing", async () => {
|
|
const overlay = renderHook(() => useBackToClose(() => {}));
|
|
await settle();
|
|
const back = vi.spyOn(window.history, "back");
|
|
try {
|
|
// What `setPage` returns from its early return: asked for the page we are already on.
|
|
const navigate = vi.fn(() => false);
|
|
act(() => navigateOverOverlays(navigate));
|
|
expect(navigate).toHaveBeenCalledTimes(1);
|
|
|
|
overlay.unmount();
|
|
await settle();
|
|
// Nothing was pushed over it, so the entry is still ours — reclaimed, not silently orphaned.
|
|
expect(back).toHaveBeenCalledTimes(1);
|
|
} finally {
|
|
back.mockRestore();
|
|
}
|
|
});
|
|
});
|