[Cases] Fixing flaky tests relating to bulk creation of attachments and alerts (#155653)

This PR fixes some flaky tests. I believe there were two issues.
1. There were a few places we were only waiting for 1 alert to be
created but we were expecting 2 alerts to be there which resulted in
undefined when accessing the second alert and produced the _can't find
_id_
2. The bulk create attachments code doesn't _**really**_ guarantee
creating the attachments in the order they are listed within the
request. This is because we do a `Promise.all` and quickly loop over the
attachments to create the timestamp. I believe this was creating
attachments with the same `created_at` value which means that they could
be returned in the `case.comments` in a different order. The solution
here is to compare the arrays ignoring the ordering.

Fixes: #154469 #154558 #154676 #154751 #154859 #154849 #154858 #154850
#154875 #155067 #155062 #155046 #155160 #155235 #155555 #155557 #155569,
#155703, #155666

Flaky test run:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2164
🟢
This commit is contained in:
Jonathan Buttner 2023-04-25 12:00:45 -04:00 committed by GitHub
parent b92f5e94fd
commit e933ad50fe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 86 additions and 63 deletions

View file

@ -31,12 +31,13 @@ import { postCaseReq } from './mock';
export const createSecuritySolutionAlerts = async (
supertest: SuperTest.SuperTest<SuperTest.Test>,
log: ToolingLog
log: ToolingLog,
numberOfSignals: number = 1
): Promise<estypes.SearchResponse<DetectionAlert & RiskEnrichmentFields>> => {
const rule = getRuleForSignalTesting(['auditbeat-*']);
const { id } = await createRule(supertest, log, rule);
await waitForRuleSuccess({ supertest, log, id });
await waitForSignalsToBePresent(supertest, log, 1, [id]);
await waitForSignalsToBePresent(supertest, log, numberOfSignals, [id]);
const signals = await getSignalsByIds(supertest, log, [id]);
return signals;

View file

@ -128,7 +128,7 @@ export const createCaseAndBulkCreateAttachments = async ({
export const getAttachments = (numberOfAttachments: number): BulkCreateCommentRequest => {
return [...Array(numberOfAttachments)].map((_, index) => {
if (index % 0) {
if (index % 10 === 0) {
return {
type: CommentType.user,
comment: `Test ${index + 1}`,

View file

@ -244,7 +244,7 @@ export default ({ getService }: FtrProviderContext): void => {
beforeEach(async () => {
await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts');
await createSignalsIndex(supertest, log);
const signals = await createSecuritySolutionAlerts(supertest, log);
const signals = await createSecuritySolutionAlerts(supertest, log, 2);
alerts = [signals.hits.hits[0], signals.hits.hits[1]];
});

View file

@ -126,7 +126,7 @@ export default ({ getService }: FtrProviderContext): void => {
beforeEach(async () => {
await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts');
await createSignalsIndex(supertest, log);
const signals = await createSecuritySolutionAlerts(supertest, log);
const signals = await createSecuritySolutionAlerts(supertest, log, 2);
alerts = [signals.hits.hits[0], signals.hits.hits[1]];
});

View file

@ -128,7 +128,7 @@ export default ({ getService }: FtrProviderContext): void => {
beforeEach(async () => {
await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts');
await createSignalsIndex(supertest, log);
const signals = await createSecuritySolutionAlerts(supertest, log);
const signals = await createSecuritySolutionAlerts(supertest, log, 2);
alerts = [signals.hits.hits[0], signals.hits.hits[1]];
});

View file

@ -13,7 +13,6 @@ import {
BulkCreateCommentRequest,
CaseResponse,
CaseStatuses,
CommentRequestAlertType,
CommentRequestExternalReferenceSOType,
CommentType,
} from '@kbn/cases-plugin/common/api';
@ -70,6 +69,7 @@ import {
} from '../../../../common/lib/alerts';
import { User } from '../../../../common/lib/authentication/types';
import { SECURITY_SOLUTION_FILE_KIND } from '../../../../common/lib/constants';
import { arraysToEqual } from '../../../../common/lib/validation';
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
@ -79,25 +79,35 @@ export default ({ getService }: FtrProviderContext): void => {
const es = getService('es');
const log = getService('log');
const validateComments = (
const validateCommentsIgnoringOrder = (
comments: CaseResponse['comments'],
attachments: BulkCreateCommentRequest
) => {
comments?.forEach((attachment, index) => {
const comment = removeServerGeneratedPropertiesFromSavedObject(attachment);
expect(comments?.length).to.eql(attachments.length);
expect(comment).to.eql({
...attachments[index],
const commentsWithoutGeneratedProps = [];
const attachmentsWithoutGeneratedProps = [];
for (const comment of comments!) {
commentsWithoutGeneratedProps.push(removeServerGeneratedPropertiesFromSavedObject(comment));
}
for (const attachment of attachments) {
attachmentsWithoutGeneratedProps.push({
...attachment,
created_by: defaultUser,
pushed_at: null,
pushed_by: null,
updated_by: null,
});
});
}
expect(arraysToEqual(commentsWithoutGeneratedProps, attachmentsWithoutGeneratedProps)).to.be(
true
);
};
// FAILING: https://github.com/elastic/kibana/issues/154859
describe.skip('bulk_create_attachments', () => {
describe('bulk_create_attachments', () => {
afterEach(async () => {
await deleteAllCaseItems(es);
});
@ -118,7 +128,7 @@ export default ({ getService }: FtrProviderContext): void => {
numberOfAttachments: 1,
});
validateComments(theCase.comments, attachments);
validateCommentsIgnoringOrder(theCase.comments, attachments);
});
it('should bulk create multiple attachments', async () => {
@ -129,7 +139,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(theCase.totalComment).to.eql(attachments.length);
expect(theCase.updated_by).to.eql(defaultUser);
validateComments(theCase.comments, attachments);
validateCommentsIgnoringOrder(theCase.comments, attachments);
});
it('creates the correct user action', async () => {
@ -1292,6 +1302,20 @@ export default ({ getService }: FtrProviderContext): void => {
});
it('should create a new attachment without alerts attached to the case', async () => {
const alertCommentWithId3 = {
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-1', 'test-id-2', 'test-id-3'],
index: ['test-index-1', 'test-index-2', 'test-index-3'],
};
const alertCommentOnlyId3 = {
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-3'],
index: ['test-index-3'],
};
const allAttachments = [postCommentAlertMultipleIdsReq, alertCommentOnlyId3];
const postedCase = await createCase(supertest, postCaseReq);
await bulkCreateAttachments({
@ -1304,52 +1328,71 @@ export default ({ getService }: FtrProviderContext): void => {
await bulkCreateAttachments({
supertest,
caseId: postedCase.id,
params: [
{
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-1', 'test-id-2', 'test-id-3'],
index: ['test-index-1', 'test-index-2', 'test-index-3'],
},
],
params: [alertCommentWithId3],
expectedHttpCode: 200,
});
const attachments = await getAllComments({ supertest, caseId: postedCase.id });
expect(attachments.length).to.eql(2);
const secondAttachment = attachments[1] as CommentRequestAlertType;
expect(secondAttachment.alertId).to.eql(['test-id-3']);
expect(secondAttachment.index).to.eql(['test-index-3']);
validateCommentsIgnoringOrder(attachments, allAttachments);
});
it('should create a new attachment without alerts attached to the case on the same request', async () => {
const alertCommentWithId3 = {
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-1', 'test-id-2', 'test-id-3'],
index: ['test-index-1', 'test-index-2', 'test-index-3'],
};
const alertCommentOnlyId3 = {
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-3'],
index: ['test-index-3'],
};
const allAttachments = [postCommentAlertMultipleIdsReq, alertCommentOnlyId3];
const postedCase = await createCase(supertest, postCaseReq);
await bulkCreateAttachments({
supertest,
caseId: postedCase.id,
params: [
postCommentAlertMultipleIdsReq,
{
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-1', 'test-id-2', 'test-id-3'],
index: ['test-index-1', 'test-index-2', 'test-index-3'],
},
],
params: [postCommentAlertMultipleIdsReq, alertCommentWithId3],
expectedHttpCode: 200,
});
const attachments = await getAllComments({ supertest, caseId: postedCase.id });
expect(attachments.length).to.eql(2);
const secondAttachment = attachments[1] as CommentRequestAlertType;
expect(secondAttachment.alertId).to.eql(['test-id-3']);
expect(secondAttachment.index).to.eql(['test-index-3']);
validateCommentsIgnoringOrder(attachments, allAttachments);
});
it('does not remove user comments when filtering out duplicate alerts', async () => {
const alertCommentWithId3 = {
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-1', 'test-id-2', 'test-id-3'],
index: ['test-index-1', 'test-index-2', 'test-index-3'],
};
const alertCommentOnlyId3 = {
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-3'],
index: ['test-index-3'],
};
const superComment = {
...postCommentUserReq,
comment: 'Super comment',
};
const allAttachments = [
postCommentAlertMultipleIdsReq,
alertCommentOnlyId3,
postCommentUserReq,
superComment,
];
const postedCase = await createCase(supertest, postCaseReq);
await bulkCreateAttachments({
@ -1362,35 +1405,14 @@ export default ({ getService }: FtrProviderContext): void => {
await bulkCreateAttachments({
supertest,
caseId: postedCase.id,
params: [
postCommentUserReq,
{
...postCommentAlertMultipleIdsReq,
alertId: ['test-id-1', 'test-id-2', 'test-id-3'],
index: ['test-index-1', 'test-index-2', 'test-index-3'],
},
postCommentUserReq,
],
params: [superComment, alertCommentWithId3, postCommentUserReq],
expectedHttpCode: 200,
});
const attachments = await getAllComments({ supertest, caseId: postedCase.id });
expect(attachments.length).to.eql(4);
const firstAlert = attachments[0] as CommentRequestAlertType;
const firstUserComment = attachments[1] as CommentRequestAlertType;
const secondAlert = attachments[2] as CommentRequestAlertType;
const secondUserComment = attachments[3] as CommentRequestAlertType;
expect(firstUserComment.type).to.eql('user');
expect(secondUserComment.type).to.eql('user');
expect(firstAlert.type).to.eql('alert');
expect(secondAlert.type).to.eql('alert');
expect(firstAlert.alertId).to.eql(['test-id-1', 'test-id-2']);
expect(firstAlert.index).to.eql(['test-index', 'test-index-2']);
expect(secondAlert.alertId).to.eql(['test-id-3']);
expect(secondAlert.index).to.eql(['test-index-3']);
validateCommentsIgnoringOrder(attachments, allAttachments);
});
});