From 955386b63fcb8f42d6f77b10a188f00ddab802ed Mon Sep 17 00:00:00 2001
From: Alex B <142788550+A13xB0@users.noreply.github.com>
Date: Sun, 11 Oct 2026 12:28:25 +0100
Subject: [PATCH] test(map): settle the router's focus frame before the
 teardown snapshot (#2179)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

Fixes an intermittent failure in
`tests/e2e/test-map-nodes-pagination-e2e.js`, the "late response after
map teardown" cases added for #2030 in #2048:

```
✗ late /observers response after map teardown: old response must not change the destination page
```

It failed on #2164 ([run
38080812509](https://github.com/Kpa-clawbot/CoreScope/actions/runs/38080812509),
shard 2/3), a Go-only change, and passed on the rerun.

## Cause

`checkMapTeardown` compares `#app`'s HTML before and after releasing the
old map's pending response. When it failed, the only difference was on
the destination page's heading:

```
before: <div class="tools-landing"><h2>Tools</h2>…
after:  <div class="tools-landing"><h2 tabindex="-1">Tools</h2>…
```

That attribute comes from the router, not the old response. Since #630
(`app.js`, "SPA focus management") it focuses the new page's first
heading in a `requestAnimationFrame` after the page renders. The
before-snapshot was taken as soon as `#leaflet-map` detached, sometimes
ahead of that frame.

## Fix

The test now waits two frames before the before-snapshot, the same
settle it already does before the after-snapshot. The router's frame is
queued by then, so it always lands first. The change is test-only: no
app code changes, and every assertion stays as it was.

## Tests

Run locally with the e2e fixture (`tools/freshen-fixture.sh`, migrated)
served by this branch's server, headless Chromium:

| | runs | failed |
|---|---|---|
| test on `master` (`352a0ccd`) | 20 | 2 |
| this branch | 50 | 0 |

- **Before:** the two failures on `master` were "old response must not
change the destination page" (teardown, `/config/regions` and `nodes`),
plus one 30 s timeout in an "after map replacement" case in the same
runs.
- **After:** none of these failures or the timeout came back in 50 runs.

## Checks

- **Lint:** ESLint passes with the repo config and with Node globals.
SonarQube rules on the file report nothing on the added lines; its four
existing minor findings are on lines this PR doesn't touch.
- **Repo rules:** one logical change, explicit `git add`, no private
data, no new dependencies. This is a test-only fix, so there's no
red/green pair: the "red" is the intermittent failure above, measured as
2 in 20.

Related: #2030, #2048 (where the test was added).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
---
 tests/e2e/test-map-nodes-pagination-e2e.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/e2e/test-map-nodes-pagination-e2e.js b/tests/e2e/test-map-nodes-pagination-e2e.js
index 0a9de850..381c58cc 100644
--- a/tests/e2e/test-map-nodes-pagination-e2e.js
+++ b/tests/e2e/test-map-nodes-pagination-e2e.js
@@ -104,6 +104,11 @@ async function checkMapTeardown(browser, stage, revisit) {
     await page.evaluate(() => { location.hash = '#/map'; });
     await page.waitForSelector('#leaflet-map[data-loaded="true"]');
   }
+  // The router focuses the new page's heading one frame after it renders
+  // (app.js, #630-7), adding tabindex="-1". That frame is already queued by
+  // now, so two more let it land before the snapshot; otherwise the
+  // comparison below can catch the router's change, not the old response.
+  await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))));
   const before = await page.evaluate(() => ({ nodes: JSON.stringify(window.__mc_nodes), html: document.getElementById('app').innerHTML }));
   await page.evaluate(async () => {
     window.__pendingMapResponse();