Skip to content

Commit

Permalink
[8.x] [Fleet] [Cloud Security] Add Testing Library ESLint for handlin…
Browse files Browse the repository at this point in the history
…g waitFor (#198735) (#199067)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Fleet] [Cloud Security] Add Testing Library ESLint for handling
waitFor (#198735)](#198735)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Paulo
Silva","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-05T22:34:18Z","message":"[Fleet]
[Cloud Security] Add Testing Library ESLint for handling waitFor
(#198735)\n\n## Summary\r\n\r\nThis PR aims to fix Flaky tests related
to agentless detected
by\r\nhttps://github.com//issues/189038
and\r\nhttps://github.com//issues/192126 by adding
proper\r\nhandling of the `waitFor` methods.\r\n\r\nIt was also detected
with\r\nhttps://github.com/elastic/security-team/issues/10979 that some
other\r\nmethods were not proper handled by `waitFor`, leading to the
assertions\r\ninside those unhandled `waitFor` being skipped by
Jest.\r\n\r\nThis PR also introduces ESLint to enforce proper handling
of waitFor\r\nmethods in tests files for Fleet and Cloud Security
plugins.\r\n\r\nAdditional note: These changes should also unblock the
failing tests on\r\nthe [React18 use waitFor with assertion callbacks in
place
of\r\nwaitForNextUpdate](#195087)
PR\r\n\r\n\r\n**Fleet changes**\r\n\r\n- ESLint rule added to enforce
handling `waitFor` on React Testing\r\nLibrary.\r\n-
`useSetupTechnology` hook tests reviewed and updated to handle
the\r\nwaitFor. Fixed issue identified when reviewing the tests.\r\n-
step_define_package_policy.test.tsx: Added package policy vars to
the\r\nmock to proper handle the use cases\r\n-
step_select_hosts.test.tsx: Handled waitFor, identified outdated
test\r\n- step_edit_hosts.test.tsx: Handled waitFor, identified outdated
test\r\nWith the introduction of the ESLint rule other tests were
triggering\r\nESLint errors, I attempted to fix them while retaining the
same\r\nintention, let me know if more changes are
needed.\r\n\r\n**Cloud Security changes**\r\n\r\n- ESLint rule added to
enforce handling `waitFor` on React Testing\r\nLibrary.\r\n- Updated
cloud security posture version to include agentless global\r\ntags on
End to End tests\r\n\r\n**@elastic/kibana-operations changes**\r\n\r\n-
Added\r\n[eslint-plugin-testing-library](https://testing-library.com/docs/ecosystem-eslint-plugin-testing-library/)\r\nan
ESLint plugin for Testing Library that helps users to follow
best\r\npractices and anticipate common mistakes when writing
tests.\r\n- The adoption and enablement of the rules are
opt-in.","sha":"5ab59fba401a189c290e55b3f73fd4fd23106e13","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","Team:Cloud
Security","v8.16.0","backport:version","v8.17.0"],"number":198735,"url":"https://github.com/elastic/kibana/pull/198735","mergeCommit":{"message":"[Fleet]
[Cloud Security] Add Testing Library ESLint for handling waitFor
(#198735)\n\n## Summary\r\n\r\nThis PR aims to fix Flaky tests related
to agentless detected
by\r\nhttps://github.com//issues/189038
and\r\nhttps://github.com//issues/192126 by adding
proper\r\nhandling of the `waitFor` methods.\r\n\r\nIt was also detected
with\r\nhttps://github.com/elastic/security-team/issues/10979 that some
other\r\nmethods were not proper handled by `waitFor`, leading to the
assertions\r\ninside those unhandled `waitFor` being skipped by
Jest.\r\n\r\nThis PR also introduces ESLint to enforce proper handling
of waitFor\r\nmethods in tests files for Fleet and Cloud Security
plugins.\r\n\r\nAdditional note: These changes should also unblock the
failing tests on\r\nthe [React18 use waitFor with assertion callbacks in
place
of\r\nwaitForNextUpdate](#195087)
PR\r\n\r\n\r\n**Fleet changes**\r\n\r\n- ESLint rule added to enforce
handling `waitFor` on React Testing\r\nLibrary.\r\n-
`useSetupTechnology` hook tests reviewed and updated to handle
the\r\nwaitFor. Fixed issue identified when reviewing the tests.\r\n-
step_define_package_policy.test.tsx: Added package policy vars to
the\r\nmock to proper handle the use cases\r\n-
step_select_hosts.test.tsx: Handled waitFor, identified outdated
test\r\n- step_edit_hosts.test.tsx: Handled waitFor, identified outdated
test\r\nWith the introduction of the ESLint rule other tests were
triggering\r\nESLint errors, I attempted to fix them while retaining the
same\r\nintention, let me know if more changes are
needed.\r\n\r\n**Cloud Security changes**\r\n\r\n- ESLint rule added to
enforce handling `waitFor` on React Testing\r\nLibrary.\r\n- Updated
cloud security posture version to include agentless global\r\ntags on
End to End tests\r\n\r\n**@elastic/kibana-operations changes**\r\n\r\n-
Added\r\n[eslint-plugin-testing-library](https://testing-library.com/docs/ecosystem-eslint-plugin-testing-library/)\r\nan
ESLint plugin for Testing Library that helps users to follow
best\r\npractices and anticipate common mistakes when writing
tests.\r\n- The adoption and enablement of the rules are
opt-in.","sha":"5ab59fba401a189c290e55b3f73fd4fd23106e13"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198735","number":198735,"mergeCommit":{"message":"[Fleet]
[Cloud Security] Add Testing Library ESLint for handling waitFor
(#198735)\n\n## Summary\r\n\r\nThis PR aims to fix Flaky tests related
to agentless detected
by\r\nhttps://github.com//issues/189038
and\r\nhttps://github.com//issues/192126 by adding
proper\r\nhandling of the `waitFor` methods.\r\n\r\nIt was also detected
with\r\nhttps://github.com/elastic/security-team/issues/10979 that some
other\r\nmethods were not proper handled by `waitFor`, leading to the
assertions\r\ninside those unhandled `waitFor` being skipped by
Jest.\r\n\r\nThis PR also introduces ESLint to enforce proper handling
of waitFor\r\nmethods in tests files for Fleet and Cloud Security
plugins.\r\n\r\nAdditional note: These changes should also unblock the
failing tests on\r\nthe [React18 use waitFor with assertion callbacks in
place
of\r\nwaitForNextUpdate](#195087)
PR\r\n\r\n\r\n**Fleet changes**\r\n\r\n- ESLint rule added to enforce
handling `waitFor` on React Testing\r\nLibrary.\r\n-
`useSetupTechnology` hook tests reviewed and updated to handle
the\r\nwaitFor. Fixed issue identified when reviewing the tests.\r\n-
step_define_package_policy.test.tsx: Added package policy vars to
the\r\nmock to proper handle the use cases\r\n-
step_select_hosts.test.tsx: Handled waitFor, identified outdated
test\r\n- step_edit_hosts.test.tsx: Handled waitFor, identified outdated
test\r\nWith the introduction of the ESLint rule other tests were
triggering\r\nESLint errors, I attempted to fix them while retaining the
same\r\nintention, let me know if more changes are
needed.\r\n\r\n**Cloud Security changes**\r\n\r\n- ESLint rule added to
enforce handling `waitFor` on React Testing\r\nLibrary.\r\n- Updated
cloud security posture version to include agentless global\r\ntags on
End to End tests\r\n\r\n**@elastic/kibana-operations changes**\r\n\r\n-
Added\r\n[eslint-plugin-testing-library](https://testing-library.com/docs/ecosystem-eslint-plugin-testing-library/)\r\nan
ESLint plugin for Testing Library that helps users to follow
best\r\npractices and anticipate common mistakes when writing
tests.\r\n- The adoption and enablement of the rules are
opt-in.","sha":"5ab59fba401a189c290e55b3f73fd4fd23106e13"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
  • Loading branch information
opauloh authored Nov 8, 2024
1 parent 2be626f commit 31eb316
Show file tree
Hide file tree
Showing 9 changed files with 302 additions and 158 deletions.
12 changes: 12 additions & 0 deletions .eslintrc.js
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
Original file line number Diff line number Diff line change
Expand Up @@ -1715,6 +1715,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
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';
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
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 31eb316

Please sign in to comment.