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.

Sign up for GitHub

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][Detection Engine] Getting rid off before hook if is not loading an archive #178876

Merged
merged 3 commits into from
Mar 19, 2024
Merged

[Security Solution][Detection Engine] Getting rid off before hook if is not loading an archive #178876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da3f3c6
Getting rid off before hook if is not loading an archive
MadameSheema Mar 18, 2024
ca2b9cc
Merge branch 'main' into detection-engine-before-hook
MadameSheema Mar 18, 2024
2815e78
Merge branch 'main' into detection-engine-before-hook
MadameSheema Mar 19, 2024
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
14 changes: 10 additions & 4 deletions ...response/detection_engine/exceptions/rule_details_flow/add_edit_exception_data_view.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ import {
waitForTheRuleToBeExecuted,
} from '../../../../../tasks/rule_details';

import { postDataView, deleteAlertsAndRules } from '../../../../../tasks/api_calls/common';
import {
postDataView,
deleteAlertsAndRules,
deleteDataView,
} from '../../../../../tasks/api_calls/common';
import {
NO_EXCEPTIONS_EXIST_PROMPT,
EXCEPTION_ITEM_VIEWER_CONTAINER,
Expand All @@ -42,6 +46,8 @@ import {
} from '../../../../../screens/exceptions';
import { waitForAlertsToPopulate } from '../../../../../tasks/create_new_rule';

const DATAVIEW = 'exceptions-*';

describe(
'Add exception using data views from rule details',
{ tags: ['@ess', '@serverless'] },
Expand All @@ -51,21 +57,21 @@ describe(

before(() => {
cy.task('esArchiverLoad', { archiveName: 'exceptions' });
login();
postDataView('exceptions-*');
});

after(() => {
cy.task('esArchiverUnload', { archiveName: 'exceptions' });
});

beforeEach(() => {
deleteDataView(DATAVIEW);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like DATAVIEW will still exist after the last test is run. Should there be an additional afterEach with a deleteDataView(DATAVIEW)?

postDataView(DATAVIEW);
login();
deleteAlertsAndRules();
createRule(
getNewRule({
query: 'agent.name:*',
data_view_id: 'exceptions-*',
data_view_id: DATAVIEW,
rule_id: 'rule_testing',
enabled: true,
})
Expand Down
14 changes: 9 additions & 5 deletions ...xceptions/shared_exception_lists_management/shared_exception_list_page/manage_lists.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ import {
import { createExceptionList } from '../../../../../../tasks/api_calls/exceptions';

import { TOASTER } from '../../../../../../screens/alerts_detection_rules';
import {
deleteAlertsAndRules,
deleteExceptionLists,
} from '../../../../../../tasks/api_calls/common';

const EXCEPTION_LIST_NAME = 'My test list';
const EXCEPTION_LIST_TO_DUPLICATE_NAME = 'A test list 2';
Expand All @@ -56,7 +60,9 @@ describe(
{ tags: ['@ess', '@serverless'] },
() => {
describe('Create/Export/Delete List', () => {
before(() => {
beforeEach(() => {
deleteAlertsAndRules();
deleteExceptionLists();
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing the same pattern as above here: after the last test, any rules and exception lists created will still be present for the next test. There should probably also be an afterEach added with these commands.

Trying to set up a pristine state in the beforeEach is a good, defensive practice, but cleaning up your own test's data is much more important. I suspect that's been the cause of some flakiness in our suite.

createRule(getNewRule({ name: 'Another rule' }));

// Create exception list associated with a rule
Expand All @@ -79,9 +85,7 @@ describe(
createExceptionList(getExceptionList1(), getExceptionList1().list_id).then((response) => {
exceptionListResponse = response;
});
});

beforeEach(() => {
login();
visit(EXCEPTIONS_URL);
waitForExceptionsTableToBeLoaded();
Expand Down Expand Up @@ -124,13 +128,13 @@ describe(
it('Delete exception list without rule reference', () => {
// Using cy.contains because we do not care about the exact text,
// just checking number of lists shown
cy.contains(EXCEPTIONS_TABLE_SHOWING_LISTS, '4');
cy.contains(EXCEPTIONS_TABLE_SHOWING_LISTS, '3');

deleteExceptionListWithoutRuleReferenceByListId(getExceptionList1().list_id);

// Using cy.contains because we do not care about the exact text,
// just checking number of lists shown
cy.contains(EXCEPTIONS_TABLE_SHOWING_LISTS, '3');
cy.contains(EXCEPTIONS_TABLE_SHOWING_LISTS, '2');
});

it('Deletes exception list with rule reference', () => {
Expand Down
7 changes: 2 additions & 5 deletions ...ress/cypress/e2e/detection_response/detection_engine/sourcerer/create_runtime_field.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,10 @@ const alertRunTimeField = 'field.name.alert.page';
const timelineRuntimeField = 'field.name.timeline';

describe('Create DataView runtime field', { tags: ['@ess', '@serverless'] }, () => {
before(() => {
deleteRuntimeField('security-solution-default', alertRunTimeField);
deleteRuntimeField('security-solution-default', timelineRuntimeField);
});

beforeEach(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, no afterEach for cleanup :(

login();
deleteRuntimeField('security-solution-default', alertRunTimeField);
deleteRuntimeField('security-solution-default', timelineRuntimeField);
});

it('adds field to alert table', () => {
Expand Down
2 changes: 1 addition & 1 deletion ...ess/cypress/e2e/detection_response/detection_engine/sourcerer/sourcerer_permissions.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { login } from '../../../../tasks/login';
const dataViews = ['auditbeat-*,fakebeat-*', 'auditbeat-*,*beat*,siem-read*,.kibana*,fakebeat-*'];

describe('Sourcerer permissions', { tags: ['@ess', '@brokenInServerless'] }, () => {
before(() => {
beforeEach(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

😢

dataViews.forEach((dataView: string) => postDataView(dataView));
});

Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/security_solution_cypress/cypress/tasks/api_calls/exceptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const updateExceptionListItem = (
failOnStatusCode: false,
});

export const deleteExceptionList = (listId: string, namespaceType: string) =>
export const deleteExceptionList = (listId: string, namespaceType: string = 'default') =>
rootRequest({
method: 'DELETE',
url: `/api/exception_lists?list_id=${listId}&namespace_type=${namespaceType}`,
Expand Down