test: the e2e suite waits for each save before it closes the popup (closes #446)

The Settings round trip switched the theme and the network and closed
the popup at once. A close before the change handler's save lands loses
the switch, and the suite then ran on Sepolia.

tests/e2e/run.js now has one helper that polls a field of the stored
record until it holds the expected value, in place of the wait that
only read viewStack. Each Settings switch and spam-filter toggle waits
for its save, the recovery-phrase reopen waits for its saved view, and
reopenPopup() waits until the view it expects to reopen on is the saved
one. README.md no longer lists #446 among the open reports of the
Chrome suite failing under load.

Model: opus-5-5

README.md

@@ -619,15 +619,13 @@ The jobs **report, they do not gate.** A failure is a red mark against the
 commit that a reviewer has to account for, not a hard block: whether a check
 blocks a merge is Gitea branch protection, which this repo does not configure.
 
-That is not only a statement about configuration. Reports of the Chrome suite
-**failing under load** are still open, among them
+That is not only a statement about configuration. A report of the Chrome suite
+**failing under load** is still open:
 [#290](https://git.eeqj.de/sneak/AutistMask/issues/290), runs on a busy machine
-failing with `the extension opened no approval window within 30000ms`, and
-[#446](https://git.eeqj.de/sneak/AutistMask/issues/446), the Settings round trip
-closing the popup before its network switch is saved. So a red `e2e-chrome` has
-to be read before it is believed, and those failures are the blocker to ever
-making this a required check. Do not answer them with a retry wrapper: a suite
-that reruns until it is green stops being evidence.
+failing with `the extension opened no approval window within 30000ms`. So a red
+`e2e-chrome` has to be read before it is believed, and those failures are the
+blocker to ever making this a required check. Do not answer them with a retry
+wrapper: a suite that reruns until it is green stops being evidence.
 
 Nothing in either job can pass vacuously. There is no `continue-on-error` and no
 `|| true`; both scripts exit non-zero when docker is missing, when the image

TODO.md

@@ -45,6 +45,18 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-10-05: The e2e suite waits for a save to land before it closes the popup
+  ([#446](https://git.eeqj.de/sneak/AutistMask/issues/446)). The Settings round
+  trip switched the theme and the network and closed the popup at once, and a
+  close before the save lands loses the switch; with the network left on
+  Sepolia, a dozen later tests failed too. Each Settings switch and spam-filter
+  toggle is now waited for in storage before the close, and `reopenPopup()`
+  waits until the view it expects to reopen on is the saved one. The restore
+  half of the round trip and the second filter toggle change a setting right
+  after a reopen, while the reopened popup's own saves may still be running;
+  they rely on the fix for
+  [#448](https://git.eeqj.de/sneak/AutistMask/issues/448).
+
 - 2026-10-05: A change made while an earlier save from the same page is still
   running is stored ([#448](https://git.eeqj.de/sneak/AutistMask/issues/448)).
   `saveStateOnce()` took its baseline from the page's state after the write, so

tests/e2e/run.js

@@ -204,40 +204,51 @@ async function goHome(page) {
     await visible(page, "#view-main");
 }
 
-// The navigation stack as it was actually persisted, read out of extension
-// storage rather than inferred from which screen is showing. A stale entry
-// left behind by a forward navigation that threw is invisible on screen
-// until the user presses Back one time too many — which is exactly the
-// second-order damage #150 did — so the stack itself is what gets asserted.
-function persistedViewStack(page) {
+// One field of the popup's state as it was actually persisted, read out of
+// extension storage rather than inferred from what is on screen.
+function persistedField(page, field) {
     return page.evaluate(
-        () =>
+        (key) =>
             new Promise((resolve) => {
                 chrome.storage.local.get("autistmask", (r) => {
-                    resolve((r.autistmask && r.autistmask.viewStack) || []);
+                    resolve(r.autistmask ? r.autistmask[key] : undefined);
                 });
             }),
+        field,
     );
 }
 
-// saveState() is fired from showView() without being awaited, so the write
-// lands shortly after the screen does. Polling for the expected stack keeps
-// that race out of the assertion; a stack that never becomes the expected
-// one fails with what it actually was.
-const VIEW_STACK_SETTLE_MS = 5000;
+// The navigation stack as it was actually persisted. A stale entry left
+// behind by a forward navigation that threw is invisible on screen until
+// the user presses Back one time too many — which is exactly the
+// second-order damage #150 did — so the stack itself is what gets asserted.
+async function persistedViewStack(page) {
+    return (await persistedField(page, "viewStack")) || [];
+}
 
-async function waitForViewStack(page, expected, where) {
+// Nothing here can await the popup's saves. showView() fires saveState()
+// without awaiting it, and a Settings control's "change" handler awaits its
+// save only after click() or selectOption() has already returned. So the
+// write lands shortly after the screen or the control changes, and a popup
+// closed before then loses it. Polling for the expected value keeps that
+// race out of the assertion and out of the close; a value that never
+// arrives fails with what it actually was.
+const SAVE_SETTLE_MS = 5000;
+
+async function waitForPersisted(page, field, expected, where) {
     const want = JSON.stringify(expected);
-    const deadline = Date.now() + VIEW_STACK_SETTLE_MS;
+    const deadline = Date.now() + SAVE_SETTLE_MS;
     let seen;
     for (;;) {
-        seen = await persistedViewStack(page);
+        seen = await persistedField(page, field);
         if (JSON.stringify(seen) === want) return;
         if (Date.now() >= deadline) break;
         await sleep(50);
     }
     throw new Error(
-        "navigation stack " +
+        "the persisted " +
+            field +
+            " " +
             where +
             " is " +
             JSON.stringify(seen) +
@@ -257,12 +268,18 @@ test("Back from Add Token unwinds the stack exactly once (#150)", async (env) =>
 
     await env.page.locator("#wallet-list .btn-addr-info").first().click();
     await visible(env.page, "#view-address");
-    await waitForViewStack(env.page, base.concat("main"), "on address detail");
+    await waitForPersisted(
+        env.page,
+        "viewStack",
+        base.concat("main"),
+        "on address detail",
+    );
 
     await env.page.click("#btn-add-token");
     await visible(env.page, "#view-add-token");
-    await waitForViewStack(
+    await waitForPersisted(
         env.page,
+        "viewStack",
         base.concat("main", "address"),
         "on the add token screen",
     );
@@ -273,15 +290,16 @@ test("Back from Add Token unwinds the stack exactly once (#150)", async (env) =>
         !(await env.page.isVisible("#view-add-token")),
         "the add token screen is still showing after Back",
     );
-    await waitForViewStack(
+    await waitForPersisted(
         env.page,
+        "viewStack",
         base.concat("main"),
         "after Back from add token",
     );
 
     await env.page.click("#btn-address-back");
     await visible(env.page, "#view-main");
-    await waitForViewStack(env.page, base, "after a second Back");
+    await waitForPersisted(env.page, "viewStack", base, "after a second Back");
 });
 
 test("a common-token quick-pick fills in the contract address (#150)", async (env) => {
@@ -679,6 +697,12 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env)
     await openPhraseScreen(env.page);
     await revealPhrase(env.page);
 
+    await waitForPersisted(
+        env.page,
+        "currentView",
+        "show-phrase",
+        "before closing the popup",
+    );
     await env.page.close();
     env.page = await openPopup(env.ctx, env.popupUrl);
     await visible(env.page, "#view-main");
@@ -714,10 +738,20 @@ function addressScreenState(page) {
 
 // Close and reopen the page rather than reload it: that is what the toolbar
 // popup does, and it is the only thing that produces the unrendered views.
-async function reopenPopup(env, restoredView) {
+// The popup reopens on the view it last saved, so the close waits until
+// `view` is the one saved. That wait cannot see a save that leaves the value
+// as it was: a caller whose popup already had `view` saved waits for a
+// screen in between first.
+async function reopenPopup(env, view) {
+    await waitForPersisted(
+        env.page,
+        "currentView",
+        view,
+        "before closing the popup",
+    );
     await env.page.close();
     env.page = await openPopup(env.ctx, env.popupUrl);
-    await visible(env.page, restoredView);
+    await visible(env.page, "#view-" + view);
 }
 
 // The reproduction from the issue, step for step.
@@ -732,7 +766,7 @@ test("Back after reopening the popup renders the address screen (#268)", async (
     await env.page.click("#btn-settings");
     await visible(env.page, "#view-settings");
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     await env.page.click("#btn-settings-back");
     await visible(env.page, "#view-address");
@@ -789,10 +823,18 @@ test("Back after reopening the popup renders the Receive screen (#268)", async (
             JSON.stringify(before.address),
     );
 
+    // The test above left `settings` saved too, so reopenPopup() could not
+    // tell this page's save of it from that one without a save in between.
+    await waitForPersisted(
+        env.page,
+        "currentView",
+        "receive",
+        "on the Receive screen",
+    );
     await env.page.click("#btn-settings");
     await visible(env.page, "#view-settings");
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     await env.page.click("#btn-settings-back");
     await visible(env.page, "#view-receive");
@@ -1175,11 +1217,24 @@ const NONDEFAULT_NETWORK = "sepolia";
 test("the theme and network selectors carry a non-default persisted value (#229)", async (env) => {
     await openSettings(env.page);
 
-    // selectOption() fires "change", which is what the handlers bind.
+    // selectOption() fires "change", which is what the handlers bind. It
+    // returns before the handler's save lands, hence each wait.
     await env.page.selectOption("#settings-theme", NONDEFAULT_THEME);
+    await waitForPersisted(
+        env.page,
+        "theme",
+        NONDEFAULT_THEME,
+        "after the switch",
+    );
     await env.page.selectOption("#settings-network", NONDEFAULT_NETWORK);
+    await waitForPersisted(
+        env.page,
+        "networkId",
+        NONDEFAULT_NETWORK,
+        "after the switch",
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     assertSelectors(
         await selectorValues(env.page),
@@ -1198,9 +1253,16 @@ test("the theme and network selectors carry a non-default persisted value (#229)
     // fixture never customised and so are the mainnet defaults
     // src/shared/state.js starts with.
     await env.page.selectOption("#settings-theme", "system");
+    await waitForPersisted(env.page, "theme", "system", "after the restore");
     await env.page.selectOption("#settings-network", "mainnet");
+    await waitForPersisted(
+        env.page,
+        "networkId",
+        "mainnet",
+        "after the restore",
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     assertSelectors(
         await selectorValues(env.page),
@@ -1225,8 +1287,14 @@ test("a spam filter toggled in Settings survives a popup reopen (#229)", async (
         immediately.checked === false,
         "clicking #" + TOGGLED_FILTER + " did not clear it",
     );
+    await waitForPersisted(
+        env.page,
+        "hideDustTransactions",
+        false,
+        "after clearing #" + TOGGLED_FILTER,
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     const after = await checkboxStates(env.page);
     for (const { id } of SPAM_FILTER_CHECKBOXES) {
@@ -1243,8 +1311,14 @@ test("a spam filter toggled in Settings survives a popup reopen (#229)", async (
 test("turning the same filter back on survives a reopen too (#229)", async (env) => {
     await openSettings(env.page);
     await env.page.click("#" + TOGGLED_FILTER);
+    await waitForPersisted(
+        env.page,
+        "hideDustTransactions",
+        true,
+        "after setting #" + TOGGLED_FILTER + " again",
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     // Restores the fixture the later sections inherit, and rules out a
     // checkbox that persists "off" only because it is stuck there.
@@ -2365,7 +2439,7 @@ test("a token whose symbol() returns markup renders as text (#307)", async (env)
 
     // Close and reopen so the refresh that runs on open fetches balances
     // with the hostile symbol in them.
-    await reopenPopup(env, "#view-address");
+    await reopenPopup(env, "address");
     await env.page.waitForFunction(
         (addr) =>
             !!document.querySelector(
@@ -2431,7 +2505,7 @@ test("a token whose symbol() returns markup renders as text (#307)", async (env)
     // Put the fixture back before the next test reads it, and let the
     // stored balances be rewritten with the honest symbol.
     env.routeOpts.tokenSymbolOverride = null;
-    await reopenPopup(env, "#view-main");
+    await reopenPopup(env, "main");
     await env.page.waitForFunction(
         (addr) => {
             const row = document.querySelector(