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.

Sign up for GitHub

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

Jump to bottom

Account.test.tsx flakiness #41402

Closed
ravicious opened this issue May 10, 2024 · 3 comments · Fixed by #41481
Closed

Account.test.tsx flakiness #41402

ravicious opened this issue May 10, 2024 · 3 comments · Fixed by #41481
Assignees
@bl-nero
Labels
flaky tests

Comments

@ravicious
Copy link
Member

Failure

Link(s) to logs

  • No logs because I was running this locally.
for i in {1..100}; do yarn test web/packages/teleport/src/Account/Account.test.tsx  || {echo "Failed after $i attempts" && break}; done

Relevant snippet

 FAIL  web/packages/teleport/src/Account/Account.test.tsx
  ● passkey + mfa button state: 2fa(optional) with pwdless(true) => passkey(true) mfa(true)

    expect(received).toBe(expected) // Object.is equality

    Expected: null
    Received: ""

      79 |
      80 |     // eslint-disable-next-line jest-dom/prefer-to-have-attribute
    > 81 |     expect(screen.getByText(/add a passkey/i).getAttribute('disabled')).toBe(
         |                                                                         ^
      82 |       pkEnabled ? null : ''
      83 |     );
      84 |

      at toBe (web/packages/teleport/src/Account/Account.test.tsx:81:73)
@ravicious
Copy link
Member Author

Got another failure after 64 attempts. I suspect it might be a similar issue to #41401. Maybe renderComponent could wait for something to be in the document rather than waiting for the lack of it?

Also, this line should probably use getBy to fail immediately:

    const statePill = screen.queryByTestId('passwordless-state-pill');
Passkey state pill: passwordless=true, 1 passkey(s) => state='active'

    expect(received).toHaveTextContent()

    received value must be a Node.
    Received has value: null

      122 |       // Note: every alternative approach is even more complicated.
      123 |       // eslint-disable-next-line jest/no-conditional-expect
    > 124 |       expect(statePill).toHaveTextContent(state);
          |                         ^
      125 |     } else {
      126 |       // eslint-disable-next-line jest/no-conditional-expect
      127 |       expect(statePill).not.toBeInTheDocument();

      at __EXTERNAL_MATCHER_TRAP__ (node_modules/expect/build/index.js:325:30)
      at Object.throwingMatcher [as toHaveTextContent] (node_modules/expect/build/index.js:326:15)
      at toHaveTextContent (web/packages/teleport/src/Account/Account.test.tsx:124:25)

@bl-nero
Copy link
Contributor

bl-nero commented May 13, 2024

I think I know what the problem is. Incidentally, I had this piece (hopefully) fixed in one of my branches that didn't yet make it to the code review.

@bl-nero
Copy link
Contributor

bl-nero commented May 13, 2024

(The problem is probably caused not by the fact that we're waiting for a component to disappear, but by the fact that spinners have a built-in small delay to prevent them from showing up every time.)

@bl-nero bl-nero mentioned this issue May 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bl-nero bl-nero

Labels
flaky tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update the account screen loading state gravitational/teleport
2 participants
@ravicious @bl-nero