[Security Solution] Skip all Detection & Response Cypress tests in Serverless #165966

banderror · 2023-09-07T12:27:13Z

Resolves: #164441

Summary

Skips all Cypress tests owned by @elastic/security-detection-rule-management and @elastic/security-detection-engine teams in Serverless using the new @skipInServerless tag.

Adds a new @skipInServerless tag
Updates x-pack/test/security_solution_cypress/cypress/README.md to explain when to use what tags
Explicitly adds missing tags to all D&R tests
Adds // TODO: comments to tests with links to follow-up tickets
Fixes the x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts script (see below)

Follow-up work:

Context

Serverless test failures will soon block PR merge

During the development of the serverless test infrastructure, we decided that serverless tests will only soft-fail. That means you see the test failure in your PR but you're still able to merge. We did that mainly in order to not block delivery of stateful features and bug fixes while serverless tests and pipelines were implemented and stabilized.
We now have the major building blocks for the serverless test infrastructure in place and will integrate serverless tests in our regular pipelines. As part of this process, we're skipping failing and flaky serverless tests that came in during the soft-fail phase. Once this is done, a PR with serverless test failures can no longer be merged, similar to how we have it for stateful test failures.

We plan to merge this in the next few days and we'll send out another notification when it's done.

Fixing `parallel.ts`

There were two problems with the x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts script we use for running Cypress tests:

The script was broken in ES Serverless support for FTR serverless tests. SSL support in kbn/es. kbn/es DX improvements. #162673 (here on this line)
I think it has never supported situations when all tests matching a spec pattern (such as ./cypress/e2e/!(investigations|explore)/**/*.cy.ts) end up being skipped via Cypress tags (such as @skipInServerless)

Both the issues are fixed in this PR.

Code owners are added for this script in the CODEOWNERS file to prevent breaking this script in future PRs.

elasticmachine · 2023-09-07T12:27:17Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine · 2023-09-07T12:27:17Z

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

banderror · 2023-09-08T15:37:55Z

.github/CODEOWNERS

@@ -1316,6 +1316,7 @@ x-pack/test/security_solution_cypress/config.ts @elastic/security-engineering-pr
 x-pack/test/security_solution_cypress/runner.ts @elastic/security-engineering-productivity
 x-pack/test/security_solution_cypress/serverless_config.ts @elastic/security-engineering-productivity
 x-pack/test/security_solution_cypress/cypress/tags.ts @elastic/security-engineering-productivity
+x-pack/plugins/security_solution/scripts/run_cypress @MadameSheema @patrykkopycinski @oatkiller @maximpn @banderror


FYI @MadameSheema @patrykkopycinski @oatkiller @maximpn

See the PR description for motivation.

banderror · 2023-09-08T15:39:54Z

x-pack/test/security_solution_cypress/package.json

@@ -19,7 +19,7 @@
      "junit:transform": "node ../../plugins/security_solution/scripts/junit_transformer --pathPattern '../../../target/kibana-security-solution/cypress/results/*.xml' --rootDirectory ../../../ --reportName 'Security Solution Cypress' --writeInPlace",
      "cypress:serverless": "TZ=UTC node ../../plugins/security_solution/scripts/start_cypress_parallel --config-file ../../test/security_solution_cypress/cypress/cypress_ci_serverless.config.ts --ftr-config-file ../../test/security_solution_cypress/serverless_config",
      "cypress:open:serverless": "yarn cypress:serverless open --config-file ../../test/security_solution_cypress/cypress/cypress_serverless.config.ts --spec './cypress/e2e/**/*.cy.ts'",
-      "cypress:run:serverless": "yarn cypress:serverless --spec '**/cypress/e2e/!(investigations|explore)/**/*.cy.ts'",
+      "cypress:run:serverless": "yarn cypress:serverless --spec './cypress/e2e/!(investigations|explore)/**/*.cy.ts'",


I also changed the pattern because it doesn't seem we need to support any nested subfolder structures which could be supported by **

maximpn · 2023-09-08T15:53:22Z

@elasticmachine merge upstream

yctercero · 2023-09-08T17:10:16Z

x-pack/test/security_solution_cypress/cypress/README.md

