Skip to content

Commit

Permalink
[8.12] Allow members of @security-solution-test-skippers to use `/ski…
Browse files Browse the repository at this point in the history
…p` comments on failed test issues (#173430) (#173687)

# Backport

This will backport the following commits from `main` to `8.12`:
- [Allow members of @security-solution-test-skippers to use `/skip`
comments on failed test issues
(#173430)](#173430)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Robert
Austin","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-12-19T19:13:12Z","message":"Allow
members of @security-solution-test-skippers to use `/skip` comments on
failed test issues (#173430)\n\n## Summary\r\n\r\nAfter a PR is merged,
CI runs all the tests on `main`. If one fails, a\r\ngithub issue with
the label `failed-test` is created. If you comment\r\n`/skip` on the
issue then a workflow action will skip the test directly\r\non the
failing branches.\r\n\r\nThis workflow can only be used by contributors
who have `admin` access\r\nto Kibana, OR to people who are on an
allow-list of elastic teams. The\r\nlist of teams allowed is
`appex-qa`.\r\n\r\nThis PR adds another team to the
list:\r\n@elastic/security-solution-test-skippers\r\nThis team was just
created to allow a small list of Security Solution\r\ncontributors the
ability to use this workflow.\r\n\r\nAllowing a few of us access to this
workflow will allow Security to\r\ntriage our own test by writing
`/skip` comments.\r\n\r\n### Without this PR we have to:\r\n\r\nWithout
this PR we have to do a bunch of stuff to skip our own
flaky\r\ntests:\r\n\r\n1. Manually edit the files to create a PR on all
affected branches.\r\n2. Create PRs using the web app.\r\n3. Get a
second developer to approve the PR.\r\n4. Wait for CI.\r\n5. Merge the
PR.\r\n\r\nThis process is slow because of manual steps, and because CI
must run\r\nbefore we can skip the test. At a minimum we should expect
that process\r\nto take as long as CI takes (e.g. 90 minutes.) But the
PRs that are\r\nmeant to skip tests may also in turn be blocked by flaky
tests. It also\r\nrequires coordination from 2 people to make and
approve the PR.\r\n\r\n### Code
references\r\n\r\n\r\n[skip-failed-test.yml](https://github.com/elastic/kibana/blob/main/.github/workflows/skip-failed-test.yml#L25-L30)\r\nis
where the workflow is defined\r\n\r\n[Docs on the syntax
for\r\nworkflows.](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsuses)\r\n\r\n\r\n[permission-check/index.ts](https://github.com/elastic/kibana-github-actions/blob/main/permission-check/index.ts#L23-L26)\r\nis
where the permission check is implemented. Users need to have:\r\n*
either the right permission level\r\n* or be a member of the
team\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Gloria Hornero
<[email protected]>","sha":"dd765ab84c98ddd823dc48152c36bc7ee6a1bd81","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:
SecuritySolution","v8.12.0","v8.13.0"],"number":173430,"url":"https://github.com/elastic/kibana/pull/173430","mergeCommit":{"message":"Allow
members of @security-solution-test-skippers to use `/skip` comments on
failed test issues (#173430)\n\n## Summary\r\n\r\nAfter a PR is merged,
CI runs all the tests on `main`. If one fails, a\r\ngithub issue with
the label `failed-test` is created. If you comment\r\n`/skip` on the
issue then a workflow action will skip the test directly\r\non the
failing branches.\r\n\r\nThis workflow can only be used by contributors
who have `admin` access\r\nto Kibana, OR to people who are on an
allow-list of elastic teams. The\r\nlist of teams allowed is
`appex-qa`.\r\n\r\nThis PR adds another team to the
list:\r\n@elastic/security-solution-test-skippers\r\nThis team was just
created to allow a small list of Security Solution\r\ncontributors the
ability to use this workflow.\r\n\r\nAllowing a few of us access to this
workflow will allow Security to\r\ntriage our own test by writing
`/skip` comments.\r\n\r\n### Without this PR we have to:\r\n\r\nWithout
this PR we have to do a bunch of stuff to skip our own
flaky\r\ntests:\r\n\r\n1. Manually edit the files to create a PR on all
affected branches.\r\n2. Create PRs using the web app.\r\n3. Get a
second developer to approve the PR.\r\n4. Wait for CI.\r\n5. Merge the
PR.\r\n\r\nThis process is slow because of manual steps, and because CI
must run\r\nbefore we can skip the test. At a minimum we should expect
that process\r\nto take as long as CI takes (e.g. 90 minutes.) But the
PRs that are\r\nmeant to skip tests may also in turn be blocked by flaky
tests. It also\r\nrequires coordination from 2 people to make and
approve the PR.\r\n\r\n### Code
references\r\n\r\n\r\n[skip-failed-test.yml](https://github.com/elastic/kibana/blob/main/.github/workflows/skip-failed-test.yml#L25-L30)\r\nis
where the workflow is defined\r\n\r\n[Docs on the syntax
for\r\nworkflows.](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsuses)\r\n\r\n\r\n[permission-check/index.ts](https://github.com/elastic/kibana-github-actions/blob/main/permission-check/index.ts#L23-L26)\r\nis
where the permission check is implemented. Users need to have:\r\n*
either the right permission level\r\n* or be a member of the
team\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Gloria Hornero
<[email protected]>","sha":"dd765ab84c98ddd823dc48152c36bc7ee6a1bd81"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/173430","number":173430,"mergeCommit":{"message":"Allow
members of @security-solution-test-skippers to use `/skip` comments on
failed test issues (#173430)\n\n## Summary\r\n\r\nAfter a PR is merged,
CI runs all the tests on `main`. If one fails, a\r\ngithub issue with
the label `failed-test` is created. If you comment\r\n`/skip` on the
issue then a workflow action will skip the test directly\r\non the
failing branches.\r\n\r\nThis workflow can only be used by contributors
who have `admin` access\r\nto Kibana, OR to people who are on an
allow-list of elastic teams. The\r\nlist of teams allowed is
`appex-qa`.\r\n\r\nThis PR adds another team to the
list:\r\n@elastic/security-solution-test-skippers\r\nThis team was just
created to allow a small list of Security Solution\r\ncontributors the
ability to use this workflow.\r\n\r\nAllowing a few of us access to this
workflow will allow Security to\r\ntriage our own test by writing
`/skip` comments.\r\n\r\n### Without this PR we have to:\r\n\r\nWithout
this PR we have to do a bunch of stuff to skip our own
flaky\r\ntests:\r\n\r\n1. Manually edit the files to create a PR on all
affected branches.\r\n2. Create PRs using the web app.\r\n3. Get a
second developer to approve the PR.\r\n4. Wait for CI.\r\n5. Merge the
PR.\r\n\r\nThis process is slow because of manual steps, and because CI
must run\r\nbefore we can skip the test. At a minimum we should expect
that process\r\nto take as long as CI takes (e.g. 90 minutes.) But the
PRs that are\r\nmeant to skip tests may also in turn be blocked by flaky
tests. It also\r\nrequires coordination from 2 people to make and
approve the PR.\r\n\r\n### Code
references\r\n\r\n\r\n[skip-failed-test.yml](https://github.com/elastic/kibana/blob/main/.github/workflows/skip-failed-test.yml#L25-L30)\r\nis
where the workflow is defined\r\n\r\n[Docs on the syntax
for\r\nworkflows.](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsuses)\r\n\r\n\r\n[permission-check/index.ts](https://github.com/elastic/kibana-github-actions/blob/main/permission-check/index.ts#L23-L26)\r\nis
where the permission check is implemented. Users need to have:\r\n*
either the right permission level\r\n* or be a member of the
team\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\nCo-authored-by:
Gloria Hornero
<[email protected]>","sha":"dd765ab84c98ddd823dc48152c36bc7ee6a1bd81"}}]}]
BACKPORT-->

Co-authored-by: Robert Austin <[email protected]>
  • Loading branch information
kibanamachine and Robert Austin authored Dec 19, 2023
1 parent 52989ee commit 1ec65e9
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion .github/workflows/skip-failed-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
uses: ./actions/permission-check
with:
permission: admin
teams: appex-qa
teams: appex-qa, security-solution-test-skippers
token: ${{secrets.KIBANAMACHINE_TOKEN}}

- name: Checkout kibana-operations
Expand Down

0 comments on commit 1ec65e9

Please sign in to comment.