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] Cypress flaky tests catcher #162376

Conversation

patrykkopycinski
Copy link
Contributor

@patrykkopycinski patrykkopycinski commented Jul 21, 2023

Summary

Inspired by https://glebbahmutov.com/blog/burning-tests/

Implements the idea presented here https://glebbahmutov.com/blog/burning-tests/#bonus-3-burning-new-or-changed-specs

In short, on PR that is changing/adding new Cypress spec files we will try to "burn" them, it means we will try to run each it 2 times to make sure tests are written in a way that gives Cypress a chance to recover from the failed test.
Also adding a command that allows to "burn" tests locally

yarn cypress:burn --spec "<>"

Right now the job is set to soft_fail, so it is not going to block the PR from merging, but hopefully will help the Team to recognize potential flakiness before it is merged to main

@patrykkopycinski patrykkopycinski marked this pull request as ready for review August 4, 2023 12:07
@patrykkopycinski patrykkopycinski requested review from a team as code owners August 4, 2023 12:07
@patrykkopycinski patrykkopycinski changed the title Chore/cypress flaky tests runner [security_solution] Cypress flaky tests catcher Aug 4, 2023
@patrykkopycinski patrykkopycinski self-assigned this Aug 4, 2023
@patrykkopycinski
Copy link
Contributor Author

@elasticmachine test this please

@patrykkopycinski patrykkopycinski enabled auto-merge (squash) August 8, 2023 21:33
@patrykkopycinski patrykkopycinski added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Aug 8, 2023
@patrykkopycinski
Copy link
Contributor Author

buildkite test this

@patrykkopycinski patrykkopycinski added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Aug 9, 2023
@patrykkopycinski
Copy link
Contributor Author

buildkite test this

@patrykkopycinski patrykkopycinski merged commit 09aaecb into elastic:main Aug 9, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 461 462 +1

Total ESLint disabled count

id before after diff
securitySolution 524 525 +1

History

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

cc @patrykkopycinski

@kibanamachine kibanamachine added v8.10.0 backport:skip This commit does not require backporting labels Aug 9, 2023
@mistic
Copy link
Member

mistic commented Aug 15, 2023

@patrykkopycinski I think we can now backport this into the 7.17 branch after the work we have done at #163363

patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request Aug 15, 2023
## Summary

Inspired by https://glebbahmutov.com/blog/burning-tests/

Implements the idea presented here
https://glebbahmutov.com/blog/burning-tests/#bonus-3-burning-new-or-changed-specs

In short, on PR that is changing/adding new Cypress spec files we will
try to "burn" them, it means we will try to run each `it` `2` times to
make sure tests are written in a way that gives Cypress a chance to
recover from the failed test.
Also adding a command that allows to "burn" tests locally
```
yarn cypress:burn --spec "<>"
```

Right now the job is set to `soft_fail`, so it is not going to block the
PR from merging, but hopefully will help the Team to recognize potential
flakiness before it is merged to `main`

(cherry picked from commit 09aaecb)

# Conflicts:
#	package.json
#	typings/index.d.ts
#	x-pack/plugins/security_solution/cypress/scripts/run_cypress/parallel.ts
#	x-pack/plugins/security_solution/cypress/support/e2e.js
#	x-pack/plugins/security_solution/package.json
#	yarn.lock
patrykkopycinski added a commit that referenced this pull request Aug 22, 2023
…3992)

# Backport

This will backport the following commits from `main` to `7.17`:
- [[security_solution] Cypress flaky tests catcher
(#162376)](#162376)

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

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

<!--BACKPORT [{"author":{"name":"Patryk
Kopyciński","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-09T21:58:04Z","message":"[security_solution]
Cypress flaky tests catcher (#162376)\n\n## Summary\r\n\r\nInspired by
https://glebbahmutov.com/blog/burning-tests/\r\n\r\nImplements the idea
presented
here\r\nhttps://glebbahmutov.com/blog/burning-tests/#bonus-3-burning-new-or-changed-specs\r\n\r\nIn
short, on PR that is changing/adding new Cypress spec files we
will\r\ntry to \"burn\" them, it means we will try to run each `it` `2`
times to\r\nmake sure tests are written in a way that gives Cypress a
chance to\r\nrecover from the failed test.\r\nAlso adding a command that
allows to \"burn\" tests locally\r\n```\r\nyarn cypress:burn --spec
\"<>\"\r\n```\r\n\r\nRight now the job is set to `soft_fail`, so it is
not going to block the\r\nPR from merging, but hopefully will help the
Team to recognize potential\r\nflakiness before it is merged to
`main`","sha":"09aaecb59d5e82e17bf6f274de3cedd394bbe25b","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","v8.10.0"],"number":162376,"url":"https://github.com/elastic/kibana/pull/162376","mergeCommit":{"message":"[security_solution]
Cypress flaky tests catcher (#162376)\n\n## Summary\r\n\r\nInspired by
https://glebbahmutov.com/blog/burning-tests/\r\n\r\nImplements the idea
presented
here\r\nhttps://glebbahmutov.com/blog/burning-tests/#bonus-3-burning-new-or-changed-specs\r\n\r\nIn
short, on PR that is changing/adding new Cypress spec files we
will\r\ntry to \"burn\" them, it means we will try to run each `it` `2`
times to\r\nmake sure tests are written in a way that gives Cypress a
chance to\r\nrecover from the failed test.\r\nAlso adding a command that
allows to \"burn\" tests locally\r\n```\r\nyarn cypress:burn --spec
\"<>\"\r\n```\r\n\r\nRight now the job is set to `soft_fail`, so it is
not going to block the\r\nPR from merging, but hopefully will help the
Team to recognize potential\r\nflakiness before it is merged to
`main`","sha":"09aaecb59d5e82e17bf6f274de3cedd394bbe25b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/162376","number":162376,"mergeCommit":{"message":"[security_solution]
Cypress flaky tests catcher (#162376)\n\n## Summary\r\n\r\nInspired by
https://glebbahmutov.com/blog/burning-tests/\r\n\r\nImplements the idea
presented
here\r\nhttps://glebbahmutov.com/blog/burning-tests/#bonus-3-burning-new-or-changed-specs\r\n\r\nIn
short, on PR that is changing/adding new Cypress spec files we
will\r\ntry to \"burn\" them, it means we will try to run each `it` `2`
times to\r\nmake sure tests are written in a way that gives Cypress a
chance to\r\nrecover from the failed test.\r\nAlso adding a command that
allows to \"burn\" tests locally\r\n```\r\nyarn cypress:burn --spec
\"<>\"\r\n```\r\n\r\nRight now the job is set to `soft_fail`, so it is
not going to block the\r\nPR from merging, but hopefully will help the
Team to recognize potential\r\nflakiness before it is merged to
`main`","sha":"09aaecb59d5e82e17bf6f274de3cedd394bbe25b"}}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Tiago Costa <[email protected]>
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 v7.17.13 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants