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

[Security Solution] Removing cleanKibana method from Cypress #170636

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

MadameSheema
Copy link
Member

@MadameSheema MadameSheema commented Nov 6, 2023

Context

We are working to execute our Cypress tests in the second Quality Gate (QA environment, deployed projects on MKI). To do so we are mimicking the orchestration we currently have thanks to the parallel.ts script on our ESS and serverless FTR environment, where each spec file is executed in its own project and each spec file can set the PLI that needs to be executed.

As part of this effort, we are trying to make our Cypress tests more robust and reliable.

While testing the pipeline, we have seen that cleanKibana() method is a bit flaky on MKI. That might be because we are deleting too many things when it is not actually necessary.

Most of the tests use it on the before hook. This is not needed anymore since each spec file is executed in its own newly created project.

In this PR

In this PR we are removing cleanKibana from our Cypress tests in favor of cleaning just what we need instead of everything.

@MadameSheema MadameSheema changed the title removing clean kibana method [Security Solution] Removing cleanKibana method from Cypress Nov 6, 2023
@MadameSheema MadameSheema self-assigned this Nov 6, 2023
@MadameSheema MadameSheema added v8.12.0 release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Nov 6, 2023
@MadameSheema MadameSheema marked this pull request as ready for review November 6, 2023 18:57
@MadameSheema MadameSheema requested review from a team as code owners November 6, 2023 18:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)


// FLAKY: https://github.com/elastic/kibana/issues/165744
describe('Export timelines', { tags: ['@ess', '@serverless'] }, () => {
before(() => {
deleteTimelines();
login();
Copy link
Contributor

Choose a reason for hiding this comment

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

Now this one is interesting. Can you elaborate why we need to login now and before we didn't need to?

Copy link
Member Author

@MadameSheema MadameSheema Nov 6, 2023

Choose a reason for hiding this comment

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

I might messed up when solving conflicts :D

I'll revisit, thanks ❤️

Copy link
Member Author

Choose a reason for hiding this comment

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

For what I see we were doing a login on beforeEach, but just with the before hook is enough, we don't need to login twice :)

Copy link
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

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

THI changes look good to me

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @MadameSheema

Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

except for Jan's comment, everything else LGTM for the Threat Hunting Investigations team.
Awesome cleanup thank you! :)

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

detection engine area LGTM

Copy link
Contributor

@jpdjere jpdjere left a comment

Choose a reason for hiding this comment

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

Rule Management LGTM 👍

@MadameSheema To fully understand these ongoing changes: the running of each spec in its own separate project happens in CI for PRs as well, or only for QA/MKI quality gates?

Just making sure to prevent that we re-introduce flake for PR pipelines by removing all these cleanKibana() calls.

@MadameSheema
Copy link
Member Author

Rule Management LGTM 👍

@MadameSheema To fully understand these ongoing changes: the running of each spec in its own separate project happens in CI for PRs as well, or only for QA/MKI quality gates?

Just making sure to prevent that we re-introduce flake for PR pipelines by removing all these cleanKibana() calls.

Happens in CI for PRs and MKI quality gates. On CI with the FTR serverless and ESS environments we didn't find flakiness, we started to see it on real serverless projects. That might be also related because real serverless projects are slower than the environments we use to execute tests on PRs.

@MadameSheema MadameSheema merged commit e7efb67 into elastic:main Nov 7, 2023
6 checks passed
@MadameSheema MadameSheema deleted the removing-clean-kibana branch November 7, 2023 13:01
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 7, 2023
e40pud added a commit that referenced this pull request Nov 7, 2023
Followup to [Security Solution] Removing cleanKibana method from Cypress #170636 (#170636)
rylnd added a commit to rylnd/kibana that referenced this pull request Nov 8, 2023
It was removed in elastic#170636, and appears not to have been replaced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants