Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions app/pages/project/instances/instance/tabs/StorageTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@ export function Component() {
[disks.items, instance.bootDiskId]
)

// Needed to keep them the same while setting boot disk.
// Extracted to keep dep array appropriately zealous.
const { ncpus, memory } = instance

const makeBootDiskActions = useCallback(
(disk: InstanceDisk): MenuAction[] => [
getSnapshotAction(disk),
Expand All @@ -168,8 +164,8 @@ export function Component() {
path: { instance: instance.id },
body: {
bootDisk: undefined,
ncpus,
memory,
ncpus: instance.ncpus,
memory: instance.memory,
// this would get unset if we left it out
autoRestartPolicy: instance.autoRestartPolicy,
},
Expand Down Expand Up @@ -200,7 +196,16 @@ export function Component() {
onActivate() {}, // it's always disabled, so noop is ok
},
],
[instanceUpdate, instance, getSnapshotAction, ncpus, memory]
[
instanceUpdate,
// don't put the entire instance in here. it is not referentially
// stable across polls, so the menus will close during polling
instance.id,
instance.autoRestartPolicy,
instance.ncpus,
instance.memory,
getSnapshotAction,
]
)

const makeOtherDiskActions = useCallback(
Expand All @@ -223,8 +228,8 @@ export function Component() {
path: { instance: instance.id },
body: {
bootDisk: disk.id,
ncpus,
memory,
ncpus: instance.ncpus,
memory: instance.memory,
// this would get unset if we left it out
autoRestartPolicy: instance.autoRestartPolicy,
},
Expand Down Expand Up @@ -262,7 +267,18 @@ export function Component() {
},
},
],
[detachDisk, instanceUpdate, instance, getSnapshotAction, bootDisks, ncpus, memory]
[
detachDisk,
instanceUpdate,
// don't put the entire instance in here. it is not referentially
// stable across polls, so the menus will close during polling
instance.id,
instance.autoRestartPolicy,
instance.ncpus,
instance.memory,
getSnapshotAction,
bootDisks,
]
)

const attachDisk = useApiMutation('instanceDiskAttach', {
Expand Down
44 changes: 44 additions & 0 deletions test/e2e/instance-disks.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
*/
import {
clickRowAction,
closeToast,
expect,
expectNoToast,
expectNotVisible,
expectRowVisible,
expectToast,
expectVisible,
openRowActions,
stopInstance,
test,
} from './utils'
Expand Down Expand Up @@ -224,3 +226,45 @@ test('Change boot disk', async ({ page }) => {

await expect(page.getByText('Attach a disk to be able to set a boot disk')).toBeVisible()
})

// silly test but we've reintroduced this bug like 3 times
test("polling doesn't close row actions menu", async ({ page }) => {
await page.goto('/projects/mock-project/instances/db1')

// stop, but don't wait until the state has changed
await page.getByRole('button', { name: 'Stop' }).click()
await page.getByRole('button', { name: 'Confirm' }).click()
await closeToast(page)

const menu = page.getByRole('menu')
const stopped = page.getByText('statestopped')

await expect(menu).toBeHidden()
await expect(stopped).toBeHidden()

await openRowActions(page, 'disk-1')
await expect(stopped).toBeHidden() // still not stopped yet
await expect(menu).toBeVisible()

// now we're stopped, which means polling has happened, but the
// menu remains visible
await expect(stopped).toBeVisible()
await expect(menu).toBeVisible()

// now start it so we can check the non-boot disks table
await page.getByRole('button', { name: 'Start' }).click()
await page.getByRole('button', { name: 'Confirm' }).click()
await closeToast(page)

const running = page.getByText('staterunning') // not running yet
await expect(running).toBeHidden()
await expect(menu).toBeHidden()

await openRowActions(page, 'disk-2')
await expect(running).toBeHidden() // still not running yet
await expect(menu).toBeVisible()

// state change means polling has happened. menu is still visible
await expect(running).toBeVisible()
await expect(menu).toBeVisible()
})
4 changes: 4 additions & 0 deletions test/e2e/instance.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,7 @@
state: expect.stringMatching(/^starting\d+s$/),
})
})

test("polling doesn't close row actions menu", async ({ page }) => {

Check warning on line 244 in test/e2e/instance.e2e.ts

View workflow job for this annotation

GitHub Actions / ci

Test has no assertions

Check warning on line 244 in test/e2e/instance.e2e.ts

View workflow job for this annotation

GitHub Actions / ci

Test has no assertions
await page.goto('/projects/mock-project/instances/db1')
})
Loading