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] Reusing Cypress tests POC2 #162058

Closed
wants to merge 8 commits into from
Closed
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 x-pack/plugins/security_solution/cypress/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ export default defineCypressConfig({
e2e: {
experimentalRunAllSpecs: true,
experimentalMemoryManagement: true,
specPattern: ['./e2e/**/*.cy.ts', '../../../../../../security_solution/cypress/e2e/**/*.cy.ts'],
Copy link
Contributor

Choose a reason for hiding this comment

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

../../../../../../security_solution/cypress/e2e/**/*.cy.ts 🤔

Is it a mistake? What folder does it point to? If it's a legit folder for some reason, could you please add a comment explaining why it's needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

No it is not. In this POC we are splitting the tests between 2 different folders, the one for just serverless tests (the first path you see on the array) and the folder we usually use for Cypress tests (the second path of the array). As in this POC the execution of the tests for the serverless environment happens from the test_serverless folder, we need to tell cypress were are placed the tests we want to execute.

As seems that we finally we'll have all the tests on the same folder, that is not going to be necessary.

},
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from '../../../tasks/alerts';
import { USER_COLUMN } from '../../../screens/alerts';

describe('user details flyout', () => {
describe('user details flyout', { tags: '@serverless' }, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would make sense to use constants for the tags to avoid typos.

beforeEach(() => {
cleanKibana();
login();
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/security_solution/cypress/tasks/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,10 @@ export const deleteDataView = (dataSource: string) => {
export const scrollToBottom = () => cy.scrollTo('bottom');

export const waitForPageToBeLoaded = () => {
cy.get(LOADING_INDICATOR_HIDDEN).should('exist');
cy.get(LOADING_INDICATOR).should('not.exist');
if (Cypress.env('grepTags') !== '@serverless') {
cy.get(LOADING_INDICATOR_HIDDEN).should('exist');
cy.get(LOADING_INDICATOR).should('not.exist');
}
Comment on lines +275 to +278
Copy link
Contributor

Choose a reason for hiding this comment

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

Cypress.env('grepTags') !== '@serverless' would be great to extract to a reusable function with a clear name -- not sure I'm able to immediately parse what this means by looking at the code

};

export const waitForWelcomePanelToBeLoaded = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,32 @@
import { defineCypressConfig } from '@kbn/cypress-config';

export default defineCypressConfig({
defaultCommandTimeout: 60000,
execTimeout: 60000,
pageLoadTimeout: 60000,
defaultCommandTimeout: 150000,
env: {
grepFilterSpecs: true,
},
execTimeout: 150000,
pageLoadTimeout: 150000,
Comment on lines +11 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the updated timeouts based on something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, I was just playing a bit with the code.

responseTimeout: 60000,
screenshotsFolder: '../../../../../../target/kibana-security-solution/cypress/screenshots',
trashAssetsBeforeRuns: false,
video: false,
viewportHeight: 946,
viewportWidth: 1680,
numTestsKeptInMemory: 10,
e2e: {
setupNodeEvents(on, config) {
// eslint-disable-next-line @typescript-eslint/no-var-requires
require('@cypress/grep/src/plugin')(config);
return config;
},
baseUrl: 'http://localhost:5620/app/security/get_started',
experimentalRunAllSpecs: true,
experimentalMemoryManagement: true,
supportFile: './support/e2e.js',
specPattern: './e2e/**/*.cy.ts',
specPattern: [
'./e2e/**/*.cy.ts',
'../../../../../plugins/security_solution/cypress/e2e/**/*.cy.ts',
],
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { LEFT_NAVIGATION } from '../screens/landing_page';
import { navigatesToLandingPage } from '../tasks/navigation';

describe('Serverless', () => {
describe('Serverless', { tags: '@serverless' }, () => {
it('Should navigate to the landing page', () => {
navigatesToLandingPage();
cy.get(LEFT_NAVIGATION).should('exist');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"private": true,
"license": "Elastic License 2.0",
"scripts": {
"cypress:open": "../../../../../../node_modules/.bin/cypress open --config-file ./cypress.config.ts",
"cypress:run": "../../../../../../node_modules/.bin/cypress run --browser chrome --config-file ./cypress.config.ts",
"cypress:open": "../../../../../../node_modules/.bin/cypress open --config-file ./cypress.config.ts --env grepTags=@serverless",
"cypress:run": "../../../../../../node_modules/.bin/cypress run --browser chrome --config-file ./cypress.config.ts --env grepTags=@serverless",
Comment on lines +8 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be specified in the config itself, or there's a reason for doing it via the CLI arguments?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the tags can be configured on the cypress config file. The only thing we need to take into consideration is that if we want to follow that direction we would need to create a specific config file per execution type, other than that, not any specific reason for having them on the CLI.

"cypress:serverless:open": "node ../../../../../../scripts/functional_tests --config ./config_visual.ts",
"cypress:serverless:run": "node ../../../../../../scripts/functional_tests --config ./config_headless.ts"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { resolve } from 'path';
import { withProcRunner } from '@kbn/dev-proc-runner';
import Url from 'url';

import { FtrProviderContext } from '../../../ftr_provider_context';

Expand All @@ -17,6 +18,10 @@ export async function SecuritySolutionCypressTestRunner(
command: string
) {
const log = getService('log');
const config = getService('config');
const esArchiver = getService('esArchiver');

await esArchiver.load('x-pack/test/security_solution_cypress/es_archives/auditbeat');
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for doing that for all tests by default? Just to have some source events for the app so it doesn't show any empty screens?

I think we should be moving away from using generic es archives like this one. If we have tests that specifically depend on this archive I'd try to refactor them before including them in the serverless suite.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. In order to be able to use our application we need to have data on the instance, if not, a splash screen is going to be displayed telling you to ingest data first. Most of our tests relies on that data, we did it that way to avoid loading it per each test with the aim of improving the execution time of the tests.


await withProcRunner(log, async (procs) => {
await procs.run('cypress', {
Expand All @@ -25,6 +30,11 @@ export async function SecuritySolutionCypressTestRunner(
cwd: resolve(__dirname),
env: {
...process.env,
FORCE_COLOR: '1',
CYPRESS_BASE_URL: Url.format(config.get('servers.kibana')),
CYPRESS_ELASTICSEARCH_URL: Url.format(config.get('servers.elasticsearch')),
CYPRESS_ELASTICSEARCH_USERNAME: config.get('servers.elasticsearch.username'),
CYPRESS_ELASTICSEARCH_PASSWORD: config.get('servers.elasticsearch.password'),
},
wait: true,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
...svlSharedConfig.get('kbnTestServer.serverArgs'),
'--csp.strict=false',
'--csp.warnLegacyBrowsers=false',
'--xpack.encryptedSavedObjects.encryptionKey="abcdefghijklmnopqrstuvwxyz123456"',
'--serverless=security',
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,57 @@
//
// -- This is will overwrite an existing command --
// Cypress.Commands.overwrite("visit", (originalFn, url, options) => { ... })

import 'cypress-file-upload';
import 'cypress-recurse/commands';

Cypress.Commands.add(
'attachFile',
{
prevSubject: 'element',
},
(input, fileName, fileType = 'text/plain') => {
cy.fixture(fileName).then((content) => {
const blob = Cypress.Blob.base64StringToBlob(btoa(content), fileType);
const testFile = new File([blob], fileName, { type: fileType });
const dataTransfer = new DataTransfer();

dataTransfer.items.add(testFile);
input[0].files = dataTransfer.files;
return input;
});
}
);

const waitUntil = (subject, fn, options = {}) => {
const { interval = 200, timeout = 5000 } = options;
let attempts = Math.floor(timeout / interval);

const completeOrRetry = (result) => {
if (result) {
return result;
}
if (attempts < 1) {
throw new Error(`Timed out while retrying, last result was: {${result}}`);
}
cy.wait(interval, { log: false }).then(() => {
attempts--;
// eslint-disable-next-line no-use-before-define
return evaluate();
});
};

const evaluate = () => {
const result = fn(subject);

if (result && result.then) {
return result.then(completeOrRetry);
} else {
return completeOrRetry(result);
}
};

return evaluate();
};

Cypress.Commands.add('waitUntil', { prevSubject: 'optional' }, waitUntil);
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
// Import commands.js using ES2015 syntax:
import './commands';
import 'cypress-real-events/support';
import registerCypressGrep from '@cypress/grep';

registerCypressGrep();

Cypress.on('uncaught:exception', () => {
return false;
Expand Down