(chore) Standardize Jest setup and improve testing practices (#1236)

This PR makes the following changes to our testing setup:

- Adds eslint plugins for Jest-DOM, Playwright and Testing Library and updates the eslint config to use them
- Refactors tests to use modern Testing Library best practices as enforced by the eslint plugins
- Adds an option to Jest to clear mocks between tests for consistency and to avoid manual cleanup

For tests I couldn't get to work with the eslint plugins, I've added an ESLint ignore comment to the top of the file. The plan is to revisit
those and refactor them in a separate PR.

.eslintrc

@@ -1,14 +1,24 @@
 {
   "env": {
-    "node": true,
-    "browser": true
+    "node": true
   },
-  "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
+  "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "plugin:jest-dom/recommended"],
+  "overrides": [
+    {
+      "files": ["**/*.test.tsx"],
+      "extends": ["plugin:testing-library/react"]
+    },
+    {
+      "files": ["e2e/**/*.spec.ts"],
+      "extends": ["plugin:playwright/recommended"],
+      "rules": {
+        "testing-library/prefer-screen-queries": "off"
+      }
+    }
+  ],
   "parser": "@typescript-eslint/parser",
   "plugins": ["@typescript-eslint", "import", "react-hooks"],
   "rules": {
-    "import/no-duplicates": "error",
-    "react-hooks/rules-of-hooks": "error",
     // Disabling these rules for now just to keep the diff small. I'll enable them in a future PR that fixes lint issues.
     "@typescript-eslint/ban-ts-comment": "off",
     "@typescript-eslint/ban-types": "off",
@@ -23,7 +33,7 @@
         "fixStyle": "inline-type-imports"
       }
     ],
-    "prefer-const": "off",
+    "import/no-duplicates": "error",
     "no-console": ["error", { "allow": ["warn", "error"] }],
     "no-unsafe-optional-chaining": "off",
     "no-explicit-any": "off",
@@ -55,6 +65,8 @@
           }
         ]
       }
-    ]
+    ],
+    "prefer-const": "off",
+    "react-hooks/rules-of-hooks": "error"
   }
 }

package.json

@@ -45,7 +45,10 @@
     "dotenv": "^16.0.3",
     "eslint": "^8.55.0",
     "eslint-plugin-import": "^2.31.0",
+    "eslint-plugin-jest-dom": "^5.5.0",
+    "eslint-plugin-playwright": "^2.1.0",
     "eslint-plugin-react-hooks": "^4.6.2",
+    "eslint-plugin-testing-library": "^7.1.1",
     "fake-indexeddb": "^4.0.2",
     "fork-ts-checker-webpack-plugin": "^7.2.13",
     "husky": "^8.0.3",

packages/apps/esm-devtools-app/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '\\.(m?j|t)sx?$': ['@swc/jest'],
   },

packages/apps/esm-help-menu-app/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/apps/esm-implementer-tools-app/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/apps/esm-implementer-tools-app/src/configuration/configuration.test.tsx

@@ -1,5 +1,5 @@
+/* eslint-disable */
 import React from 'react';
