From be26b7d7bcad0f01efc6eddd8544958dd1e7ea5a Mon Sep 17 00:00:00 2001 From: Konrad Szwarc Date: Mon, 9 Oct 2023 11:21:02 +0200 Subject: [PATCH] [EDR Workflows] Flaky CY e2e tests (#167870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pull request tackles flaky `artifacts.cy.ts` e2e test. Ran 7+ times without flakiness in CI. Added Test Burning to Defend Workflows pipeline. Changed: 1. Removed `.first()` selectors in favour of explicit element getters 2. Removed method chain 3. Moved url change from `before` to test body. 4. In `beforeAll` now we first fetch current revision number, then run `Promise.all` on `removeAllArtifacts` which resolves with `true/false` based on whether they actually removed an artifact. Its being counted then and with this number we know what next revision should be (revision fetched before removal + times removal was successful). We query API recursively until revisions match and only then we can be sure that displayed in the UI revision is up to date. This fixes the main reason of the flakiness of this test and makes it a pure test that should always yield the same result no matter the number of runs. --------- Co-authored-by: Patryk Kopyciński --- .buildkite/pipelines/pull_request/base.yml | 22 +++++ .../steps/functional/defend_workflows_burn.sh | 17 ++++ .../defend_workflows_serverless_burn.sh | 17 ++++ x-pack/plugins/security_solution/package.json | 1 + .../cypress/e2e/artifacts/artifacts.cy.ts | 95 +++++++++---------- .../management/cypress/tasks/artifacts.ts | 18 ++++ 6 files changed, 121 insertions(+), 49 deletions(-) create mode 100644 .buildkite/scripts/steps/functional/defend_workflows_burn.sh create mode 100644 .buildkite/scripts/steps/functional/defend_workflows_serverless_burn.sh diff --git a/.buildkite/pipelines/pull_request/base.yml b/.buildkite/pipelines/pull_request/base.yml index 0a03db978fa21..fd47dd6c08225 100644 --- a/.buildkite/pipelines/pull_request/base.yml +++ b/.buildkite/pipelines/pull_request/base.yml @@ -141,6 +141,17 @@ steps: - exit_status: '*' limit: 1 + - command: .buildkite/scripts/steps/functional/defend_workflows_burn.sh + label: 'Defend Workflows Cypress Tests, burning changed specs' + agents: + queue: n2-4-virt + depends_on: build + timeout_in_minutes: 60 + soft_fail: true + parallelism: 1 + retry: + automatic: false + - command: .buildkite/scripts/steps/functional/defend_workflows_serverless.sh label: 'Defend Workflows Cypress Tests on Serverless' agents: @@ -153,6 +164,17 @@ steps: - exit_status: '*' limit: 1 + - command: .buildkite/scripts/steps/functional/defend_workflows_serverless_burn.sh + label: 'Defend Workflows Cypress Tests on Serverless, burning changed specs' + agents: + queue: n2-4-virt + depends_on: build + timeout_in_minutes: 60 + soft_fail: true + parallelism: 1 + retry: + automatic: false + - command: .buildkite/scripts/steps/functional/threat_intelligence.sh label: 'Threat Intelligence Cypress Tests' agents: diff --git a/.buildkite/scripts/steps/functional/defend_workflows_burn.sh b/.buildkite/scripts/steps/functional/defend_workflows_burn.sh new file mode 100644 index 0000000000000..bc7342c93e719 --- /dev/null +++ b/.buildkite/scripts/steps/functional/defend_workflows_burn.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +set -euo pipefail + +source .buildkite/scripts/steps/functional/common.sh +source .buildkite/scripts/steps/functional/common_cypress.sh + +.buildkite/scripts/bootstrap.sh +node scripts/build_kibana_platform_plugins.js + +export JOB=kibana-defend-workflows-cypress + +buildkite-agent meta-data set "${BUILDKITE_JOB_ID}_is_test_execution_step" 'false' + +echo "--- Defend Workflows Cypress tests, burning changed specs (Chrome)" + +yarn --cwd x-pack/plugins/security_solution cypress:changed-specs-only diff --git a/.buildkite/scripts/steps/functional/defend_workflows_serverless_burn.sh b/.buildkite/scripts/steps/functional/defend_workflows_serverless_burn.sh new file mode 100644 index 0000000000000..5981c5bd210cc --- /dev/null +++ b/.buildkite/scripts/steps/functional/defend_workflows_serverless_burn.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +set -euo pipefail + +source .buildkite/scripts/steps/functional/common.sh +source .buildkite/scripts/steps/functional/common_cypress.sh + +.buildkite/scripts/bootstrap.sh +node scripts/build_kibana_platform_plugins.js + +export JOB=kibana-defend-workflows-serverless-cypress + +buildkite-agent meta-data set "${BUILDKITE_JOB_ID}_is_test_execution_step" 'false' + +echo "--- Defend Workflows Cypress tests, burning changed specs (Chrome)" + +yarn --cwd x-pack/plugins/security_solution cypress:dw:serverless:changed-specs-only diff --git a/x-pack/plugins/security_solution/package.json b/x-pack/plugins/security_solution/package.json index 5e2bc285bca9a..a13046b0bc59f 100644 --- a/x-pack/plugins/security_solution/package.json +++ b/x-pack/plugins/security_solution/package.json @@ -16,6 +16,7 @@ "cypress:dw:serverless": "NODE_OPTIONS=--openssl-legacy-provider node ./scripts/start_cypress_parallel --config-file ./public/management/cypress/cypress_serverless.config.ts --ftr-config-file ../../test/defend_workflows_cypress/serverless_config", "cypress:dw:serverless:open": "yarn cypress:dw:serverless open", "cypress:dw:serverless:run": "yarn cypress:dw:serverless run", + "cypress:dw:serverless:changed-specs-only": "yarn cypress:dw:serverless run --changed-specs-only --env burn=2", "cypress:dw:endpoint": "echo '\n** WARNING **: Run script `cypress:dw:endpoint` no longer valid! Use `cypress:dw` instead\n'", "cypress:dw:endpoint:run": "echo '\n** WARNING **: Run script `cypress:dw:endpoint:run` no longer valid! Use `cypress:dw:run` instead\n'", "cypress:dw:endpoint:open": "echo '\n** WARNING **: Run script `cypress:dw:endpoint:open` no longer valid! Use `cypress:dw:open` instead\n'", diff --git a/x-pack/plugins/security_solution/public/management/cypress/e2e/artifacts/artifacts.cy.ts b/x-pack/plugins/security_solution/public/management/cypress/e2e/artifacts/artifacts.cy.ts index 6f9ce4f863ff7..012456d92db9e 100644 --- a/x-pack/plugins/security_solution/public/management/cypress/e2e/artifacts/artifacts.cy.ts +++ b/x-pack/plugins/security_solution/public/management/cypress/e2e/artifacts/artifacts.cy.ts @@ -6,14 +6,14 @@ */ import { recurse } from 'cypress-recurse'; +import { performUserActions } from '../../tasks/perform_user_actions'; import type { IndexedFleetEndpointPolicyResponse } from '../../../../../common/endpoint/data_loaders/index_fleet_endpoint_policy'; import { HOST_METADATA_LIST_ROUTE } from '../../../../../common/endpoint/constants'; import type { MetadataListResponse, PolicyData } from '../../../../../common/endpoint/types'; import { APP_ENDPOINTS_PATH } from '../../../../../common/constants'; import { getArtifactsListTestsData } from '../../fixtures/artifacts_page'; -import { removeAllArtifacts } from '../../tasks/artifacts'; +import { removeAllArtifacts, removeAllArtifactsPromise } from '../../tasks/artifacts'; import { login } from '../../tasks/login'; -import { performUserActions } from '../../tasks/perform_user_actions'; import { request, loadPage } from '../../tasks/common'; import { createAgentPolicyTask, @@ -36,8 +36,7 @@ const yieldAppliedEndpointRevision = (): Cypress.Chainable => const parseRevNumber = (revString: string) => Number(revString.match(/\d+/)?.[0]); -// Failing: See https://github.com/elastic/kibana/issues/168272 -describe.skip('Artifact pages', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => { +describe('Artifact pages', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => { let indexedPolicy: IndexedFleetEndpointPolicyResponse; let policy: PolicyData; let createdHost: CreateAndEnrollEndpointHostResponse; @@ -56,32 +55,26 @@ describe.skip('Artifact pages', { tags: ['@ess', '@serverless', '@brokenInServer }); }) ); - - login(); - removeAllArtifacts(); - - // wait for ManifestManager to pick up artifact changes that happened either here - // or in a previous test suite `after` - // eslint-disable-next-line cypress/no-unnecessary-waiting - cy.wait(6000); // packagerTaskInterval + 1s - - yieldEndpointPolicyRevision().then((actualEndpointPolicyRevision) => { - const hasReachedActualRevision = (revision: number) => - revision === actualEndpointPolicyRevision; - - // need to wait until revision is bumped to ensure test success - recurse(yieldAppliedEndpointRevision, hasReachedActualRevision, { delay: 1500 }); - }); }); beforeEach(() => { - login(); - loadPage(APP_ENDPOINTS_PATH); + // We need to wait until revision is bumped to ensure test success. + // Fetch current revision, count how many artifacts are deleted, and wait until revision is bumped by that amount. + yieldEndpointPolicyRevision().then((actualEndpointPolicyRevision) => { + login(); + removeAllArtifactsPromise().then((removedArtifactCount) => { + const hasReachedActualRevision = (revision: number) => + revision === actualEndpointPolicyRevision + removedArtifactCount; + recurse(yieldAppliedEndpointRevision, hasReachedActualRevision, { + delay: 2000, + timeout: 90000, + }); + }); + }); }); after(() => { removeAllArtifacts(); - if (createdHost) { cy.task('destroyEndpointHost', createdHost); } @@ -98,32 +91,36 @@ describe.skip('Artifact pages', { tags: ['@ess', '@serverless', '@brokenInServer for (const testData of getArtifactsListTestsData()) { describe(`${testData.title}`, () => { it(`should update Endpoint Policy on Endpoint when adding ${testData.artifactName}`, () => { - cy.getByTestSubj('policyListRevNo') - .first() - .invoke('text') - .then(parseRevNumber) - .then((initialRevisionNumber) => { - loadPage(`/app/security/administration/${testData.urlPath}`); - - cy.getByTestSubj(`${testData.pagePrefix}-emptyState-addButton`).click(); - performUserActions(testData.create.formActions); - cy.getByTestSubj(`${testData.pagePrefix}-flyout-submitButton`).click(); - - // Check new artifact is in the list - for (const checkResult of testData.create.checkResults) { - cy.getByTestSubj(checkResult.selector).should('have.text', checkResult.value); - } - - loadPage(APP_ENDPOINTS_PATH); - - // depends on the 10s auto refresh - cy.getByTestSubj('policyListRevNo') - .first() - .should(($div) => { - const revisionNumber = parseRevNumber($div.text()); - expect(revisionNumber).to.eq(initialRevisionNumber + 1); - }); - }); + loadPage(APP_ENDPOINTS_PATH); + + cy.get(`[data-endpoint-id="${createdHost.agentId}"]`).within(() => { + cy.getByTestSubj('policyListRevNo') + .invoke('text') + .then((text) => { + cy.wrap(parseRevNumber(text)).as('initialRevisionNumber'); + }); + }); + + loadPage(`/app/security/administration/${testData.urlPath}`); + + cy.getByTestSubj(`${testData.pagePrefix}-emptyState-addButton`).click(); + performUserActions(testData.create.formActions); + cy.getByTestSubj(`${testData.pagePrefix}-flyout-submitButton`).click(); + + for (const checkResult of testData.create.checkResults) { + cy.getByTestSubj(checkResult.selector).should('have.text', checkResult.value); + } + + loadPage(APP_ENDPOINTS_PATH); + (cy.get('@initialRevisionNumber') as unknown as Promise).then( + (initialRevisionNumber) => { + cy.get(`[data-endpoint-id="${createdHost.agentId}"]`).within(() => { + cy.getByTestSubj('policyListRevNo') + .invoke('text') + .should('include', initialRevisionNumber + 1); + }); + } + ); }); }); } diff --git a/x-pack/plugins/security_solution/public/management/cypress/tasks/artifacts.ts b/x-pack/plugins/security_solution/public/management/cypress/tasks/artifacts.ts index 8b039bdd8c708..62b083cf21889 100644 --- a/x-pack/plugins/security_solution/public/management/cypress/tasks/artifacts.ts +++ b/x-pack/plugins/security_solution/public/management/cypress/tasks/artifacts.ts @@ -26,6 +26,11 @@ export const removeAllArtifacts = () => { } }; +export const removeAllArtifactsPromise = () => + Cypress.Promise.all(ENDPOINT_ARTIFACT_LIST_IDS.map(removeExceptionsListPromise)).then( + (result) => result.filter(Boolean).length + ); + export const removeExceptionsList = (listId: string) => { request({ method: 'DELETE', @@ -36,6 +41,19 @@ export const removeExceptionsList = (listId: string) => { }); }; +const removeExceptionsListPromise = (listId: string) => { + return new Cypress.Promise((resolve) => { + request({ + method: 'DELETE', + url: `${EXCEPTION_LIST_URL}?list_id=${listId}&namespace_type=agnostic`, + failOnStatusCode: false, + }).then(({ status }) => { + expect(status).to.be.oneOf([200, 404]); // should either be success or not found + resolve(status === 200); + }); + }); +}; + const ENDPOINT_ARTIFACT_LIST_TYPES = { [ENDPOINT_ARTIFACT_LISTS.trustedApps.id]: ExceptionListTypeEnum.ENDPOINT, [ENDPOINT_ARTIFACT_LISTS.eventFilters.id]: ExceptionListTypeEnum.ENDPOINT_EVENTS,