Setup user-event and change some fireEvent to userEvent in tests

[djalmaaraujo]

Description

React Testing Library UserEvent API seems to have a better API than fireEvent to deal with user events. @brookewp mentioned this during a call with us, and I believe we should change some tests to have a better coverage of user events.

Checklist

• This PR has good automated test coverage
• The storybook for the component has been updated

Steps to Test

1. Pull down PR.
2. npm run install.
3. npm run test.
4. Tests should pass.

[netlify]

✅ Deploy Preview for vip-design-system-components ready!

Name	Link
🔨 Latest commit	9470f80
🔍 Latest deploy log	https://app.netlify.com/sites/vip-design-system-components/deploys/660d47db16f71100083ee644
😎 Deploy Preview	https://deploy-preview-371--vip-design-system-components.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[djalmaaraujo]

We also disable this in the VIP dashboard, making it impossible to pass tests using user events.

[brookewp]

When you replaced fireEvent with userEvent (i.e. in the Accordion test), the test would fail unless this rule was disabled? I haven't been able to replicate the error so I'm curious if I'm missing something here.

[djalmaaraujo]

Yes, it's an issue that the community still needs to solve. I tried many different things and didn't work. Many TS errors cannot be solved.

testing-library/user-event#1146

[brookewp]

Thanks for the context!

[brookewp]

Not a cause of this PR, but I noticed the comments are mixed up in the second test. It should be:

diff --git a/src/system/Accordion/Accordion.test.tsx b/src/system/Accordion/Accordion.test.tsx
index cc9d93a..cbd460c 100644
--- a/src/system/Accordion/Accordion.test.tsx
+++ b/src/system/Accordion/Accordion.test.tsx
@@ -52,11 +52,11 @@ describe( '<Accordion />', () => {
 
-		// Should find the open content
+		// Should not find the closed content
 		expect( screen.queryByText( 'content one' ) ).toBeNull();
 
-		// Should not find the closed content
-		expect( screen.queryByText( 'content two' ) ).toBeInTheDocument();
+		// Should find the open content
+		expect( screen.getByText( 'content two' ) ).toBeInTheDocument();

We could also change the queryBy to getBy for the second assertion since I believe queryBy is usually used when we expect it not to be found.

[brookewp]

This could be a follow-up, but if we're already editing the test... 😄 we could make the test more robust by making it more specific and checking against things like the expanded/open state which would affect the user's experience. Also asserting toBeVisible, which checks if it's in the document like toBeInTheDocument, but it also checks to ensure it's not hidden via attributes or CSS.

diff --git a/src/system/Accordion/Accordion.test.tsx b/src/system/Accordion/Accordion.test.tsx
index d770ccc..26d5f34 100644
--- a/src/system/Accordion/Accordion.test.tsx
+++ b/src/system/Accordion/Accordion.test.tsx
@@ -51,13 +51,20 @@ describe( '<Accordion />', () => {
 
 		const { container } = renderComponent();
 
-		await user.click( screen.getByRole( 'button', { name: 'trigger two' } ) );
-
-		// Should find the open content
-		expect( screen.queryByText( 'content one' ) ).toBeNull();
+		await user.click( screen.getByRole( 'button', { name: 'trigger two', expanded: false } ) );
 
 		// Should not find the closed content
-		expect( screen.queryByText( 'content two' ) ).toBeInTheDocument();
+		expect(
+			screen.getByRole( 'button', { name: 'trigger one', expanded: false } )
+		).toHaveAttribute( 'data-state', 'closed' );
+		expect( screen.queryByText( 'content one' ) ).not.toBeInTheDocument();
+
+		// Should find the open content
+		expect( screen.getByRole( 'button', { name: 'trigger two', expanded: true } ) ).toHaveAttribute(
+			'data-state',
+			'open'
+		);
+		expect( screen.getByText( 'content two' ) ).toBeVisible();
 
 		// Check for accessibility issues
 		expect( await axe( container ) ).toHaveNoViolations();

[brookewp]

Thank you for opening this to implement userEvent! 🎉 Are the CSS changes a result of auto-formatting based on some rules?

[djalmaaraujo]

Thank you for opening this to implement userEvent! 🎉 Are the CSS changes a result of auto-formatting based on some rules?

CSS changes were a minor thing I ended up fixing because the tests were warning me something annoying in the terminal.

[brookewp]

Thanks again for adding this! 😄 Tests are passing! 🚀