+- `@serverless` includes a test in the Serverless test suite. You need to explicitly add this tag to any test you want to run against a Serverless environment.
+- `@ess` includes a test in the normal, non-Serverless test suite. You need to explicitly add this tag to any test you want to run against a non-Serverless environment.
+- `@brokenInServerless` excludes a test from the Serverless test suite (even if it's tagged as `@serverless`). Indicates that a test should run in Serverless, but currently is broken.
+- `@skipInServerless` excludes a test from the Serverless test suite (even if it's tagged as `@serverless`). Could indicate many things, e.g. "the test is flaky in Serverless", "the test is Flaky in any type of environemnt", "the test has been temporarily excluded, see the comment above why".


Should we add @skipInEss? If we need to skip for ESS right now we have to skip for all right?

@yctercero This is a good point. I think we will need this tag eventually. And yes, if we need to skip for ESS right now we have to skip for all.

I'm not sure about adding it now because I think that "now we have to skip for all" is exactly what we should be doing now. Until we basically eliminate flakiness in our tests, we should treat a flaky test in ESS as being in general not yet fixed. Then, when we can say something like "ok, the rule creation Cypress tests have been all fixed and not flaky anymore in ESS and Serverless", then we could start skipping them in either environment if flakiness ever occurs again.

MadameSheema

security-engineering-productivity changes LGTM!!

Lots of thanks for doing this effort 🙏

yctercero

LGTM - thanks for the changes! Ready to help with the next step of re-enabling these for DE.

banderror · 2023-09-08T17:41:03Z

Most of the tests passed in https://buildkite.com/elastic/kibana-pull-request/builds/157714. The failed ones look like flaky tests. Also, the Serverless Security Cypress Tests job didn't run any tests and exited early, which is expected and exactly what we were aiming for when fixing the parallel.ts script.

info Config spec pattern: ./cypress/e2e/**/*.cy.ts
--
  | info Arguments spec pattern: ./cypress/e2e/!(investigations\|explore)/**/*.cy.ts
  | info Resulting spec pattern: ./cypress/e2e/!(investigations\|explore)/**/*.cy.ts
  | @cypress/grep: filtering using tag(s) "@serverless --@brokenInServerless --@skipInServerless"
  | @cypress/grep: will omit filtered tests
  | grep and/or grepTags has eliminated all specs
  | grepTags: @serverless --@brokenInServerless --@skipInServerless
  | Will leave all specs to run to filter at run-time
  | info Resolved spec files or pattern after grep: ./cypress/e2e/!(investigations\|explore)/**/*.cy.ts
  | info No tests found - all tests could have been skipped via Cypress tags
  | Done in 7.91s.

I also tested locally that if I enable at least one test using Cypress tags, the job will run it.

I will remove the "all cypress tests" label and restart the build - hope it takes less time after that.

spong

Thank you for the detailed description, updated docs, and providing context/next steps -- appreciate it @banderror! LGTM 👍

maximpn

Cool! This PR also fixes Threat Intelligence Cypress Tests cc @PhilippeOberti

kibana-ci · 2023-09-08T23:55:32Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] Explore - Security Solution Cypress Tests #1 / Cases Creates a new case with timeline and opens the timeline Creates a new case with timeline and opens the timeline

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	452	453	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	519	520	+1

History

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

cc @banderror

tomsonpl · 2023-09-11T09:09:11Z

Hey folks, this PR introduced a SKIP for all tests that do not contain tags yet. Please take a look eg. at this https://buildkite.com/elastic/kibana-pull-request/builds/157814#018a769b-185b-4d22-bb6d-e2bf155745c6

We have some false passes around :P

…rverless (elastic#165966) **Resolves: elastic#164441 ## Summary Skips all Cypress tests owned by @elastic/security-detection-rule-management and @elastic/security-detection-engine teams in Serverless using the new `@skipInServerless` tag. - Adds a new `@skipInServerless` tag - Updates `x-pack/test/security_solution_cypress/cypress/README.md` to explain when to use what tags - Explicitly adds missing tags to all D&R tests - Adds `// TODO:` comments to tests with links to follow-up tickets - Fixes the `x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts` script (see below) Follow-up work: - elastic#161540 - elastic#161539 ## Context > Serverless test failures will soon block PR merge > During the development of the serverless test infrastructure, we decided that serverless tests will only soft-fail. That means you see the test failure in your PR but you're still able to merge. We did that mainly in order to not block delivery of stateful features and bug fixes while serverless tests and pipelines were implemented and stabilized. We now have the major building blocks for the serverless test infrastructure in place and will integrate serverless tests in our regular pipelines. As part of this process, we're skipping failing and flaky serverless tests that came in during the soft-fail phase. Once this is done, a PR with serverless test failures can no longer be merged, similar to how we have it for stateful test failures. > We plan to merge this in the next few days and we'll send out another notification when it's done. ## Fixing `parallel.ts` There were two problems with the `x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts` script we use for running Cypress tests: - The script was broken in elastic#162673 (here on [this line](https://github.com/elastic/kibana/pull/162673/files#diff-9f40ced6d29c4fc2709af881680400293d8ce1bc9ebb07b9138d6d99c83c09c9R67)) - I think it has never supported situations when all tests matching a spec pattern (such as `./cypress/e2e/!(investigations|explore)/**/*.cy.ts`) end up being skipped via Cypress tags (such as `@skipInServerless`) Both the issues are fixed in this PR. Code owners are added for this script in the CODEOWNERS file to prevent breaking this script in future PRs.

banderror self-assigned this Sep 7, 2023

banderror requested review from a team as code owners September 7, 2023 12:27

banderror requested a review from xcrzx September 7, 2023 12:27

banderror requested review from maximpn, yctercero, WafaaNasr and MadameSheema and removed request for xcrzx September 7, 2023 12:29

banderror force-pushed the skip-detection-and-response-serverless-cypress-tests branch from b385328 to 0c7d76b Compare September 7, 2023 12:33

banderror requested a review from a team as a code owner September 7, 2023 13:24

banderror force-pushed the skip-detection-and-response-serverless-cypress-tests branch 4 times, most recently from 90c3bfc to ef8562a Compare September 8, 2023 11:57

banderror marked this pull request as draft September 8, 2023 12:02

banderror force-pushed the skip-detection-and-response-serverless-cypress-tests branch from ef8562a to c181eef Compare September 8, 2023 15:15

banderror requested a review from oatkiller September 8, 2023 15:26

banderror commented Sep 8, 2023

View reviewed changes

banderror marked this pull request as ready for review September 8, 2023 15:40

maximpn added the ci:all-cypress-suites label Sep 8, 2023

yctercero reviewed Sep 8, 2023

View reviewed changes

MadameSheema approved these changes Sep 8, 2023

View reviewed changes

yctercero approved these changes Sep 8, 2023

View reviewed changes

banderror removed the ci:all-cypress-suites label Sep 8, 2023

banderror force-pushed the skip-detection-and-response-serverless-cypress-tests branch from e1f4fa2 to 22c2ab1 Compare September 8, 2023 17:43

banderror enabled auto-merge (squash) September 8, 2023 17:55

spong approved these changes Sep 8, 2023

View reviewed changes

maximpn approved these changes Sep 8, 2023

View reviewed changes

banderror force-pushed the skip-detection-and-response-serverless-cypress-tests branch from 22c2ab1 to 9be0ae5 Compare September 8, 2023 21:02

banderror added 4 commits September 9, 2023 01:07

Skip all Detection & Response Cypress tests in Serverless

a089da7

Fixes after rebasing

ad332aa

Fix parallel.ts script and CI

4c7d628

Skip flaky tests

23ed5fc

banderror force-pushed the skip-detection-and-response-serverless-cypress-tests branch from 9be0ae5 to 23ed5fc Compare September 8, 2023 23:07

banderror merged commit 1d65e78 into elastic:main Sep 9, 2023
34 of 36 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Sep 9, 2023

banderror deleted the skip-detection-and-response-serverless-cypress-tests branch September 9, 2023 00:28

tomsonpl mentioned this pull request Sep 11, 2023

[EDR Workflows] [Cypress] Fix early return issue in cypress without tags #166140

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Skip all Detection & Response Cypress tests in Serverless #165966

[Security Solution] Skip all Detection & Response Cypress tests in Serverless #165966

banderror commented Sep 7, 2023 •

edited

elasticmachine commented Sep 7, 2023

elasticmachine commented Sep 7, 2023

banderror Sep 8, 2023

banderror Sep 8, 2023

maximpn commented Sep 8, 2023

yctercero Sep 8, 2023

banderror Sep 8, 2023

MadameSheema left a comment

yctercero left a comment

banderror commented Sep 8, 2023 •

edited

spong left a comment

maximpn left a comment

kibana-ci commented Sep 8, 2023 •

edited

ESLint disabled line counts

Total ESLint disabled count

tomsonpl commented Sep 11, 2023

[Security Solution] Skip all Detection & Response Cypress tests in Serverless #165966

[Security Solution] Skip all Detection & Response Cypress tests in Serverless #165966

Conversation

banderror commented Sep 7, 2023 • edited

Summary

Context

Fixing parallel.ts

elasticmachine commented Sep 7, 2023

elasticmachine commented Sep 7, 2023

banderror Sep 8, 2023

Choose a reason for hiding this comment

banderror Sep 8, 2023

Choose a reason for hiding this comment

maximpn commented Sep 8, 2023

yctercero Sep 8, 2023

Choose a reason for hiding this comment

banderror Sep 8, 2023

Choose a reason for hiding this comment

MadameSheema left a comment

Choose a reason for hiding this comment

yctercero left a comment

Choose a reason for hiding this comment

banderror commented Sep 8, 2023 • edited

spong left a comment

Choose a reason for hiding this comment

maximpn left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 8, 2023 • edited

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

tomsonpl commented Sep 11, 2023

banderror commented Sep 7, 2023 •

edited

Fixing `parallel.ts`

banderror commented Sep 8, 2023 •

edited

kibana-ci commented Sep 8, 2023 •

edited