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

Fix gazebo AccountSettings flaky test #2270

Closed
suejung-sentry opened this issue Aug 13, 2024 · 1 comment · Fixed by codecov/gazebo#3131
Closed

Fix gazebo AccountSettings flaky test #2270

suejung-sentry opened this issue Aug 13, 2024 · 1 comment · Fixed by codecov/gazebo#3131
Labels
bug Something isn't working

Comments

@suejung-sentry
Copy link

suejung-sentry commented Aug 13, 2024

Describe the bug
It seems we have a flaky test in App cloud routing renders the AccountSettings page. I see it appear once every ~10 runs of the CI.

Here's a previous solution that seems like it helped but maybe the issue is back (?) - https://github.com/codecov/gazebo/pull/2951/files

Here's an example of the test failure - codecov/gazebo#3118 (comment)
Image

To Reproduce
Steps to reproduce the behavior:

  1. Have our test suite run many times and once in a while you may see test above fail. I have never seen the issue in local, but it shows up every once in a while if you have the test suite run in CI

Expected behavior
A clear and concise description of what you expected to happen.

Expect that test not to fail

Screenshots
If applicable, add screenshots to help explain your problem.

See above in description

Desktop (please complete the following information):

  • OS: [e.g. iOS] mac m1 pro
  • Browser [e.g. chrome, safari] chrome
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@suejung-sentry suejung-sentry added the bug Something isn't working label Aug 13, 2024
@suejung-sentry suejung-sentry changed the title Bug: Flaky test AccountSettings Fix AccountSettings flaky test Aug 13, 2024
@suejung-sentry suejung-sentry changed the title Fix AccountSettings flaky test Fix gazebo AccountSettings flaky test Aug 13, 2024
@suejung-sentry
Copy link
Author

https://sentry.slack.com/archives/C04L2RSEY3H/p1723589627601339

Suejung Shin
Tuesday at 3:53 PM
hey i've spun off this ticket for a potentially flaky test in gazebo. I saw it fail in 2 commits yesterday (resolves after re-run)
#2270
can try to grab it next sprint maybe? sharing in case you see it too "AccountSettings"
:plus1:
1

Calvin Yau
🚌 Tuesday at 3:59 PM
It looks like it's trying to confirm this route.
[
{
testLabel: 'AccountSettings',
pathname: '/account/gh/codecov',
expected: {
page: /AccountSettings/i,
location: '/account/gh/codecov',
},
},
],
If I try to go to /account/gh/codecov, I just get redirected to https://app.codecov.io/account/gh/codecov/yaml/. Is this due to my permissions? If anyone else is able to access, let me know (edited)
👀
1

Spencer Murray
Yesterday at 5:33 AM
it's permissions yea

Spencer Murray
Yesterday at 5:34 AM
have to be org admin

Spencer Murray
Yesterday at 5:34 AM
This test used to be a lot flakier. I've spend a bit of time looking into it... I have NO idea why it's flaky.
:thinkspin:
1

Ajay Singh
:bufo-waddle: Today at 8:33 AM
https://github.com/codecov/gazebo/actions/runs/10406104181/job/28818425445

Ajay Singh
:bufo-waddle: Today at 8:33 AM
Anotha one

Ajay Singh
:bufo-waddle: Today at 8:33 AM
How much value are we actually getting from this test :thinking_face: 😬

Spencer Murray
Today at 8:34 AM
negative

Ajay Singh
:bufo-waddle: Today at 8:38 AM
facts
Also sent to the channel

Ajay Singh
:bufo-waddle: Today at 8:44 AM
anyone not in favor of nuking it?

Spencer Murray
Today at 8:48 AM
u want to ignore it or actually nuke it?

Spencer Murray
Today at 9:09 AM
This test seems to never fail locally

Spencer Murray
Today at 9:09 AM
does that tell us anything?

Ajay Singh
:bufo-waddle: Today at 9:12 AM
That's a good point, when we run the tests locally are we using multiple workers or a single one?

Spencer Murray
Today at 9:12 AM
it's running parallel tests

Spencer Murray
Today at 9:12 AM
but not separate in the same way (edited)

Spencer Murray
Today at 9:13 AM
I'm whipping up a PR to try to debug this a bit more

Spencer Murray
Today at 9:13 AM
during all hands lol

Ajay Singh
:bufo-waddle: Today at 9:20 AM
Wish I had wifi on the train :sadge:

