Skip to content

Commit a2773c4

Browse files
authored
Fix NIC e2e test flake in safari (#3198)
Test failure in #3195 repros locally no problem: <img width="747" height="409" alt="image" src="https://github.com/user-attachments/assets/d1fed181-f26c-455b-a0ab-c8f8e962c1ae" /> Claude's speculation on why it suddenly became a problem after #3178 is good enough for me. This is the second or third time lately we've run into small issues because we're not awaiting `queryClient.invalidateEndpoint` in the mutation handlers. Something to keep an eye on. > The race: `NetworkingTab` gates the Delete row action on `nic.primary && multipleNics`, where `multipleNics = nics.length > 1`. When the `my-nic` delete mutation succeeds, `onSuccess` synchronously calls `queryClient.invalidateEndpoint` (not awaited) and closes the confirm modal. If the test opens nic-3's row-actions menu before the refetch lands, `nics.length` is still 2, so Delete renders disabled and wrapped in a `<Wrap with={<Tooltip/>}>`. When the refetch then lands mid-click, the `<Wrap>` unwraps, the `Menu.Item` remounts (old DOM detached), and on Safari/Firefox base-ui's Menu treats that focus/DOM churn as a close — so Playwright's retry never resolves a clickable Delete and hits the 60s timeout. > > When it was introduced: the disable rule landed April 2025 in #2806 ("Disable primary NIC delete when multiple NICs present"). The `DropdownMenu.Item` `<Wrap>`-with-Tooltip pattern has been in place since ~March 2025. So the race has been latent for about a year. > > Why it's surfacing now: first CI appearance was on the #3178 "Responsive grid layout" PR (2026-04-21), which landed on trunk 2026-04-22; every workflow run after that hits it. #3178 moved TopBar/Sidebar from CSS-grid cells to fixed-position elements, switched to document-level scroll, removed `overflow-y-hidden` on body, and set `height: 100%` on html/body/#root. None of that touches the NIC flow, but it changed paint/layout timing around modal open/close and table reflow. That shifted the window such that `expect(my-nic cell).toBeHidden()` now reliably resolves while the page is briefly inert during the modal's close transition — before the refetch has actually landed — so the test races into opening nic-3's menu in the stale state. The layout PR didn't introduce the bug; it just removed the timing cushion that had been hiding it.
1 parent 4a9be31 commit a2773c4

1 file changed

Lines changed: 7 additions & 1 deletion

File tree

test/e2e/instance-networking.e2e.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,13 @@ test('Instance networking tab — NIC table', async ({ page }) => {
100100
await clickRowAction(page, 'my-nic', 'Delete')
101101
await expect(page.getByText('Are you sure you want to delete my-nic?')).toBeVisible()
102102
await page.getByRole('button', { name: 'Confirm' }).click()
103-
await expect(page.getByRole('cell', { name: 'my-nic' })).toBeHidden()
103+
// Wait for the NIC list refetch to land before opening nic-3's menu.
104+
// Otherwise `multipleNics` is still true, Delete renders disabled (wrapped
105+
// in a tooltip), and the unwrap when the refetch lands detaches the
106+
// menuitem and closes the menu on Safari/FF. Row count (vs. checking my-nic
107+
// is gone) because any absence check passes transiently while the confirm
108+
// modal closes and the page is inert.
109+
await expect(nicTable.getByRole('row')).toHaveCount(2) // header + nic-3
104110

105111
// Now the primary NIC is deletable
106112
await clickRowAction(page, 'nic-3', 'Delete')

0 commit comments

Comments
 (0)