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

test: Improve unit test stability when running locally #10278

Merged
merged 14 commits into from
Jun 5, 2024
Merged

test: Improve unit test stability when running locally #10278

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
df9f692
Remove calls to "act" to resolve "The current testing environment is …
Mar 8, 2024
573d3a8
Increase wait timeout to give time for Password input module to load
Mar 8, 2024
bc587ae
Merge branch 'develop' into unit-test-stability
Mar 11, 2024
dda9138
Clean up test warnings
Mar 11, 2024
00664b5
Resolve RegEx global flag warnings
Mar 12, 2024
e583433
Remove act
Mar 12, 2024
51c6007
Merge branch 'develop' into unit-test-stability
Mar 12, 2024
3960d8e
Upgrade Vitest
Mar 12, 2024
1c4e758
Increase test timeout for CreateAPITokenDrawer and CreateCertificateD…
Mar 13, 2024
686fcd7
Merge branch 'develop' into unit-test-stability
Mar 13, 2024
4e63e1d
Clean up
Mar 13, 2024
f7b8509
Added changeset: Improve unit test suite stability
Mar 13, 2024
0cdb44e
Upgrade APIv4 to Vitest 1.3.x
Mar 14, 2024
a40637f
Merge branch 'develop' into unit-test-stability
Jun 5, 2024
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
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10278-tests-1710347078555.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Improve unit test suite stability ([#10278](https://github.com/linode/manager/pull/10278))
2 changes: 1 addition & 1 deletion packages/manager/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@
"ts-node": "^10.9.2",
"vite": "^5.0.12",
"vite-plugin-svgr": "^3.2.0",
"vitest": "^1.2.0"
"vitest": "^1.3.1"
},
"browserslist": [
">1%",
Expand Down
6 changes: 3 additions & 3 deletions packages/manager/src/features/Billing/BillingPanels/BillingSummary/BillingSummary.test.tsx
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolves these warnings:
billing-summary test

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('BillingSummary', () => {
<BillingSummary balance={0} balanceUninvoiced={5} paymentMethods={[]} />
</PayPalScriptProvider>
);
within(screen.getByTestId(accountBalanceText)).getByText(/no balance/gi);
within(screen.getByTestId(accountBalanceText)).getByText(/no balance/i);
within(screen.getByTestId(accountBalanceValue)).getByText('$0.00');
});

Expand All @@ -45,7 +45,7 @@ describe('BillingSummary', () => {
/>
</PayPalScriptProvider>
);
within(screen.getByTestId(accountBalanceText)).getByText(/credit/gi);
within(screen.getByTestId(accountBalanceText)).getByText(/credit/i);
within(screen.getByTestId(accountBalanceValue)).getByText('$10.00');
});

Expand All @@ -59,7 +59,7 @@ describe('BillingSummary', () => {
/>
</PayPalScriptProvider>
);
within(screen.getByTestId(accountBalanceText)).getByText(/Balance/gi);
within(screen.getByTestId(accountBalanceText)).getByText(/Balance/i);
within(screen.getByTestId(accountBalanceValue)).getByText('$10.00');
});

Expand Down
10 changes: 6 additions & 4 deletions packages/manager/src/features/Linodes/LinodesDetail/LinodeSettings/ImageAndPassword.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';

