Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test flake: create instance with additional disks in safari #2694

Open
david-crespo opened this issue Feb 13, 2025 · 4 comments
Open

Test flake: create instance with additional disks in safari #2694

david-crespo opened this issue Feb 13, 2025 · 4 comments

Comments

@david-crespo
Copy link
Collaborator

We've seen this one on and off for a while.

https://github.com/oxidecomputer/console/actions/runs/13317596534/job/37195314937

    Error: Timed out 5000ms waiting for expect(locator).toHaveValue(expected)

    Locator: getByRole('dialog', { name: 'Create disk' }).getByRole('textbox', { name: 'Size (GiB)' })
    Expected string: "5"
    Received string: "10"
    Call log:
      - expect.toHaveValue with timeout 5000ms
      - waiting for getByRole('dialog', { name: 'Create disk' }).getByRole('textbox', { name: 'Size (GiB)' })
        8 × locator resolved to <input id=":r73:" value="10" type="text" autocorrect="off" autocomplete="off" spellcheck="false" inputmode="numeric" aria-labelledby=":r73:-label" aria-roledescription="Number field" class="w-full rounded border-none px-3 py-[0.6875rem] !outline-offset-1 text-sans-md text-raise bg-default placeholder:text-tertiary focus:outline-none disabled:cursor-not-allowed disabled:text-secondary disabled:bg-disabled"/>
          - unexpected value "10"


      555 |   const sizeField = createForm.getByRole('textbox', { name: 'Size (GiB)' })
      556 |   await sizeField.fill('5')
    > 557 |   await expect(sizeField).toHaveValue('5')
          |                           ^
      558 |
      559 |   await createForm.getByRole('button', { name: 'Create disk' }).click()
      560 |   await expect(createForm.getByText('Name is already in use')).toBeVisible()
        at /Users/runner/work/console/console/test/e2e/instance-create.e2e.ts:557:27
@david-crespo
Copy link
Collaborator Author

Seeing this one a lot now. It's the only flake we're seeing consistently.

https://github.com/oxidecomputer/console/actions/runs/13800679236/job/38602321597

@benjaminleonard
Copy link
Contributor

benjaminleonard commented Mar 12, 2025

That field will only update on blur right? Wonder if we still get the flake if we add something to move focus out of the field, allow it to update and then run the toHaveValue.

Edit: Is is possible that submitting the form happens so quickly it also does so before it can update?

@david-crespo
Copy link
Collaborator Author

I can get an occasional failure locally with this. It fails a few lines lower, but I think it's for the same reason.

npx playwright test --project=safari instance-create -g 'with additional disks' --repeat-each 20
 2) [safari] › test/e2e/instance-create.e2e.ts:538:1 › create instance with additional disks ──────

    Error: expect(received).toEqual(expected) // deep equality

    Expected: ArrayContaining [ObjectContaining {"Name": "new-disk-1", "Size": "5GiB", "Type": "create"}]
    Received: [{"Name": "new-disk-1", "Size": "10GiB", "Type": "create"}]

    Call Log:
    - Timeout 10000ms exceeded while waiting on the predicate

       at utils.ts:107

      105 |   // wait up to 5s for the row to be there
      106 |   // https://playwright.dev/docs/test-assertions#polling
    > 107 |   await expect
          |   ^
      108 |     // poll slowly because getRows is slow. usually under 200ms but still
      109 |     .poll(getRows, { intervals: [500] })
      110 |     .toEqual(expect.arrayContaining([expect.objectContaining(expectedRow)]))
        at expectRowVisible (/Users/david/oxide/console/test/e2e/utils.ts:107:3)
        at /Users/david/oxide/console/test/e2e/instance-create.e2e.ts:568:3

We're changing that field to 5, but somehow it's flipping back to 10 by the time we submit? I suspect things are just happening too fast and a very short sleep before submit would fix it.

2025-03-12-safari-flake.mp4

@david-crespo
Copy link
Collaborator Author

david-crespo commented Mar 12, 2025

Ok, well this is highly informative. When I add a 300ms sleep after the fill but before the check that the field has the 5 we entered, it fails almost every time. When I add a shorter sleep, it fails less often, and when I add a longer sleep it fails 100% of the time.

   const sizeField = createForm.getByRole('textbox', { name: 'Size (GiB)' })
   await sizeField.fill('5')
+  await sleep(500)
   await expect(sizeField).toHaveValue('5')
 
   await createForm.getByRole('button', { name: 'Create disk' }).click()

So it's the opposite of what I pictured: rather than failing when it's too fast, it only passes when it's fast enough. It fails when it's a little slower. This is consistent with the higher likelihood of failure in CI, which is generally slower than local.

Image

When I add the sleep before the field fill, it passes every time. No flakes!

   const sizeField = createForm.getByRole('textbox', { name: 'Size (GiB)' })
+  await sleep(1000)
   await sizeField.fill('5')
   await expect(sizeField).toHaveValue('5')
 
   await createForm.getByRole('button', { name: 'Create disk' }).click()

That is an immediate fix, but I'm spending a few minutes looking at what this form does on initial render that could be causing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants