chore(tests): Fix uncalled .toBeInTheDocument and .toBeTruthy referen…

[pdehaan]

…ces in tests

Because

• .toBeInDocument should be .toBeInDocument(), etc.

This pull request

• Fixes the errors, but doesn't add ESLint rules to catch future breakages, I'll file that as a separate issue.

Issue that this pull request solves

Closes: #12868

Checklist

Put an x in the boxes that apply

• My commit is GPG signed.
• If applicable, I have modified or added tests which pass locally.
• I have added necessary documentation (if appropriate).
• I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

[dschom]

LGTM!

[LZoog]

I know I mentioned this on Slack but just also calling out here that test_pull_request didn't run. I'm pretty certain this one doesn't pass locally since I tried fixing that one before.

We had some issues with CI running last week, I can't tell if that's what's happened here or if it didn't run since you're not in fxa-devs? 🤔 But either way, if you run yarn test in fxa-settings you should see at least that one fail (unless I'm somehow mistaken on that not passing). If that's something you can investigate I can come back and re-review/pull your commit into a different branch, or otherwise, we can have a separate ticket for just that one failure and I can r+ the rest.

[pdehaan]

OK, not what I expected...
Getting 6 failures locally and they all seem to be related to things being null:

  ● BentoMenu › renders and toggles as expected with default values

    expect(received).toBeInTheDocument()

    received value must be an HTMLElement or an SVGElement.
    Received has value: null

      24 |     fireEvent.click(toggleButton);
      25 |     expect(toggleButton).toHaveAttribute('aria-expanded', 'true');
    > 26 |     expect(dropDown).toBeInTheDocument();
         |                      ^
      27 |
      28 |     fireEvent.click(toggleButton);
      29 |     expect(toggleButton).toHaveAttribute('aria-expanded', 'false');

      at __EXTERNAL_MATCHER_TRAP__ (../../node_modules/react-scripts/node_modules/expect/build/index.js:342:30)
      at Object.toBeInTheDocument (../../node_modules/react-scripts/node_modules/expect/build/index.js:343:15)
      at Object.<anonymous> (src/components/BentoMenu/index.test.tsx:26:22)

So it seems to pass if I change it to expect(dropDown).toBeNull(), but not sure if that's correct/expected here and 5 other places:

• src/components/DropDownAvatarMenu/index.test.tsx:64
• src/components/DropDownAvatarMenu/index.test.tsx:95
• src/components/DropDownAvatarMenu/index.test.tsx:113
• src/components/BentoMenu/index.test.tsx:26
• src/components/BentoMenu/index.test.tsx:38
• src/components/BentoMenu/index.test.tsx:54

received value must be an HTMLElement or an SVGElement.
Received has value: null

[LZoog]

Yeah, those should not be null. That's what I was running into as well.

If you revert the BentoMenu and DropDownAvatarMenu changes, I can pull your commit into another PR for you (we actually have docs on how to do this here FWIW), and then we can have a separate issue to fix those remaining ones.

[pdehaan]

/giphy "As you wish"

git grep -n ".toBe" packages | grep -Ev "\.toBe.*\("

packages/fxa-settings/src/components/BentoMenu/index.test.tsx:26:    expect(dropDown).toBeInTheDocument;
packages/fxa-settings/src/components/BentoMenu/index.test.tsx:38:    expect(dropDown).toBeInTheDocument;
packages/fxa-settings/src/components/BentoMenu/index.test.tsx:54:    expect(dropDown).toBeInTheDocument;

packages/fxa-settings/src/components/DropDownAvatarMenu/index.test.tsx:64:    expect(dropDown).toBeInTheDocument;
packages/fxa-settings/src/components/DropDownAvatarMenu/index.test.tsx:95:    expect(dropDown).toBeInTheDocument;
packages/fxa-settings/src/components/DropDownAvatarMenu/index.test.tsx:113:    expect(dropDown).toBeInTheDocument;

[LZoog]

@pdehaan Running CI now. When you see it passes, do you mind fixing up your previous commit so there's only one that will be merged? When you force push CI won't run again but that's fine, will r+/merge and close #12965

[LZoog]

@pdehaan I think you need to rebase against main for CI to get the sr-LATN fix. Do you mind rebasing & squashing your commits, and then I'll pull that commit into that other branch?

Sorry this is turning into so much trouble for what this fixes 🤣

[LZoog]

@pdehaan Will you file that follow up to fix the tests that failed? Thank you! 🙏

[pdehaan]

@pdehaan Will you file that follow up to fix the tests that failed? Thank you! 🙏

Filed as #12978