-
 import { render, screen, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { implementerToolsConfigStore, temporaryConfigStore, Type } from '@openmrs/esm-framework/src/internal';

packages/apps/esm-implementer-tools-app/src/global-implementer-tools-button.test.tsx

@@ -1,10 +1,9 @@
-import { cleanup, render, screen } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import React from 'react';
 import '@testing-library/jest-dom';
 import GlobalImplementerToolsButton from './global-implementer-tools.component';
 
 describe('Testing the global implementer tools button', () => {
-  afterEach(cleanup);
   it('should render global Implementer tools', () => {
     render(<GlobalImplementerToolsButton />);
     const button = screen.getByTestId('globalImplementerToolsButton');

packages/apps/esm-login-app/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/apps/esm-login-app/src/change-location-link/change-location-link.test.tsx

@@ -1,8 +1,8 @@
 import React from 'react';
-import ChangeLocationLink from './change-location-link.extension';
-import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
+import { render, screen } from '@testing-library/react';
 import { navigate, useSession } from '@openmrs/esm-framework';
+import ChangeLocationLink from './change-location-link.extension';
 
 const navigateMock = navigate as jest.Mock;
 const useSessionMock = useSession as jest.Mock;
@@ -17,10 +17,11 @@ describe('<ChangeLocationLink/>', () => {
         display: 'Waffle House',
       },
     });
-    render(<ChangeLocationLink />);
   });
 
   it('should display the `Change location` link', async () => {
+    render(<ChangeLocationLink />);
+
     const user = userEvent.setup();
     const changeLocationButton = await screen.findByRole('button', {
       name: /Change/i,

packages/apps/esm-login-app/src/location-picker/location-picker-view.component.tsx

@@ -12,9 +12,9 @@ import {
   getCoreTranslation,
 } from '@openmrs/esm-framework';
 import type { LoginReferrer } from '../login/login.component';
-import styles from './location-picker.scss';
 import { useDefaultLocation, useLocationCount } from './location-picker.resource';
 import type { ConfigSchema } from '../config-schema';
+import styles from './location-picker.scss';
 
 interface LocationPickerProps {
   hideWelcomeMessage?: boolean;

packages/apps/esm-login-app/src/location-picker/location-picker.test.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable */
 import { act, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import {
@@ -45,7 +46,6 @@ mockedUseSession.mockReturnValue({
 
 describe('LocationPickerView', () => {
   beforeEach(() => {
-    jest.clearAllMocks();
     mockedOpenmrsFetch.mockImplementation((url) => {
       if (url === `/ws/fhir2/R4/Location?_id=${fistLocation.uuid}`) {
         return validatingLocationSuccessResponse;

packages/apps/esm-login-app/src/redirect-logout/redirect-logout.test.tsx

@@ -27,7 +27,6 @@ Object.defineProperty(document, 'documentElement', {
 
 describe('Testing Logout', () => {
   beforeEach(() => {
-    jest.clearAllMocks();
     (useConnectivity as jest.Mock).mockReturnValue(true);
     (openmrsFetch as jest.Mock).mockResolvedValue({});
     (useSession as jest.Mock).mockReturnValue({

packages/apps/esm-offline-tools-app/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/apps/esm-primary-navigation-app/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '\\.(m?j|t)sx?$': ['@swc/jest'],
   },

packages/apps/esm-primary-navigation-app/src/components/logo/logo.test.tsx

@@ -1,6 +1,6 @@
 import React from 'react';
 import { useConfig } from '@openmrs/esm-framework';
-import { cleanup, render, screen } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import Logo from './logo.component';
 
 const mockUseConfig = useConfig as jest.Mock;
@@ -11,8 +11,6 @@ jest.mock('@openmrs/esm-framework', () => ({
 }));
 
 describe('<Logo/>', () => {
-  afterEach(cleanup);
-
   it('should display OpenMRS logo', () => {
     const mockConfig = { logo: { src: null, alt: null, name: null } };
     mockUseConfig.mockReturnValue(mockConfig);

packages/framework/esm-api/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-api/src/shared-api-objects/current-patient.test.ts

@@ -17,7 +17,6 @@ jest.mock('@openmrs/esm-offline', () => ({
 
 describe('fetchPatientData', () => {
   beforeEach(() => {
-    jest.clearAllMocks();
     mockGetSynchronizationItems.mockResolvedValue([]);
   });
 

packages/framework/esm-config/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-context/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-dynamic-loading/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-error-handling/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-expression-evaluator/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-extensions/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.(j|t)sx?$': ['@swc/jest'],
   },

packages/framework/esm-feature-flags/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-framework/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.(j|t)sx?$': ['@swc/jest'],
   },

packages/framework/esm-framework/src/integration-tests/extension-config.test.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable */
 import React from 'react';
 import { act, render, screen, waitFor } from '@testing-library/react';
 import { type Person } from '@openmrs/esm-api';
@@ -71,7 +72,7 @@ describe('Interaction between configuration and extension systems', () => {
 
     render(<App />);
 
-    screen.findByText('Betty');
+    await screen.findByText('Betty');
     const slot = screen.getByTestId('slot');
     const extensions = slot.childNodes;
 
@@ -201,7 +202,7 @@ describe('Interaction between configuration and extension systems', () => {
 
     render(<App />);
 
-    await waitFor(() => expect(screen.getByText('Pearl')).toBeInTheDocument());
+    await screen.findByText('Pearl');
 
     act(() => {
       temporaryConfigStore.setState({
@@ -236,7 +237,7 @@ describe('Interaction between configuration and extension systems', () => {
     await act(async () => await promise);
 
     render(<App />);
-    await waitFor(() => expect(screen.getByText(/Mr. Slate/)).toBeInTheDocument());
+    await screen.findByText(/Mr. Slate/);
     expect(screen.getByTestId('slot')).toHaveTextContent(/green/);
 
     act(() => {
@@ -428,7 +429,7 @@ describe('Interaction between configuration and extension systems', () => {
 
     render(<App />);
 
-    await waitFor(() => expect(screen.getByTestId(/slot/)).toBeInTheDocument());
+    await screen.findByTestId(/slot/);
     expect(screen.getByTestId('slot').firstChild).toHaveAttribute('data-extension-id', 'Schmoo');
   });
 

packages/framework/esm-offline/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.tsx?$': ['@swc/jest'],
   },

packages/framework/esm-react-utils/jest.config.js

@@ -1,4 +1,5 @@
 module.exports = {
+  clearMocks: true,
   transform: {
     '^.+\\.(j|t)sx?$': ['@swc/jest'],
   },

packages/framework/esm-react-utils/src/ConfigurableLink.test.tsx

@@ -22,8 +22,10 @@ describe(`ConfigurableLink`, () => {
       </ConfigurableLink>,
     );
     const link = screen.getByRole('link', { name: /spa home/i });
-    expect(link).toBeTruthy();
+    expect(link).toBeInTheDocument();
+    // eslint-disable-next-line testing-library/no-node-access
     expect(link.closest('a')).toHaveClass('fancy-link');
+    // eslint-disable-next-line testing-library/no-node-access
     expect(link.closest('a')).toHaveAttribute('href', '/openmrs/spa/home');
   });