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

[Fleet] [Cloud Security] Add Testing Library ESLint for handling waitFor #198735

Merged
merged 13 commits into from
Nov 5, 2024

Conversation

opauloh
Copy link
Contributor

@opauloh opauloh commented Nov 1, 2024

Summary

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

It was also detected with https://github.com/elastic/security-team/issues/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 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 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.

@opauloh opauloh added the Team:Cloud Security Cloud Security team related label Nov 1, 2024
});
});

it('should have global_data_tags with the integration team when updating the agentless policy', 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.

test case deleted since it's not possible to update agent policy from the integration edit page.

@opauloh opauloh changed the title [Fleet] [Cloud Security] Add Testing Library ESLint [Fleet] [Cloud Security] Add Testing Library ESLint for handling waitFor Nov 1, 2024
@opauloh opauloh added v9.0.0 v8.16.0 backport:version Backport to applied version labels v8.17.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 1, 2024
@opauloh opauloh marked this pull request as ready for review November 4, 2024 21:57
@opauloh opauloh requested review from a team as code owners November 4, 2024 21:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

@elastic elastic deleted a comment from kibanamachine Nov 5, 2024
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 1.7MB 1.7MB +5.0B

History

@opauloh opauloh merged commit 5ab59fb into elastic:main Nov 5, 2024
40 checks passed
@opauloh opauloh deleted the reliability/add-eslint-waitFor branch November 5, 2024 22:34
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11693637360

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.16 Backport failed because of merge conflicts
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 198735

Questions ?

Please refer to the Backport tool documentation

opauloh added a commit to opauloh/kibana that referenced this pull request Nov 5, 2024
…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
opauloh added a commit to opauloh/kibana that referenced this pull request Nov 5, 2024
…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
#	x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/single_page_layout/hooks/setup_technology.ts
@opauloh
Copy link
Contributor Author

opauloh commented Nov 5, 2024

💚 All backports created successfully

Status Branch Result
8.x
8.16

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@Dosant
Copy link
Contributor

Dosant commented Nov 6, 2024

Additional note: These changes should also unblock the failing tests on the #195087 PR

@opauloh nice! thank you 🙇‍♂️

mgadewoll pushed a commit to mgadewoll/kibana that referenced this pull request Nov 7, 2024
…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.
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 7, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@maxcold
Copy link
Contributor

maxcold commented Nov 8, 2024

@opauloh I see that you closed the backport to 8.16 pr. Do we not want this change in 8.16?

opauloh added a commit that referenced this pull request Nov 8, 2024
…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-->
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 8, 2024
@opauloh
Copy link
Contributor Author

opauloh commented Nov 9, 2024

@opauloh I see that you closed the backport to 8.16 pr. Do we not want this change in 8.16?

I closed because backporting that PR alone would introduce an issue where the agentless API won't accept the global data tags, in that case I concluded the absence of the tags would be better than backporting the issue spotted here to 8.16.0. We fixed the issue in another PR but not in time for the last 8.16.0 BC.

On my understanding the purposes of sending the global data tags in the policy are for labelling the created containers, the labels are not a critical requirement as it's mainly for internal infrastructure control, so it didn't have to be a release blocker, as it wouldn't prevent customers from experimenting with the Beta Agentless Feature.

If the labels essentially needed to go to the 8.16 branch, a backport on both this PR and #199247 is needed in order to have the labeling functionality working properly, however that would target 8.16.1 as it doesn't seem we are going to have another Build Candidate for 8.16.0.

pinging @seanrathier for acknowledgement

@opauloh opauloh removed the v8.16.0 label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment