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

Spike for unusual test timeout for fleet #201443

Closed
wants to merge 2 commits into from
Closed

Spike for unusual test timeout for fleet #201443

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fde6dad
correct usages of testing library for fleet
eokoneyo Nov 22, 2024
bef9452
Merge branch 'main' into fix/correct-usages-of-testing-library-for-fleet
juliaElastic Nov 28, 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
38 changes: 17 additions & 21 deletions ...eet/public/applications/fleet/sections/settings/components/multi_row_input/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React from 'react';
import { fireEvent, act } from '@testing-library/react';
import { fireEvent, waitFor } from '@testing-library/react';

import { createFleetTestRendererMock } from '../../../../../../mock';

Expand Down Expand Up @@ -44,13 +44,11 @@ test('it should allow to add a new host', async () => {
test('it should allow to remove an host', async () => {
const { utils, mockOnChange } = renderInput(['http://host1.com', 'http://host2.com']);

await act(async () => {
const deleteRowEl = await utils.container.querySelector('[aria-label="Delete row"]');
if (!deleteRowEl) {
throw new Error('Delete row button not found');
}
fireEvent.click(deleteRowEl);
});
const deleteRowEl = await utils.container.querySelector('[aria-label="Delete row"]');
if (!deleteRowEl) {
throw new Error('Delete row button not found');
}
Comment on lines +48 to +50
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 should actually be an assertion

fireEvent.click(deleteRowEl);

expect(mockOnChange).toHaveBeenCalledWith(['http://host2.com']);
});
Expand Down Expand Up @@ -95,7 +93,7 @@ test('Should display errors in order', async () => {
{ message: 'Error 3', index: 2 },
]
);
await act(async () => {
await waitFor(async () => {
const errors = await utils.queryAllByText(/Error [1-3]/);
expect(errors[0]).toHaveTextContent('Error 1');
expect(errors[1]).toHaveTextContent('Error 2');
Expand Down Expand Up @@ -126,18 +124,16 @@ test('Should remove error when item deleted', async () => {
);
});

await act(async () => {
const deleteRowButtons = await utils.container.querySelectorAll('[aria-label="Delete row"]');
if (deleteRowButtons.length !== 3) {
throw new Error('Delete row buttons not found');
}
const deleteRowButtons = await utils.container.querySelectorAll('[aria-label="Delete row"]');
if (deleteRowButtons.length !== 3) {
throw new Error('Delete row buttons not found');
}
Comment on lines +128 to +130
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 could be an assertion


fireEvent.click(deleteRowButtons[1]);
expect(mockOnChange).toHaveBeenCalled();
fireEvent.click(deleteRowButtons[1]);
expect(mockOnChange).toHaveBeenCalled();

const renderedErrors = await utils.queryAllByText(/Error [1-3]/);
expect(renderedErrors).toHaveLength(2);
expect(renderedErrors[0]).toHaveTextContent('Error 1');
expect(renderedErrors[1]).toHaveTextContent('Error 3');
});
const renderedErrors = await utils.queryAllByText(/Error [1-3]/);
expect(renderedErrors).toHaveLength(2);
expect(renderedErrors[0]).toHaveTextContent('Error 1');
expect(renderedErrors[1]).toHaveTextContent('Error 3');
});
6 changes: 1 addition & 5 deletions ...plugins/fleet/public/applications/integrations/sections/epm/screens/detail/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import React, { lazy, memo } from 'react';
import { Route } from '@kbn/shared-ux-router';
import { act, cleanup } from '@testing-library/react';
import { act } from '@testing-library/react';

import { INTEGRATIONS_ROUTING_PATHS, pagePathGetters } from '../../../../constants';
import type {
Expand Down Expand Up @@ -65,10 +65,6 @@ describe('When on integration detail', () => {
act(() => testRenderer.mountHistory.push(detailPageUrlPath));
});

afterEach(() => {
cleanup();
});

describe('and the package is installed', () => {
beforeEach(async () => {
await render();
Expand Down
45 changes: 20 additions & 25 deletions .../plugins/fleet/public/components/agent_enrollment_flyout/agent_enrollment_flyout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import './agent_enrollment_flyout.test.mocks';

import React from 'react';
import { act, cleanup, fireEvent, waitFor } from '@testing-library/react';
import { fireEvent, waitFor } from '@testing-library/react';
import type { RenderResult } from '@testing-library/react';

import { createFleetTestRendererMock } from '../../mock';
Expand All @@ -23,11 +23,8 @@ import type { FlyOutProps } from './types';
import { AgentEnrollmentFlyout } from '.';

const render = (props?: Partial<FlyOutProps>) => {
cleanup();
Copy link
Contributor Author

@eokoneyo eokoneyo Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We shouldn't be cleaning up just before test starts, what's more with jest this is handled automatically for us

const renderer = createFleetTestRendererMock();
const results = renderer.render(<AgentEnrollmentFlyout onClose={jest.fn()} {...props} />);

return results;
return renderer.render(<AgentEnrollmentFlyout onClose={jest.fn()} {...props} />);
};

const testAgentPolicy: AgentPolicy = {
Expand All @@ -46,7 +43,7 @@ const testAgentPolicy: AgentPolicy = {
describe('<AgentEnrollmentFlyout />', () => {
let results: RenderResult;

beforeEach(async () => {
beforeEach(() => {
jest.mocked(useAuthz).mockReturnValue({
fleet: {
readAgentPolicies: true,
Expand Down Expand Up @@ -88,10 +85,6 @@ describe('<AgentEnrollmentFlyout />', () => {
agentPolicies: [{ id: 'fleet-server-policy' } as AgentPolicy],
refreshAgentPolicies: jest.fn(),
});

act(() => {
results = render();
});
Comment on lines -92 to -94
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also there's no need to render here, if we still invoke render again in each test suite

});

afterEach(() => {
Expand All @@ -113,6 +106,8 @@ describe('<AgentEnrollmentFlyout />', () => {

describe('managed instructions', () => {
it('uses the agent policy selection step', () => {
results = render();

expect(results.queryByTestId('agentEnrollmentFlyout')).not.toBeNull();
expect(results.queryByTestId('agent-policy-selection-step')).not.toBeNull();
expect(results.queryByTestId('agent-enrollment-key-selection-step')).toBeNull();
Expand Down Expand Up @@ -154,24 +149,22 @@ describe('<AgentEnrollmentFlyout />', () => {

describe('standalone instructions', () => {
function goToStandaloneTab() {
act(() => {
fireEvent.click(results.getByTestId('standaloneTab'));
});
Comment on lines -157 to -159
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no need to wrap fireEvent in act

fireEvent.click(results.getByTestId('standaloneTab'));
}

beforeEach(() => {
it('uses the agent policy selection step', async () => {
results = render({
isIntegrationFlow: true,
});
});

it('uses the agent policy selection step', async () => {
goToStandaloneTab();

expect(results.queryByTestId('agentEnrollmentFlyout')).not.toBeNull();
expect(results.queryByTestId('agent-policy-selection-step')).not.toBeNull();
expect(results.queryByTestId('agent-enrollment-key-selection-step')).toBeNull();
expect(results.queryByTestId('configure-standalone-step')).not.toBeNull();
await waitFor(() => {
expect(results.queryByTestId('agentEnrollmentFlyout')).not.toBeNull();
expect(results.queryByTestId('agent-policy-selection-step')).not.toBeNull();
expect(results.queryByTestId('agent-enrollment-key-selection-step')).toBeNull();
expect(results.queryByTestId('configure-standalone-step')).not.toBeNull();
});
});

describe('with a specific policy', () => {
Expand All @@ -185,13 +178,15 @@ describe('<AgentEnrollmentFlyout />', () => {
});
});

it('does not use either of the agent policy selection or enrollment key steps', () => {
it('does not use either of the agent policy selection or enrollment key steps', async () => {
goToStandaloneTab();

expect(results.queryByTestId('agentEnrollmentFlyout')).not.toBeNull();
expect(results.queryByTestId('agent-policy-selection-step')).toBeNull();
expect(results.queryByTestId('agent-enrollment-key-selection-step')).toBeNull();
expect(results.queryByTestId('configure-standalone-step')).not.toBeNull();
await waitFor(() => {
expect(results.queryByTestId('agentEnrollmentFlyout')).not.toBeNull();
expect(results.queryByTestId('agent-policy-selection-step')).toBeNull();
expect(results.queryByTestId('agent-enrollment-key-selection-step')).toBeNull();
expect(results.queryByTestId('configure-standalone-step')).not.toBeNull();
});
});
});
});
Expand Down
3 changes: 1 addition & 2 deletions .../plugins/fleet/public/components/enrollment_instructions/root_privileges_callout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@

import React from 'react';

import { cleanup, waitFor } from '@testing-library/react';
import { waitFor } from '@testing-library/react';

import { createFleetTestRendererMock } from '../../mock';

import { RootPrivilegesCallout } from './root_privileges_callout';

describe('RootPrivilegesCallout', () => {
function render(rootIntegrations?: Array<{ name: string; title: string }>) {
cleanup();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yields from from removing this:
Screenshot 2024-11-22 at 18 00 07

const renderer = createFleetTestRendererMock();
const results = renderer.render(<RootPrivilegesCallout rootIntegrations={rootIntegrations} />);

Expand Down
23 changes: 11 additions & 12 deletions x-pack/plugins/fleet/public/components/multiple_agent_policy_summary_line.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { act, fireEvent } from '@testing-library/react';
import { fireEvent, waitFor } from '@testing-library/react';
import React from 'react';

import type { TestRenderer } from '../mock';
Expand All @@ -31,7 +31,7 @@ describe('MultipleAgentPolicySummaryLine', () => {
testRenderer = createFleetTestRendererMock();
});

test('it should only render the policy name when there is only one policy', async () => {
test('it should only render the policy name when there is only one policy', () => {
const results = render([{ name: 'Test policy', revision: 2 }] as AgentPolicy[]);
expect(results.container.textContent).toBe('Test policyrev. 2');
expect(results.queryByTestId('agentPolicyNameLink')).toBeInTheDocument();
Expand All @@ -48,19 +48,18 @@ describe('MultipleAgentPolicySummaryLine', () => {
expect(results.queryByTestId('agentPoliciesNumberBadge')).toBeInTheDocument();
expect(results.container.textContent).toBe('Test policy 1+2');

await act(async () => {
fireEvent.click(results.getByTestId('agentPoliciesNumberBadge'));
});
expect(results.queryByTestId('agentPoliciesPopover')).toBeInTheDocument();
expect(results.queryByTestId('agentPoliciesPopoverButton')).toBeInTheDocument();
expect(results.queryByTestId('policy-0001')).toBeInTheDocument();
expect(results.queryByTestId('policy-0002')).toBeInTheDocument();
expect(results.queryByTestId('policy-0003')).toBeInTheDocument();
fireEvent.click(results.getByTestId('agentPoliciesNumberBadge'));

await act(async () => {
fireEvent.click(results.getByTestId('agentPoliciesPopoverButton'));
await waitFor(() => {
expect(results.queryByTestId('agentPoliciesPopover')).toBeInTheDocument();
expect(results.queryByTestId('agentPoliciesPopoverButton')).toBeInTheDocument();
expect(results.queryByTestId('policy-0001')).toBeInTheDocument();
expect(results.queryByTestId('policy-0002')).toBeInTheDocument();
expect(results.queryByTestId('policy-0003')).toBeInTheDocument();
});

fireEvent.click(results.getByTestId('agentPoliciesPopoverButton'));

expect(results.queryByTestId('manageAgentPoliciesModal')).toBeInTheDocument();
});
});
25 changes: 13 additions & 12 deletions x-pack/plugins/fleet/public/components/package_policy_actions_menu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import React from 'react';

import { act } from '@testing-library/react';
import { waitFor } from '@testing-library/react';

import type { AgentPolicy, InMemoryPackagePolicy } from '../types';
import { createIntegrationsTestRendererMock } from '../mock';
Expand Down Expand Up @@ -118,7 +118,8 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies();
const packagePolicy = createMockPackagePolicy({ hasUpgrade: false });
const { utils } = renderMenu({ agentPolicies, packagePolicy });
await act(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's no need to have assertions run within an act function


await waitFor(() => {
const upgradeButton = utils.getByTestId('PackagePolicyActionsUpgradeItem');
expect(upgradeButton).toBeDisabled();
});
Expand All @@ -129,7 +130,7 @@ describe('PackagePolicyActionsMenu', () => {
const packagePolicy = createMockPackagePolicy({ hasUpgrade: true });
const { utils } = renderMenu({ agentPolicies, packagePolicy });

await act(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here

await waitFor(() => {
const upgradeButton = utils.getByTestId('PackagePolicyActionsUpgradeItem');
expect(upgradeButton).not.toBeDisabled();
});
Expand All @@ -140,7 +141,7 @@ describe('PackagePolicyActionsMenu', () => {
const packagePolicy = createMockPackagePolicy({ hasUpgrade: true });
const { utils } = renderMenu({ agentPolicies, packagePolicy });

await act(async () => {
await waitFor(() => {
const upgradeButton = utils.getByTestId('PackagePolicyActionsUpgradeItem');
expect(upgradeButton).toBeDisabled();
});
Expand All @@ -150,7 +151,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies({ is_managed: true });
const packagePolicy = createMockPackagePolicy();
const { utils } = renderMenu({ agentPolicies, packagePolicy });
await act(async () => {
await waitFor(() => {
expect(utils.queryByText('Delete integration')).toBeNull();
});
});
Expand All @@ -159,7 +160,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies({ is_managed: false });
const packagePolicy = createMockPackagePolicy();
const { utils } = renderMenu({ agentPolicies, packagePolicy });
await act(async () => {
await waitFor(() => {
expect(utils.queryByText('Delete integration')).not.toBeNull();
});
});
Expand All @@ -168,7 +169,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies({ is_managed: false, supports_agentless: true });
const packagePolicy = createMockPackagePolicy();
const { utils } = renderMenu({ agentPolicies, packagePolicy });
await act(async () => {
await waitFor(() => {
expect(utils.queryByText('Delete integration')).not.toBeNull();
});
});
Expand All @@ -177,7 +178,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies();
const packagePolicy = createMockPackagePolicy({ hasUpgrade: true });
const { utils } = renderMenu({ agentPolicies, packagePolicy, showAddAgent: true });
await act(async () => {
await waitFor(() => {
expect(utils.queryByText('Add agent')).not.toBeNull();
});
});
Expand All @@ -186,7 +187,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies({ is_managed: true });
const packagePolicy = createMockPackagePolicy({ hasUpgrade: true });
const { utils } = renderMenu({ agentPolicies, packagePolicy, showAddAgent: true });
await act(async () => {
await waitFor(() => {
expect(utils.queryByText('Add agent')).toBeNull();
});
});
Expand All @@ -195,7 +196,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies({ supports_agentless: true });
const packagePolicy = createMockPackagePolicy({ hasUpgrade: true });
const { utils } = renderMenu({ agentPolicies, packagePolicy, showAddAgent: true });
await act(async () => {
await waitFor(() => {
expect(utils.queryByText('Add agent')).toBeNull();
});
});
Expand All @@ -204,7 +205,7 @@ describe('PackagePolicyActionsMenu', () => {
const agentPolicies = createMockAgentPolicies();
const packagePolicy = createMockPackagePolicy();
const { utils } = renderMenu({ agentPolicies, packagePolicy });
await act(async () => {
await waitFor(() => {
const editButton = utils.getByTestId('PackagePolicyActionsEditItem');
expect(editButton).not.toHaveAttribute('disabled');
expect(editButton).toHaveAttribute('href');
Expand All @@ -227,7 +228,7 @@ describe('PackagePolicyActionsMenu', () => {
agentPolicies: [],
packagePolicy,
});
await act(async () => {
await waitFor(() => {
const editButton = utils.getByTestId('PackagePolicyActionsEditItem');
expect(editButton).not.toHaveAttribute('disabled');
expect(editButton).toHaveAttribute('href');
Expand Down
Loading