Spencer Murray
Today at 9:23 AM
https://github.com/codecov/gazebo/actions/runs/10406927442/job/28821203998

Spencer Murray
Today at 9:23 AM
i've run the test file 25 times on its own and no fails

Spencer Murray
Today at 9:24 AM
which kind of makes me think there's some other test that's spilling over or somethign

Spencer Murray
Today at 9:24 AM
interestingly the flaky test is the very first test in that file

Spencer Murray
Today at 9:24 AM
So what I'm going to do next is figure out the test file that runs immediately before App.spec.tsx and add that to my test pr

Ajay Singh
:bufo-waddle: 1 hour ago
That's a solid workflow, maybe ordering of the cases could be part of it too? :thonk:

Spencer Murray
1 hour ago
I'm playing around still haven't found cause yet. Will take ordering under consideration

Ajay Singh
:bufo-waddle: 1 hour ago
When the test runs, does it also call the hooks for each page?

Spencer Murray
1 hour ago
no they're all completely mocked out

Ajay Singh
:bufo-waddle: 1 hour ago
Any hook that has an unstable mock maybe?

Spencer Murray
1 hour ago
Screenshot 2024-08-15 at 12.52.54.png

Screenshot 2024-08-15 at 12.52.54.png

Ajay Singh
:bufo-waddle: 1 hour ago
Ah okay gotcha :thinking_face:

Ajay Singh
:bufo-waddle: 1 hour ago
Account settings page rendered conditionally?

Spencer Murray
44 minutes ago
sorry you sent me down a rabbit hole lol

Suejung Shin
44 minutes ago
Oh this game looks fun lol.
Maybe it's something about overwritten global variables when you run a describe.each on a list and set a beforeEach that tries to overwrite the same vars

Spencer Murray
43 minutes ago
My current question:
https://github.com/codecov/gazebo/actions/runs/10406104181/job/28818425445
In this failure, the DOM when the test fails is rendering our Loader. The Loader should not be able to be in the dom. Interestingly the Loader is called inside of AccountSettings.
👀
1

Spencer Murray
42 minutes ago
Is the mock not working?

Spencer Murray
42 minutes ago
something to do with mocking lazy imports?
:bufo-hmm:
2

Spencer Murray
39 minutes ago
https://stackoverflow.com/questions/53189059/how-to-test-snapshots-with-jest-and-new-react-lazy-16-6-api
Stack OverflowStack Overflow
How to test snapshots with Jest and new React lazy 16.6 API
I have to components imported with the new React lazy API (16.6).
import React, {PureComponent, lazy} from 'react';
const Component1 = lazy(() => import('./Component1'));
const Component2 = l...

Spencer Murray
39 minutes ago
I'm adding suspense to the app.spec.tsx wrapper
😮
1

Spencer Murray
39 minutes ago
:prayge: this is the issue
:prayge:
2

Spencer Murray
38 minutes ago
Oh this game looks fun lol.
Maybe it's something about overwritten global variables when you run a describe.each on a list and set a beforeEach that tries to overwrite the same vars
This is what I was thinking also. I wasn't able to reproduce the flake locally though.
:sadpepe:
1

Spencer Murray
37 minutes ago
First run with fix passed
🤞🤞:skin-tone-2:
3

Spencer Murray
37 minutes ago
will run a bunch more times to be safe

Spencer Murray
36 minutes ago
codecov/gazebo#3131

Spencer Murray
36 minutes ago
tune in live. Test runner #0 is the one we're watching

Spencer Murray
36 minutes ago
lol

Spencer Murray
34 minutes ago
rip
Screenshot 2024-08-15 at 13.10.41.png

Screenshot 2024-08-15 at 13.10.41.png

Spencer Murray
34 minutes ago
unlucky

Ajay Singh
:bufo-waddle: 23 minutes ago
GUH

Suejung Shin
22 minutes ago
guess we can go with skipping that test, gather more data? 😞
it's possible them the next one will be the flaky one, then maybe we find it's an issue with the /logout one before it or something
but at least it saves us a little time (edited)

2

Ajay Singh
:bufo-waddle: 20 minutes ago
That sounds like a solid plan, we'll get more info if the test is a troublesome one in isolation, or just the "unlucky" one from an upstream flake

Spencer Murray
16 minutes ago
agreed

Spencer Murray
16 minutes ago
I'll add the ignore

Spencer Murray
8 minutes ago
codecov/gazebo#3131

Image
Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant