From 6c1cc6c1ae0a1cf9774eb625e2b07730d82b5b42 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 23 Aug 2026 10:27:28 -0400 Subject: [PATCH] test(web): cover the settings toast's reload-failure title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review r4. The r3 title branch was the one modified behavior with no test of its own — the existing ServerListReloadError cases cover the pagination, add, and rename paths only, so a regression back to "Failed to save settings" would have gone green. `useSettingsDraft` is mocked in this suite, so the test invokes the `onError` App handed it and asserts both titles: the reload-failure one, and that a genuine write failure still reads as a failure. Forcing the branch off fails it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall --- clients/web/src/App.test.tsx | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index db5a28d5..032f46e9 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -2581,6 +2581,39 @@ describe("App config submit with a failed list reload (#1914)", () => { ); }); + it("labels a settings save whose reload failed as saved, not failed (#1914)", () => { + // The settings draft debounces and flushes on close, so this toast is the + // user's only signal — a flush that rejects usually does so after the + // modal is gone. `useSettingsDraft` is mocked here, so the App's `onError` + // is invoked directly with the options it was handed. + renderWithMantine(); + const onError = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0].onError; + expect(onError).toBeDefined(); + + act(() => { + onError!( + "A", + new ServerListReloadError( + "The server was saved, but the server list could not be reloaded: disk full", + ), + ); + }); + expect(notificationsMock.show).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Saved settings for "A", but the server list did not reload', + message: expect.stringContaining("disk full"), + }), + ); + + // A genuinely failed write still reads as a failure. + act(() => { + onError!("A", new Error("disk full")); + }); + expect(notificationsMock.show).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Failed to save settings for "A"' }), + ); + }); + it("keeps a successful add in the highlight batch when the reload failed (#1914)", async () => { // `addServerHighlighted` marked the new id only after `addServer` // resolved. The row is on disk either way, so on a reload failure it has