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] Skip all Detection & Response Cypress tests in Serverless #165966

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @MadameSheema @patrykkopycinski @oatkiller @maximpn

See the PR description for motivation.


## 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".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add @skipInEss? If we need to skip for ESS right now we have to skip for all right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yctercero This is a good point. I think we will need this tag eventually. And yes, if we need to skip for ESS right now we have to skip for all.

I'm not sure about adding it now because I think that "now we have to skip for all" is exactly what we should be doing now. Until we basically eliminate flakiness in our tests, we should treat a flaky test in ESS as being in general not yet fixed. Then, when we can say something like "ok, the rule creation Cypress tests have been all fixed and not flaky anymore in ESS and Serverless", then we could start skipping them in either environment if flakiness ever occurs again.


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