Skip to content

Commit

Permalink
[Fleet] [Cloud Security] Add Testing Library ESLint for handling wait…
Browse files Browse the repository at this point in the history 
…For (elastic#198735)

## Summary

This PR aims to fix Flaky tests related to agentless detected by
elastic#189038 and
elastic#192126 by adding proper
handling of the `waitFor` methods.

It was also detected with
elastic/security-team#10979 that some other
methods were not proper handled by `waitFor`, leading to the assertions
inside those unhandled `waitFor` being skipped by Jest.

This PR also introduces ESLint to enforce proper handling of waitFor
methods in tests files for Fleet and Cloud Security plugins.

Additional note: These changes should also unblock the failing tests on
the [React18 use waitFor with assertion callbacks in place of
waitForNextUpdate](elastic#195087) PR

**Fleet changes**

- ESLint rule added to enforce handling `waitFor` on React Testing
Library.
- `useSetupTechnology` hook tests reviewed and updated to handle the
waitFor. Fixed issue identified when reviewing the tests.
- step_define_package_policy.test.tsx: Added package policy vars to the
mock to proper handle the use cases
- step_select_hosts.test.tsx: Handled waitFor, identified outdated test
- step_edit_hosts.test.tsx: Handled waitFor, identified outdated test
With the introduction of the ESLint rule other tests were triggering
ESLint errors, I attempted to fix them while retaining the same
intention, let me know if more changes are needed.

**Cloud Security changes**

- ESLint rule added to enforce handling `waitFor` on React Testing
Library.
- Updated cloud security posture version to include agentless global
tags on End to End tests

**@elastic/kibana-operations changes**

- Added
[eslint-plugin-testing-library](https://testing-library.com/docs/ecosystem-eslint-plugin-testing-library/)
an ESLint plugin for Testing Library that helps users to follow best
practices and anticipate common mistakes when writing tests.
- The adoption and enablement of the rules are opt-in.

(cherry picked from commit 5ab59fb)

# Conflicts:
#	x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/single_page_layout/hooks/setup_technology.test.ts
  • Loading branch information
@opauloh
opauloh committed Nov 5, 2024
1 parent 2120e2a commit 70ba927
Show file tree
Hide file tree
Showing 9 changed files with 302 additions and 158 deletions.
12 changes: 12 additions & 0 deletions .eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,9 @@ module.exports = {
*/
{
files: ['x-pack/plugins/fleet/**/*.{js,mjs,ts,tsx}'],
plugins: ['testing-library'],
rules: {
'testing-library/await-async-utils': 'error',
'@typescript-eslint/consistent-type-imports': 'error',
'import/order': [
'warn',
Expand Down Expand Up @@ -1954,6 +1956,16 @@ module.exports = {
},
},

/**
* Cloud Security Team overrides
*/
{
files: ['x-pack/plugins/cloud_security_posture/**/*.{js,mjs,ts,tsx}'],
plugins: ['testing-library'],
rules: {
'testing-library/await-async-utils': 'error',
},
},
/**
* Code inside .buildkite runs separately from everything else in CI, before bootstrap, with ts-node. It needs a few tweaks because of this.
*/
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1714,6 +1714,7 @@
"eslint-plugin-react": "^7.32.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-react-perf": "^3.3.1",
"eslint-plugin-testing-library": "^6.4.0",
"eslint-traverse": "^1.0.0",
"exit-hook": "^2.2.0",
"expect": "^29.7.0",
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cloud_security_posture/common/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,4 @@ export const SINGLE_ACCOUNT = 'single-account';

export const CLOUD_SECURITY_PLUGIN_VERSION = '1.9.0';
// Cloud Credentials Template url was implemented in 1.10.0-preview01. See PR - https://github.com/elastic/integrations/pull/9828
export const CLOUD_CREDENTIALS_PACKAGE_VERSION = '1.11.0-preview10';
export const CLOUD_CREDENTIALS_PACKAGE_VERSION = '1.11.0-preview13';
122 changes: 79 additions & 43 deletions ...nt_policy/create_package_policy_page/components/steps/step_define_package_policy.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
*/

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

import { userEvent } from '@testing-library/user-event';

import { getInheritedNamespace } from '../../../../../../../../common/services';

Expand Down Expand Up @@ -60,18 +62,6 @@ describe('StepDefinePackagePolicy', () => {
package_policies: [],
is_protected: false,
},
{
id: 'agent-policy-2',
namespace: 'default',
name: 'Agent policy 2',
is_managed: false,
status: 'active',
updated_at: '',
updated_by: '',
revision: 1,
package_policies: [],
is_protected: false,
},
];
let packagePolicy: NewPackagePolicy;
const mockUpdatePackagePolicy = jest.fn().mockImplementation((val: any) => {
Expand All @@ -86,78 +76,124 @@ describe('StepDefinePackagePolicy', () => {
description: null,
namespace: null,
inputs: {},
vars: {},
vars: {
'Required var': ['Required var is required'],
},
};

let testRenderer: TestRenderer;
let renderResult: ReturnType<typeof testRenderer.render>;
const render = () =>

const render = (namespacePlaceholder = getInheritedNamespace(agentPolicies)) =>
(renderResult = testRenderer.render(
<StepDefinePackagePolicy
namespacePlaceholder={getInheritedNamespace(agentPolicies)}
namespacePlaceholder={namespacePlaceholder}
packageInfo={packageInfo}
packagePolicy={packagePolicy}
updatePackagePolicy={mockUpdatePackagePolicy}
validationResults={validationResults}
submitAttempted={false}
submitAttempted={true}
/>
));

beforeEach(() => {
packagePolicy = {
name: '',
description: 'desc',
namespace: 'default',
namespace: 'package-policy-ns',
enabled: true,
policy_id: '',
policy_ids: [''],
enabled: true,
package: {
name: 'apache',
title: 'Apache',
version: '1.0.0',
},
inputs: [],
vars: {
'Show user var': {
type: 'string',
value: 'showUserVarVal',
},
'Required var': {
type: 'bool',
value: undefined,
},
'Advanced var': {
type: 'bool',
value: true,
},
},
};
testRenderer = createFleetTestRendererMock();
});

describe('default API response', () => {
beforeEach(() => {
render();
});

it('should display vars coming from package policy', async () => {
waitFor(() => {
expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
expect(renderResult.getByRole('switch')).toHaveAttribute('aria-label', 'Required var');
expect(renderResult.getByText('Required var is required')).toHaveAttribute(
'class',
'euiFormErrorText'
act(() => {
render();
});
expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
expect(renderResult.getByRole('switch', { name: 'Required var' })).toBeInTheDocument();
expect(renderResult.queryByRole('switch', { name: 'Advanced var' })).not.toBeInTheDocument();

expect(renderResult.getByText('Required var is required')).toHaveClass('euiFormErrorText');

await userEvent.click(renderResult.getByText('Advanced options').closest('button')!);

await waitFor(() => {
expect(renderResult.getByRole('switch', { name: 'Advanced var' })).toBeInTheDocument();
expect(renderResult.getByTestId('packagePolicyNamespaceInput')).toHaveTextContent(
'package-policy-ns'
);
});
});

await act(async () => {
fireEvent.click(renderResult.getByText('Advanced options').closest('button')!);
it(`should display namespace from agent policy when there's no package policy namespace`, async () => {
packagePolicy.namespace = '';
act(() => {
render();
});

waitFor(() => {
expect(renderResult.getByRole('switch')).toHaveAttribute('aria-label', 'Advanced var');
expect(renderResult.getByTestId('packagePolicyNamespaceInput')).toHaveAttribute(
await userEvent.click(renderResult.getByText('Advanced options').closest('button')!);

await waitFor(() => {
expect(renderResult.getByTestId('comboBoxSearchInput')).toHaveAttribute(
'placeholder',
'ns'
);
});
});

it(`should fallback to the default namespace when namespace is not set in package policy and there's no agent policy`, async () => {
packagePolicy.namespace = '';
act(() => {
render(getInheritedNamespace([]));
});

await userEvent.click(renderResult.getByText('Advanced options').closest('button')!);

await waitFor(() => {
expect(renderResult.getByTestId('comboBoxSearchInput')).toHaveAttribute(
'placeholder',
'default'
);
});
});
});

describe('update', () => {
describe('when package vars are introduced in a new package version', () => {
it('should display new package vars', () => {
render();

waitFor(async () => {
expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
expect(renderResult.getByText('Required var')).toBeInTheDocument();
it('should display new package vars', async () => {
act(() => {
render();
});
expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
expect(renderResult.getByText('Required var')).toBeInTheDocument();

await act(async () => {
fireEvent.click(renderResult.getByText('Advanced options').closest('button')!);
});
await userEvent.click(renderResult.getByText('Advanced options').closest('button')!);

await waitFor(async () => {
expect(renderResult.getByText('Advanced var')).toBeInTheDocument();
});
});
Expand Down
46 changes: 21 additions & 25 deletions ...tions/agent_policy/create_package_policy_page/components/steps/step_select_hosts.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
*/

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

import { userEvent } from '@testing-library/user-event';

import type { TestRenderer } from '../../../../../../../mock';
import { createFleetTestRendererMock } from '../../../../../../../mock';
Expand Down Expand Up @@ -108,22 +110,23 @@ describe('StepSelectHosts', () => {
testRenderer = createFleetTestRendererMock();
});

it('should display create form when no agent policies', () => {
it('should display create form when no agent policies', async () => {
(useGetAgentPolicies as jest.MockedFunction<any>).mockReturnValue({
data: {
items: [],
},
});
(useAllNonManagedAgentPolicies as jest.MockedFunction<any>).mockReturnValue([]);

render();

waitFor(() => {
expect(renderResult.getByText('Agent policy 1')).toBeInTheDocument();
await waitFor(() => {
expect(renderResult.getByText('New agent policy name')).toBeInTheDocument();
});
expect(renderResult.queryByRole('tablist')).not.toBeInTheDocument();
});

it('should display tabs with New hosts selected when agent policies exist', () => {
it('should display tabs with New hosts selected when agent policies exist', async () => {
(useGetAgentPolicies as jest.MockedFunction<any>).mockReturnValue({
data: {
items: [{ id: '1', name: 'Agent policy 1', namespace: 'default' }],
Expand All @@ -135,10 +138,7 @@ describe('StepSelectHosts', () => {

render();

waitFor(() => {
expect(renderResult.getByRole('tablist')).toBeInTheDocument();
expect(renderResult.getByText('Agent policy 3')).toBeInTheDocument();
});
expect(renderResult.getByRole('tablist')).toBeInTheDocument();
expect(renderResult.getByText('New hosts').closest('button')).toHaveAttribute(
'aria-selected',
'true'
Expand All @@ -157,16 +157,15 @@ describe('StepSelectHosts', () => {

render();

waitFor(() => {
expect(renderResult.getByRole('tablist')).toBeInTheDocument();
});
act(() => {
fireEvent.click(renderResult.getByText('Existing hosts').closest('button')!);
});
expect(renderResult.getByRole('tablist')).toBeInTheDocument();

await userEvent.click(renderResult.getByText('Existing hosts').closest('button')!);

expect(
renderResult.container.querySelector('[data-test-subj="agentPolicySelect"]')?.textContent
).toContain('Agent policy 1');
await waitFor(() => {
expect(
renderResult.container.querySelector('[data-test-subj="agentPolicySelect"]')?.textContent
).toContain('Agent policy 1');
});
});

it('should display dropdown without preselected value when Existing hosts selected with mulitple agent policies', async () => {
Expand All @@ -185,14 +184,11 @@ describe('StepSelectHosts', () => {

render();

waitFor(() => {
expect(renderResult.getByRole('tablist')).toBeInTheDocument();
});
act(() => {
fireEvent.click(renderResult.getByText('Existing hosts').closest('button')!);
});
expect(renderResult.getByRole('tablist')).toBeInTheDocument();

await userEvent.click(renderResult.getByText('Existing hosts').closest('button')!);

await act(async () => {
await waitFor(() => {
const select = renderResult.container.querySelector('[data-test-subj="agentPolicySelect"]');
expect((select as any)?.value).toEqual('');
});
Expand Down
Loading

0 comments on commit 70ba927

Please sign in to comment.