Skip to content

Commit

Permalink
[EDR Workflows] Flaky CY e2e tests (elastic#167870)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
szwarckonrad and patrykkopycinski authored Oct 9, 2023
1 parent 59af9b3 commit be26b7d
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 49 deletions.
22 changes: 22 additions & 0 deletions .buildkite/pipelines/pull_request/base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
17 changes: 17 additions & 0 deletions .buildkite/scripts/steps/functional/defend_workflows_burn.sh
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions x-pack/plugins/security_solution/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -36,8 +36,7 @@ const yieldAppliedEndpointRevision = (): Cypress.Chainable<number> =>

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;
Expand All @@ -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);
}
Expand All @@ -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<number>).then(
(initialRevisionNumber) => {
cy.get(`[data-endpoint-id="${createdHost.agentId}"]`).within(() => {
cy.getByTestSubj('policyListRevNo')
.invoke('text')
.should('include', initialRevisionNumber + 1);
});
}
);
});
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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,
Expand Down

0 comments on commit be26b7d

Please sign in to comment.