import { profileFactory } from 'src/factories';
import { accountUserFactory } from 'src/factories/accountUsers';
import { grantsFactory } from 'src/factories/grants';
Expand All @@ -26,12 +25,15 @@ describe('ImageAndPassword', () => {
expect(getByLabelText('Image')).toBeEnabled();
});
it('should render a password error if defined', async () => {
const passwordError = 'Unable to set password.';
const errorMessage = 'Unable to set password.';
const { findByText } = renderWithTheme(
<ImageAndPassword {...props} passwordError={passwordError} />
<ImageAndPassword {...props} passwordError={errorMessage} />
);

expect(await findByText(passwordError)).toBeVisible();
const passwordError = await findByText(errorMessage, undefined, {
timeout: 2500,
});
expect(passwordError).toBeVisible();
});
it('should render an SSH Keys section', async () => {
const { getByText } = renderWithTheme(<ImageAndPassword {...props} />);
Expand Down
52 changes: 28 additions & 24 deletions ...c/features/LoadBalancers/LoadBalancerDetail/Certificates/CreateCertificateDrawer.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,32 @@ import { renderWithTheme } from 'src/utilities/testHelpers';
import { CreateCertificateDrawer } from './CreateCertificateDrawer';

describe('CreateCertificateDrawer', () => {
it('should be submittable when form is filled out correctly', async () => {
const onClose = vi.fn();

const { getByLabelText, getByTestId } = renderWithTheme(
<CreateCertificateDrawer
loadbalancerId={0}
onClose={onClose}
open
type="downstream"
/>
);

const labelInput = getByLabelText('Label');
const certInput = getByLabelText('TLS Certificate');
const keyInput = getByLabelText('Private Key');

await userEvent.type(labelInput, 'my-cert-0');
await userEvent.type(certInput, 'massive cert');
await userEvent.type(keyInput, 'massive key');

await userEvent.click(getByTestId('submit'));

await waitFor(() => expect(onClose).toBeCalled());
});
it(
'should be submittable when form is filled out correctly',
async () => {
const onClose = vi.fn();

const { getByLabelText, getByTestId } = renderWithTheme(
<CreateCertificateDrawer
loadbalancerId={0}
onClose={onClose}
open
type="downstream"
/>
);

const labelInput = getByLabelText('Label');
const certInput = getByLabelText('TLS Certificate');
const keyInput = getByLabelText('Private Key');

await userEvent.type(labelInput, 'my-cert-0');
await userEvent.type(certInput, 'massive cert');
await userEvent.type(keyInput, 'massive key');

await userEvent.click(getByTestId('submit'));

await waitFor(() => expect(onClose).toBeCalled());
},
{ timeout: 10000 }
);
});
19 changes: 7 additions & 12 deletions ...src/features/LoadBalancers/LoadBalancerDetail/Certificates/EditCertificateDrawer.test.tsx
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolves:
Screenshot 2024-03-13 at 12 28 58 PM

We're calling userEvent.type and userEvent.click inside of act which triggers the warning since these functions call act themselves.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { act, waitFor } from '@testing-library/react';
import { waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';

Expand Down Expand Up @@ -82,10 +82,8 @@ describe('EditCertificateDrawer', () => {
expect(labelInput).toHaveDisplayValue(mockCACertificate.label);
expect(certInput).toHaveDisplayValue(mockCACertificate.certificate.trim());

await act(async () => {
await userEvent.type(labelInput, 'my-updated-cert-0');
await userEvent.click(getByTestId('submit'));
});
await userEvent.type(labelInput, 'my-updated-cert-0');
await userEvent.click(getByTestId('submit'));

await waitFor(() => expect(onClose).toBeCalled());
});
Expand All @@ -105,13 +103,10 @@ describe('EditCertificateDrawer', () => {
const certInput = getByLabelText('TLS Certificate');
const keyInput = getByLabelText('Private Key');

await act(async () => {
await userEvent.type(labelInput, 'my-cert-0');
await userEvent.type(certInput, 'massive cert');
await userEvent.type(keyInput, 'massive key');

await userEvent.click(getByTestId('submit'));
});
await userEvent.type(labelInput, 'my-cert-0');
await userEvent.type(certInput, 'massive cert');
await userEvent.type(keyInput, 'massive key');
await userEvent.click(getByTestId('submit'));

await waitFor(() => expect(onClose).toBeCalled());
});
Expand Down
8 changes: 4 additions & 4 deletions packages/manager/src/features/Longview/LongviewDetail/DetailTabs/Disks/Graphs.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('UI', () => {
expect(queryByTestId('space-graph')).toBeNull();
expect(queryByTestId('diskio-graph')).toBeInTheDocument();

expect(getByText(/gather space and inode data/gim));
expect(getByText(/gather space and inode data/im));
});

it('should not render Disk I/O graph for OpenVZ Configs', () => {
Expand All @@ -55,7 +55,7 @@ describe('UI', () => {
expect(queryByTestId('inodes-graph')).toBeInTheDocument();
expect(queryByTestId('diskio-graph')).toBeNull();

expect(getByText(/gather Disk I\/O on OpenVZ Linodes/gim));
expect(getByText(/gather Disk I\/O on OpenVZ Linodes/im));
});

it('should render warning text for ChildOf Disks', () => {
Expand All @@ -68,7 +68,7 @@ describe('UI', () => {
expect(queryByTestId('space-graph')).toBeNull();
expect(queryByTestId('diskio-graph')).toBeNull();

expect(getByText(/doesn't gather data on this type of device/gim));
expect(getByText(/doesn't gather data on this type of device/im));
});

it('should render warning text for unmounted disks', () => {
Expand All @@ -81,6 +81,6 @@ describe('UI', () => {
expect(queryByTestId('space-graph')).toBeNull();
expect(queryByTestId('diskio-graph')).toBeInTheDocument();

expect(getByText(/gather Space and Inode data on unmounted disks/gim));
expect(getByText(/gather Space and Inode data on unmounted disks/im));
});
});
7 changes: 4 additions & 3 deletions packages/manager/src/features/Longview/LongviewLanding/LongviewLanding.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,10 @@ describe('Longview clients list view', () => {
expect(props.getLongviewClients).toHaveBeenCalledTimes(1);
});

it('should have an Add Client button', () => {
const { queryByText } = renderWithTheme(<LongviewLanding {...props} />);
expect(queryByText('Add Client')).toBeInTheDocument();
it('should have an Add Client button', async () => {
const { findByText } = renderWithTheme(<LongviewLanding {...props} />);
const addButton = await findByText('Add Client');
expect(addButton).toBeInTheDocument();
Comment on lines +110 to +113
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This occasionally fails because queryByText('Add Client') returns null and the expect fails. I'm not sure why it fails to find the button (I don't see any logic that would delay the button being rendered), but using findByText allows the test to wait for it to appear.

});

it('should attempt to add a new client when the Add Client button is clicked', async () => {
Expand Down
26 changes: 9 additions & 17 deletions packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { act, fireEvent } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import * as React from 'react';

import {
Expand All @@ -10,8 +10,6 @@ import { renderWithTheme } from 'src/utilities/testHelpers';

import { PlacementGroupsDeleteModal } from './PlacementGroupsDeleteModal';

import type { RenderResult } from '@testing-library/react';

const queryMocks = vi.hoisted(() => ({
useAllLinodesQuery: vi.fn().mockReturnValue({}),
useDeletePlacementGroup: vi.fn().mockReturnValue({
Expand Down Expand Up @@ -100,12 +98,9 @@ describe('PlacementGroupsDeleteModal', () => {
}),
});

let renderResult: RenderResult;
await act(async () => {
renderResult = renderWithTheme(<PlacementGroupsDeleteModal {...props} />);
});

const { getByRole, getByTestId, getByText } = renderResult!;
const { getByRole, getByTestId, getByText } = renderWithTheme(
<PlacementGroupsDeleteModal {...props} />
);

expect(
getByRole('heading', {
Expand Down Expand Up @@ -136,12 +131,9 @@ describe('PlacementGroupsDeleteModal', () => {
}),
});

let renderResult: RenderResult;
await act(async () => {
renderResult = renderWithTheme(<PlacementGroupsDeleteModal {...props} />);
});

const { getByRole, getByTestId, getByText } = renderResult!;
const { getByRole, getByTestId, getByText } = renderWithTheme(
<PlacementGroupsDeleteModal {...props} />
);

expect(getByText('No Linodes assigned to this Placement Group.'));

Expand All @@ -151,10 +143,10 @@ describe('PlacementGroupsDeleteModal', () => {
expect(textField).toBeEnabled();
expect(deleteButton).toBeDisabled();

fireEvent.change(textField, { target: { value: 'PG-to-delete' } });
await userEvent.type(textField, 'PG-to-delete');

expect(deleteButton).toBeEnabled();
fireEvent.click(deleteButton);
await userEvent.click(deleteButton);

expect(queryMocks.useDeletePlacementGroup).toHaveBeenCalled();
});
Expand Down
46 changes: 26 additions & 20 deletions packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.test.tsx
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spent the most time troubleshooting this test and found that it fails by timing out nearly every time when running alongside the rest of the test suite.

(The quickest way to reproduce I've found is to run yarn test src/features/PlacementGroups src/features/Profile, but this may vary from machine-to-machine).

What I've found is that the test, interactions, etc. all work exactly as expected, but when running alongside other tests some of the interactions (specifically and surprisingly the userEvent.click(submit)) take a really long time to execute. I don't think it has anything to do with the component itself.

My (purely speculative) guess is that when transpiling/running a lot of tests concurrently, the CPU is taking such a hit that it's slowing down JSDom to the point that the tests are timing out. I opened a ticket to reassess Happy DOM which might prove or disprove this theory, but in the meantime increasing the timeout seems to be a reliable (if unfortunate) mitigation.

Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,32 @@ describe('Create API Token Drawer', () => {
expect(cancelBtn).toBeVisible();
});

it('Should see secret modal with secret when you type a label and submit the form successfully', async () => {
server.use(
rest.post('*/profile/tokens', (req, res, ctx) => {
return res(ctx.json(appTokenFactory.build({ token: 'secret-value' })));
})
);

const { getByTestId, getByText } = renderWithTheme(
<CreateAPITokenDrawer {...props} />
);

const labelField = getByTestId('textfield-input');
await userEvent.type(labelField, 'my-test-token');
const submit = getByText('Create Token');
await userEvent.click(submit);

await waitFor(() =>
expect(props.showSecret).toBeCalledWith('secret-value')
);
});
it(
'Should see secret modal with secret when you type a label and submit the form successfully',
async () => {
server.use(
rest.post('*/profile/tokens', (req, res, ctx) => {
return res(
ctx.json(appTokenFactory.build({ token: 'secret-value' }))
);
})
);

const { getByTestId, getByText } = renderWithTheme(
<CreateAPITokenDrawer {...props} />
);

const labelField = getByTestId('textfield-input');
await userEvent.type(labelField, 'my-test-token');
const submit = getByText('Create Token');
await userEvent.click(submit);

await waitFor(() =>
expect(props.showSecret).toBeCalledWith('secret-value')
);
},
{ timeout: 10000 }
);

it('Should default to None for all scopes', () => {
const { getByLabelText } = renderWithTheme(
Expand Down
48 changes: 21 additions & 27 deletions packages/manager/src/features/Profile/APITokens/EditAPITokenDrawer.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { act, waitFor } from '@testing-library/react';
import { waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import * as React from 'react';

Expand Down Expand Up @@ -33,44 +33,38 @@ describe('Edit API Token Drawer', () => {
expect(cancelBtn).not.toHaveAttribute('aria-disabled', 'true');
expect(cancelBtn).toBeVisible();
});

it('Save button should become enabled when label is changed', async () => {
const { getByTestId } = renderWithTheme(<EditAPITokenDrawer {...props} />);

const saveButton = getByTestId('save-button');
expect(saveButton).toHaveAttribute('aria-disabled', 'true');

await act(async () => {
const labelField = getByTestId('textfield-input');

await userEvent.type(labelField, 'updated-token-label');
expect(getByTestId('save-button')).toHaveAttribute('aria-disabled', 'true');

const saveButton = getByTestId('save-button');
const labelField = getByTestId('textfield-input');

await waitFor(() =>
expect(saveButton).toHaveAttribute('aria-disabled', 'false')
);
});
await userEvent.type(labelField, 'updated-token-label');
await waitFor(() =>
expect(getByTestId('save-button')).toHaveAttribute(
'aria-disabled',
'false'
)
);
});

it('Should close when updating a label and saving', async () => {
// @note: this test uses handlers for PUT */profile/tokens/:id in serverHandlers.ts
const { getByTestId } = renderWithTheme(<EditAPITokenDrawer {...props} />);

await act(async () => {
const labelField = getByTestId('textfield-input');

await userEvent.type(labelField, 'my-token-updated');

const saveButton = getByTestId('save-button');

await waitFor(() =>
expect(saveButton).toHaveAttribute('aria-disabled', 'false')
);

await userEvent.click(saveButton);
const labelField = getByTestId('textfield-input');
await userEvent.type(labelField, 'my-token-updated');

await waitFor(() => expect(props.onClose).toBeCalled());
});
const saveButton = getByTestId('save-button');
await waitFor(() =>
expect(saveButton).toHaveAttribute('aria-disabled', 'false')
);
await userEvent.click(saveButton);
await waitFor(() => expect(props.onClose).toBeCalled());
});

it('Should close when Cancel is pressed', async () => {
const { getByText } = renderWithTheme(<EditAPITokenDrawer {...props} />);
const cancelButton = getByText(/Cancel/);
Expand Down
Loading
Loading