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

test: Improve unit test stability when running locally #10278

Merged
merged 14 commits into from
Jun 5, 2024
Merged
2 changes: 1 addition & 1 deletion packages/api-v4/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"lint-staged": "^13.2.2",
"prettier": "~2.2.1",
"tsup": "^7.2.0",
"vitest": "^1.0.1"
"vitest": "^1.3.1"
},
"lint-staged": {
"*.{ts,tsx,js}": [
Expand Down
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10278-tests-1710347078555.md
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
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@
"ts-node": "^10.9.2",
"vite": "^5.1.7",
"vite-plugin-svgr": "^3.2.0",
"vitest": "^1.2.0"
"vitest": "^1.3.1"
},
"browserslist": [
">1%",
Expand Down
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
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
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 }
);
});
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
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));
});
});
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
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { act, fireEvent } from '@testing-library/react';
import * as React from 'react';

import { userEvent } from '@testing-library/user-event';
import { linodeFactory, placementGroupFactory } from 'src/factories';
import { renderWithTheme } from 'src/utilities/testHelpers';

import { PlacementGroupsDeleteModal } from './PlacementGroupsDeleteModal';

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

const queryMocks = vi.hoisted(() => ({
useDeletePlacementGroup: vi.fn().mockReturnValue({
mutateAsync: vi.fn().mockResolvedValue({}),
Expand All @@ -31,36 +29,31 @@ const props = {

describe('PlacementGroupsDeleteModal', () => {
it('should render the right form elements', async () => {
let renderResult: RenderResult;
await act(async () => {
renderResult = renderWithTheme(
<PlacementGroupsDeleteModal
{...props}
linodes={[
linodeFactory.build({
id: 1,
label: 'test-linode',
region: 'us-east',
}),
]}
selectedPlacementGroup={placementGroupFactory.build({
affinity_type: 'anti_affinity:local',
const { getByRole, getByTestId, getByText } = renderWithTheme(
<PlacementGroupsDeleteModal
{...props}
linodes={[
linodeFactory.build({
id: 1,
label: 'PG-to-delete',
members: [
{
is_compliant: true,
linode_id: 1,
},
],
label: 'test-linode',
region: 'us-east',
})}
disableUnassignButton={false}
/>
);
});

const { getByRole, getByTestId, getByText } = renderResult!;
}),
]}
selectedPlacementGroup={placementGroupFactory.build({
affinity_type: 'anti_affinity:local',
id: 1,
label: 'PG-to-delete',
members: [
{
is_compliant: true,
linode_id: 1,
},
],
region: 'us-east',
})}
disableUnassignButton={false}
/>
);

expect(
getByRole('heading', {
Expand All @@ -80,41 +73,36 @@ describe('PlacementGroupsDeleteModal', () => {
});

it("should be enabled when there's no assigned linodes", async () => {
let renderResult: RenderResult;
await act(async () => {
renderResult = renderWithTheme(
<PlacementGroupsDeleteModal
{...props}
linodes={[
linodeFactory.build({
id: 1,
label: 'test-linode',
region: 'us-east',
}),
]}
selectedPlacementGroup={placementGroupFactory.build({
affinity_type: 'anti_affinity:local',
const { getByRole, getByTestId } = renderWithTheme(
<PlacementGroupsDeleteModal
{...props}
linodes={[
linodeFactory.build({
id: 1,
label: 'PG-to-delete',
members: [],
})}
disableUnassignButton={false}
/>
);
});

const { getByRole, getByTestId } = renderResult!;
label: 'test-linode',
region: 'us-east',
}),
]}
selectedPlacementGroup={placementGroupFactory.build({
affinity_type: 'anti_affinity:local',
id: 1,
label: 'PG-to-delete',
members: [],
})}
disableUnassignButton={false}
/>
);

const textField = getByTestId('textfield-input');
const deleteButton = getByRole('button', { name: 'Delete' });

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
Loading
Loading