Skip to content

Commit

Permalink
[Security Solution] Skip all Detection & Response Cypress tests in Se…
Browse files Browse the repository at this point in the history
…rverless (#165966)

**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:

- #161540
- #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 #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.
  • Loading branch information
banderror authored Sep 9, 2023
1 parent aa6ad19 commit 1d65e78
Show file tree
Hide file tree
Showing 69 changed files with 1,283 additions and 1,107 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -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

## Security Solution sub teams - adaptive-workload-protection
x-pack/plugins/security_solution/public/common/components/sessions_viewer @elastic/kibana-cloud-security-posture
Expand Down
81 changes: 66 additions & 15 deletions x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,14 @@ const retrieveIntegrations = (integrationsPaths: string[]) => {
export const cli = () => {
run(
async () => {
const log = new ToolingLog({
level: 'info',
writeTo: process.stdout,
});

const { argv } = yargs(process.argv.slice(2))
.coerce('spec', (arg) => (_.isArray(arg) ? [_.last(arg)] : [arg]))
.coerce('configFile', (arg) => (_.isArray(arg) ? _.last(arg) : arg))
.coerce('spec', (arg) => (_.isArray(arg) ? _.last(arg) : arg))
.coerce('env', (arg: string) =>
arg.split(',').reduce((acc, curr) => {
const [key, value] = curr.split('=');
Expand All @@ -77,22 +83,72 @@ export const cli = () => {
}, {} as Record<string, string | number>)
);

log.info(`
----------------------------------------------
Script arguments:
----------------------------------------------
${JSON.stringify(argv, null, 2)}
----------------------------------------------
`);

const isOpen = argv._[0] === 'open';
const cypressConfigFilePath = require.resolve(
`../../${_.isArray(argv.configFile) ? _.last(argv.configFile) : argv.configFile}`
) as string;

const cypressConfigFilePath = require.resolve(`../../${argv.configFile}`) as string;
const cypressConfigFile = await import(cypressConfigFilePath);

log.info(`
----------------------------------------------
Cypress config for file: ${cypressConfigFilePath}:
----------------------------------------------
${JSON.stringify(cypressConfigFile, null, 2)}
----------------------------------------------
`);

const specConfig = cypressConfigFile.e2e.specPattern;
const specArg = argv.spec;
const specPattern = specArg ?? specConfig;

log.info('Config spec pattern:', specConfig);
log.info('Arguments spec pattern:', specArg);
log.info('Resulting spec pattern:', specPattern);

// The grep function will filter Cypress specs by tags: it will include and exclude
// spec files according to the tags configuration.
const grepSpecPattern = grep({
...cypressConfigFile,
specPattern: argv.spec ?? cypressConfigFile.e2e.specPattern,
specPattern,
excludeSpecPattern: [],
}).specPattern;

let files = retrieveIntegrations(
_.isArray(grepSpecPattern)
? grepSpecPattern
: globby.sync(argv.spec ?? cypressConfigFile.e2e.specPattern)
);
log.info('Resolved spec files or pattern after grep:', grepSpecPattern);

const isGrepReturnedFilePaths = _.isArray(grepSpecPattern);
const isGrepReturnedSpecPattern = !isGrepReturnedFilePaths && grepSpecPattern === specPattern;

// IMPORTANT!
// When grep returns the same spec pattern as it gets in its arguments, we treat it as
// it couldn't find any concrete specs to execute (maybe because all of them are skipped).
// In this case, we do an early return - it's important to do that.
// If we don't return early, these specs will start executing, and Cypress will be skipping
// tests at runtime: those that should be excluded according to the tags passed in the config.
// This can take so much time that the job can fail by timeout in CI.
if (isGrepReturnedSpecPattern) {
log.info('No tests found - all tests could have been skipped via Cypress tags');
// eslint-disable-next-line no-process-exit
return process.exit(0);
}

const concreteFilePaths = isGrepReturnedFilePaths
? grepSpecPattern // use the returned concrete file paths
: globby.sync(specPattern); // convert the glob pattern to concrete file paths

let files = retrieveIntegrations(concreteFilePaths);

log.info('Resolved spec files after retrieveIntegrations:', files);

if (argv.changedSpecsOnly) {
files = (findChangedFiles('main', false) as string[]).reduce((acc, itemPath) => {
Expand All @@ -108,11 +164,6 @@ export const cli = () => {
files = files.slice(0, 3);
}

const log = new ToolingLog({
level: 'info',
writeTo: process.stdout,
});

if (!files?.length) {
log.info('No tests found');
// eslint-disable-next-line no-process-exit
Expand Down
7 changes: 6 additions & 1 deletion x-pack/test/security_solution_cypress/cypress/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@ of data for your test, [**Running the tests**](#running-the-tests) to know how t

Please, before opening a PR with the new test, please make sure that the test fails. If you never see your test fail you don’t know if your test is actually testing the right thing, or testing anything at all.

Note that we use tags in order to select which tests we want to execute: @serverless, @ess and @brokenInServerless
Note that we use tags in order to select which tests we want to execute:

- `@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".

Please, before opening a PR with a new test, make sure that the test fails. If you never see your test fail you don’t know if your test is actually testing the right thing, or testing anything at all.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default defineCypressConfig({
env: {
grepFilterSpecs: true,
grepOmitFiltered: true,
grepTags: '@serverless --@brokenInServerless',
grepTags: '@serverless --@brokenInServerless --@skipInServerless',
},
execTimeout: 150000,
pageLoadTimeout: 150000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default defineCypressConfig({
numTestsKeptInMemory: 10,
env: {
grepFilterSpecs: true,
grepTags: '@serverless --@brokenInServerless',
grepTags: '@serverless --@brokenInServerless --@skipInServerless',
},
e2e: {
experimentalRunAllSpecs: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { GET_TIMELINE_HEADER } from '../../screens/timeline';
const alertRunTimeField = 'field.name.alert.page';
const timelineRuntimeField = 'field.name.timeline';

// TODO: https://github.com/elastic/kibana/issues/161539
describe(
'Create DataView runtime field',
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ const rolesToCreate = [secReadCasesAll];
const siemDataViewTitle = 'Security Default Data View';
const dataViews = ['auditbeat-*,fakebeat-*', 'auditbeat-*,*beat*,siem-read*,.kibana*,fakebeat-*'];

describe('Sourcerer', () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('Sourcerer', { tags: ['@ess', '@serverless', '@skipInServerless'] }, () => {
before(() => {
cy.task('esArchiverResetKibana');
dataViews.forEach((dataView: string) => postDataView(dataView));
});

// TODO: https://github.com/elastic/kibana/issues/161539
describe('permissions', { tags: ['@ess', '@brokenInServerless'] }, () => {
before(() => {
createUsersAndRoles(usersToCreate, rolesToCreate);
Expand All @@ -52,6 +54,7 @@ describe('Sourcerer', () => {
});
});

// TODO: https://github.com/elastic/kibana/issues/161539
// FLAKY: https://github.com/elastic/kibana/issues/165766
describe('Default scope', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import { closeTimeline, openTimelineById } from '../../tasks/timeline';
const siemDataViewTitle = 'Security Default Data View';
const dataViews = ['auditbeat-*,fakebeat-*', 'auditbeat-*,*beat*,siem-read*,.kibana*,fakebeat-*'];

describe('Timeline scope', { tags: '@brokenInServerless' }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe.skip('Timeline scope', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
beforeEach(() => {
cy.clearLocalStorage();
login();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import { closeTimeline, openTimelineById } from '../../tasks/timeline';
const siemDataViewTitle = 'Security Default Data View';
const dataViews = ['auditbeat-*,fakebeat-*', 'auditbeat-*,*beat*,siem-read*,.kibana*,fakebeat-*'];

describe('Timeline scope', { tags: '@brokenInServerless' }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('Timeline scope', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
beforeEach(() => {
cy.clearLocalStorage();
login();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
UNSELECTED_ALERT_TAG,
} from '../../screens/alerts';

// TODO: https://github.com/elastic/kibana/issues/161539
describe('Alert tagging', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
before(() => {
cleanKibana();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '../../screens/search_bar';
import { TOASTER } from '../../screens/alerts_detection_rules';

// TODO: https://github.com/elastic/kibana/issues/161539
describe(
'Histogram legend hover actions',
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ const waitForPageTitleToBeShown = () => {
cy.get(PAGE_TITLE).should('be.visible');
};

// TODO: https://github.com/elastic/kibana/issues/161539 Does it need to run in Serverless?
describe(
'Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set',
{ tags: '@ess' },
{ tags: ['@ess', '@skipInServerless'] },
() => {
before(() => {
// First, we have to open the app on behalf of a privileged user in order to initialize it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { openJsonView, openThreatIndicatorDetails } from '../../tasks/alerts_det
import { ruleDetailsUrl } from '../../urls/navigation';
import { addsFieldsToTimeline } from '../../tasks/rule_details';

// TODO: https://github.com/elastic/kibana/issues/161539
describe('CTI Enrichment', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
before(() => {
cleanKibana();
Expand All @@ -50,6 +51,7 @@ describe('CTI Enrichment', { tags: ['@ess', '@serverless', '@brokenInServerless'
);
});

// TODO: https://github.com/elastic/kibana/issues/161539
// Skipped: https://github.com/elastic/kibana/issues/162818
it.skip('Displays enrichment matched.* fields on the timeline', () => {
const expectedFields = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { login, visit } from '../../tasks/login';

import { ALERTS_URL } from '../../urls/navigation';

// TODO: https://github.com/elastic/kibana/issues/161539
describe('Enrichment', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
before(() => {
cleanKibana();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ const waitForPageTitleToBeShown = () => {
cy.get(PAGE_TITLE).should('be.visible');
};

describe('Detections > Callouts', { tags: '@ess' }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('Detections > Callouts', { tags: ['@ess', '@skipInServerless'] }, () => {
before(() => {
// First, we have to open the app on behalf of a privileged user in order to initialize it.
// Otherwise the app will be disabled and show a "welcome"-like page.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { TIMELINE_QUERY, TIMELINE_VIEW_IN_ANALYZER } from '../../screens/timelin
import { selectAlertsHistogram } from '../../tasks/alerts';
import { createTimeline } from '../../tasks/timelines';

// TODO: https://github.com/elastic/kibana/issues/161539
describe(
'Ransomware Detection Alerts',
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { selectAlertsHistogram } from '../../tasks/alerts';
import { createTimeline } from '../../tasks/timelines';
import { cleanKibana } from '../../tasks/common';

// TODO: https://github.com/elastic/kibana/issues/161539
describe(
'Ransomware Prevention Alerts',
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ const loadPageAsReadOnlyUser = (url: string) => {
waitForPageWithoutDateRange(url, ROLES.reader);
};

// TODO: https://github.com/elastic/kibana/issues/164451 We should find a way to make this spec work in Serverless
// TODO: https://github.com/elastic/kibana/issues/161540
describe(
'Detection rules, Prebuilt Rules Installation and Update - Authorization/RBAC',
{ tags: '@ess' },
{ tags: ['@ess', '@serverless', '@skipInServerless'] },
() => {
beforeEach(() => {
login();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ import {
ruleUpdatesTabClick,
} from '../../../tasks/prebuilt_rules';

// TODO: https://github.com/elastic/kibana/issues/161540
describe(
'Detection rules, Prebuilt Rules Installation and Update - Error handling',
{ tags: '@ess' },
{ tags: ['@ess', '@serverless', '@skipInServerless'] },
() => {
beforeEach(() => {
login();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ import {
ruleUpdatesTabClick,
} from '../../../tasks/prebuilt_rules';

// TODO: https://github.com/elastic/kibana/issues/161540
describe(
'Detection rules, Prebuilt Rules Installation and Update workflow',
{ tags: ['@ess', '@brokenInServerless'] },
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
() => {
beforeEach(() => {
login();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ const rules = Array.from(Array(5)).map((_, i) => {
});
});

describe('Prebuilt rules', { tags: ['@ess', '@serverless'] }, () => {
// TODO: https://github.com/elastic/kibana/issues/161540
describe('Prebuilt rules', { tags: ['@ess', '@serverless', '@skipInServerless'] }, () => {
before(() => {
cleanKibana();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ const RULE_1 = createRuleAssetSavedObject({
rule_id: 'rule_1',
});

// TODO: https://github.com/elastic/kibana/issues/161540
describe(
'Detection rules, Prebuilt Rules Installation and Update Notifications',
{ tags: ['@ess', '@brokenInServerless'] },
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
() => {
beforeEach(() => {
login();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ import { login, visit } from '../../../tasks/login';

import { RULE_CREATION } from '../../../urls/navigation';

// TODO: https://github.com/elastic/kibana/issues/161539
describe(
'Rule actions during detection rule creation',
{ tags: ['@ess', '@brokenInServerless'] },
{ tags: ['@ess', '@serverless', '@brokenInServerless'] },
() => {
const indexConnector = getIndexConnector();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ import {
import { enablesRule, getDetails, waitForTheRuleToBeExecuted } from '../../../tasks/rule_details';
import { ruleDetailsUrl, ruleEditUrl, RULE_CREATION } from '../../../urls/navigation';

describe('Custom query rules', { tags: ['@ess', '@brokenInServerless'] }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('Custom query rules', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
beforeEach(() => {
deleteAlertsAndRules();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ import { getDetails, waitForTheRuleToBeExecuted } from '../../../tasks/rule_deta

import { RULE_CREATION } from '../../../urls/navigation';

describe('Custom query rules', { tags: ['@ess', '@brokenInServerless'] }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('Custom query rules', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
describe('Custom detection rules creation with data views', () => {
const rule = getDataViewRule();
const expectedUrls = rule.references?.join('');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ const savedQueryName = 'custom saved query';
const savedQueryQuery = 'process.name: test';
const savedQueryFilterKey = 'testAgent.value';

describe('Custom saved_query rules', { tags: ['@ess', '@brokenInServerless'] }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('Saved query rules', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
before(() => {
cleanKibana();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ import { login, visit } from '../../../tasks/login';

import { RULE_CREATION } from '../../../urls/navigation';

describe('EQL rules', { tags: ['@ess', '@brokenInServerless'] }, () => {
// TODO: https://github.com/elastic/kibana/issues/161539
describe('EQL rules', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
before(() => {
cleanKibana();
});
Expand Down
Loading

0 comments on commit 1d65e78

Please sign in to comment.