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

Upgrade package @testing-library/user-event from 12.8.3 to 14.5.2 #2605

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,497 changes: 1,282 additions & 1,215 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
"@babel/preset-typescript": "^7.26.0",
"@testing-library/jest-dom": "^6.6.3",
"@testing-library/react": "^16.0.1",
"@testing-library/user-event": "^12.1.10",
"@testing-library/user-event": "^14.5.2",
"@types/inquirer": "^9.0.7",
"@types/jest": "^26.0.24",
"@types/js-cookie": "^3.0.6",
Expand Down Expand Up @@ -147,7 +147,8 @@
"eslint-plugin-tsdoc": "^0.3.0",
"husky": "^9.1.6",
"identity-obj-proxy": "^3.0.0",
"jest": "^27.4.5",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"jest-localstorage-mock": "^2.4.19",
"jest-location-mock": "^2.0.0",
"jest-preview": "^0.3.1",
Expand Down
4 changes: 2 additions & 2 deletions scripts/custom-test-env.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import Environment from 'jest-environment-jsdom';
import { TextEncoder, TextDecoder } from 'util';
import { TestEnvironment } from 'jest-environment-jsdom';

/**
* A custom environment to set the TextEncoder and TextDecoder variables, that is required by @pdfme during testing.
* Providing a polyfill to the environment for the same
*/
export default class CustomTestEnvironment extends Environment {
export default class CustomTestEnvironment extends TestEnvironment {
async setup() {
await super.setup();
if (typeof this.global.TextEncoder === 'undefined') {
Expand Down
31 changes: 17 additions & 14 deletions src/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,14 @@ describe('Testing AddOnRegister', () => {
);

// Simulate user interactions
userEvent.click(screen.getByRole('button', { name: /Add New/i }));
userEvent.type(screen.getByPlaceholderText(/Ex: Donations/i), 'myplugin');
userEvent.type(
const user = userEvent.setup();
await user.click(screen.getByRole('button', { name: /Add New/i }));
await user.type(screen.getByPlaceholderText(/Ex: Donations/i), 'myplugin');
await user.type(
screen.getByPlaceholderText(/This Plugin enables UI for/i),
'test description',
);
userEvent.type(
await user.type(
screen.getByPlaceholderText(/Ex: john Doe/i),
'test creator',
);
Expand All @@ -143,16 +144,17 @@ describe('Testing AddOnRegister', () => {
});
await waitFor(() => new Promise((resolve) => setTimeout(resolve, 0)));

userEvent.click(screen.getByRole('button', { name: /Add New/i }));
const user = userEvent.setup();
await user.click(screen.getByRole('button', { name: /Add New/i }));
await wait(100);
expect(screen.getByTestId('addonregisterBtn')).toBeInTheDocument();
userEvent.type(screen.getByTestId('pluginName'), pluginData.pluginName);
userEvent.type(
await user.type(screen.getByTestId('pluginName'), pluginData.pluginName);
await user.type(
screen.getByTestId('pluginCreatedBy'),
pluginData.pluginCreatedBy,
);
userEvent.type(screen.getByTestId('pluginDesc'), pluginData.pluginDesc);
userEvent.click(screen.getByTestId('addonregisterBtn'));
await user.type(screen.getByTestId('pluginDesc'), pluginData.pluginDesc);
await user.click(screen.getByTestId('addonregisterBtn'));

await wait(100);
expect(toast.success).toHaveBeenCalledWith('Plugin added Successfully');
Expand All @@ -174,16 +176,17 @@ describe('Testing AddOnRegister', () => {
});
await waitFor(() => new Promise((resolve) => setTimeout(resolve, 0)));

userEvent.click(screen.getByRole('button', { name: /Add New/i }));
const user = userEvent.setup();
await user.click(screen.getByRole('button', { name: /Add New/i }));
await wait(100);
expect(screen.getByTestId('addonregisterBtn')).toBeInTheDocument();
userEvent.type(screen.getByTestId('pluginName'), pluginData.pluginName);
userEvent.type(
await user.type(screen.getByTestId('pluginName'), pluginData.pluginName);
await user.type(
screen.getByTestId('pluginCreatedBy'),
pluginData.pluginCreatedBy,
);
userEvent.type(screen.getByTestId('pluginDesc'), pluginData.pluginDesc);
userEvent.click(screen.getByTestId('addonregisterBtn'));
await user.type(screen.getByTestId('pluginDesc'), pluginData.pluginDesc);
await user.click(screen.getByTestId('addonregisterBtn'));

await wait(3000); // Waiting for 3 seconds to reload the page as timeout is set to 2 seconds in the component
expect(mockNavigate).toHaveBeenCalledWith(0);
Comment on lines 190 to 192
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace fixed timeout with waitFor

Using a fixed timeout of 3 seconds for waiting for navigation is brittle and could lead to flaky tests:

-await wait(3000); // Waiting for 3 seconds to reload the page
+await waitFor(
+  () => {
+    expect(mockNavigate).toHaveBeenCalledWith(0);
+  },
+  { timeout: 3000 }
+);
-expect(mockNavigate).toHaveBeenCalledWith(0);

This approach is more reliable as it will complete as soon as the condition is met, without always waiting for the full duration.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await wait(3000); // Waiting for 3 seconds to reload the page as timeout is set to 2 seconds in the component
expect(mockNavigate).toHaveBeenCalledWith(0);
await waitFor(
() => {
expect(mockNavigate).toHaveBeenCalledWith(0);
},
{ timeout: 3000 }
);

Expand Down
7 changes: 4 additions & 3 deletions src/components/AddOn/core/AddOnStore/AddOnStore.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ jest.mock('components/AddOn/support/services/Plugin.helper', () => ({
]),
generateLinks: jest.fn().mockImplementation((plugins) => {
return plugins
.filter((plugin: { enabled: any }) => plugin.enabled)
.map((installedPlugin: { pluginName: any; component: string }) => {
.filter((plugin: { enabled: unknown }) => plugin.enabled)
.map((installedPlugin: { pluginName: unknown; component: string }) => {
return {
name: installedPlugin.pluginName,
url: `/plugin/${installedPlugin.component.toLowerCase()}`,
Expand Down Expand Up @@ -318,7 +318,8 @@ describe('Testing AddOnStore Component', () => {
);

await wait();
userEvent.click(screen.getByText('Installed'));
const user = userEvent.setup();
await user.click(screen.getByText('Installed'));

expect(screen.getByText('Filters')).toBeInTheDocument();
expect(screen.getByLabelText('Enabled')).toBeInTheDocument();
Expand Down
9 changes: 5 additions & 4 deletions src/components/AddPeopleToTag/AddPeopleToTag.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -293,19 +293,20 @@ describe('Organisation Tags Page', () => {
await waitFor(() => {
expect(screen.getAllByTestId('selectMemberBtn')[0]).toBeInTheDocument();
});
userEvent.click(screen.getAllByTestId('selectMemberBtn')[0]);
const user = userEvent.setup();
await user.click(screen.getAllByTestId('selectMemberBtn')[0]);

await waitFor(() => {
expect(screen.getAllByTestId('selectMemberBtn')[1]).toBeInTheDocument();
});
userEvent.click(screen.getAllByTestId('selectMemberBtn')[1]);
await user.click(screen.getAllByTestId('selectMemberBtn')[1]);

await waitFor(() => {
expect(screen.getAllByTestId('selectMemberBtn')[2]).toBeInTheDocument();
});
userEvent.click(screen.getAllByTestId('selectMemberBtn')[2]);
await user.click(screen.getAllByTestId('selectMemberBtn')[2]);

userEvent.click(screen.getByTestId('assignPeopleBtn'));
await user.click(screen.getByTestId('assignPeopleBtn'));

await waitFor(() => {
expect(toast.success).toHaveBeenCalledWith(
Expand Down
26 changes: 14 additions & 12 deletions src/components/Advertisements/Advertisements.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,10 @@ describe('Testing Advertisement Component', () => {

await wait();

userEvent.click(screen.getByText('Create Advertisement'));
userEvent.type(
screen.getByLabelText('Enter name of Advertisement'),
'Cookie Shop',
);
fireEvent.click(screen.getByText('Create Advertisement'));
fireEvent.change(screen.getByLabelText('Enter name of Advertisement'), {
target: { value: 'Cookie Shop' },
});
const mediaFile = new File(['media content'], 'test.png', {
type: 'image/png',
});
Expand All @@ -398,14 +397,17 @@ describe('Testing Advertisement Component', () => {
});
const mediaPreview = await screen.findByTestId('mediaPreview');
expect(mediaPreview).toBeInTheDocument();
userEvent.selectOptions(
screen.getByLabelText('Select type of Advertisement'),
'POPUP',
);
userEvent.type(screen.getByLabelText('Select Start Date'), '2023-01-01');
userEvent.type(screen.getByLabelText('Select End Date'), '2023-02-02');
fireEvent.change(screen.getByLabelText('Select type of Advertisement'), {
target: { value: 'POPUP' },
});
fireEvent.change(screen.getByLabelText('Select Start Date'), {
target: { value: '2023-01-01' },
});
fireEvent.change(screen.getByLabelText('Select End Date'), {
target: { value: '2023-02-02' },
});

userEvent.click(screen.getByTestId('addonregister'));
fireEvent.click(screen.getByTestId('addonregister'));
expect(
await screen.findByText('Advertisement created successfully.'),
).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,11 @@ describe('Testing Advertisement Register Component', () => {
type: 'video/mp4',
});
const mediaInput = screen.getByTestId('advertisementMedia');
userEvent.upload(mediaInput, mediaFile);
fireEvent.change(mediaInput, {
target: {
files: [mediaFile],
},
});
Comment on lines +631 to +635
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider using userEvent.upload for file inputs

The change from userEvent.upload to fireEvent.change for file inputs is a regression in terms of testing fidelity. userEvent.upload provides a more realistic simulation of user interactions with file inputs.

-    fireEvent.change(mediaInput, {
-      target: {
-        files: [mediaFile],
-      },
-    });
+    const user = userEvent.setup();
+    await user.upload(mediaInput, mediaFile);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fireEvent.change(mediaInput, {
target: {
files: [mediaFile],
},
});
const user = userEvent.setup();
await user.upload(mediaInput, mediaFile);


const mediaPreview = await screen.findByTestId('mediaPreview');
expect(mediaPreview).toBeInTheDocument();
Expand Down
18 changes: 10 additions & 8 deletions src/components/AgendaCategory/AgendaCategoryContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,22 @@ describe('Testing Agenda Category Component', () => {
screen.getAllByTestId('editAgendCategoryModalBtn')[0],
).toBeInTheDocument();
});
userEvent.click(screen.getAllByTestId('editAgendCategoryModalBtn')[0]);
const user = userEvent.setup();
await user.click(screen.getAllByTestId('editAgendCategoryModalBtn')[0]);

const name = screen.getByPlaceholderText(translations.name);
const description = screen.getByPlaceholderText(translations.description);

fireEvent.change(name, { target: { value: '' } });
userEvent.type(name, formData.name);
await user.type(name, formData.name);

fireEvent.change(description, { target: { value: '' } });
userEvent.type(description, formData.description);
await user.type(description, formData.description);

await waitFor(() => {
expect(screen.getByTestId('editAgendaCategoryBtn')).toBeInTheDocument();
});
userEvent.click(screen.getByTestId('editAgendaCategoryBtn'));
await user.click(screen.getByTestId('editAgendaCategoryBtn'));

await waitFor(() => {
expect(toast.success).toHaveBeenCalledWith(
Expand Down Expand Up @@ -297,21 +298,22 @@ describe('Testing Agenda Category Component', () => {
screen.getAllByTestId('editAgendCategoryModalBtn')[0],
).toBeInTheDocument();
});
userEvent.click(screen.getAllByTestId('editAgendCategoryModalBtn')[0]);
const user = userEvent.setup();
await user.click(screen.getAllByTestId('editAgendCategoryModalBtn')[0]);

const nameInput = screen.getByLabelText(translations.name);
const descriptionInput = screen.getByLabelText(translations.description);
fireEvent.change(nameInput, { target: { value: '' } });
fireEvent.change(descriptionInput, {
target: { value: '' },
});
userEvent.type(nameInput, formData.name);
userEvent.type(descriptionInput, formData.description);
await user.type(nameInput, formData.name);
await user.type(descriptionInput, formData.description);

await waitFor(() => {
expect(screen.getByTestId('editAgendaCategoryBtn')).toBeInTheDocument();
});
userEvent.click(screen.getByTestId('editAgendaCategoryBtn'));
await user.click(screen.getByTestId('editAgendaCategoryBtn'));

await waitFor(() => {
expect(toast.error).toHaveBeenCalled();
Expand Down
Loading
Loading