From 7a1c87845a6851d8e54a49bc79e05731af79ad1f Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 3 Dec 2020 17:05:31 +0000 Subject: [PATCH] [Alerting][Event Log] ensures we wait for the right number of events in test (#84189) (#84912) Keeps the exact same assertions, but ensures the retry loop waits for them to complete so we don't assert too soon. --- .../common/lib/get_event_log.ts | 53 +++++++++++++++---- .../tests/actions/execute.ts | 4 +- .../tests/alerting/alerts.ts | 4 +- .../tests/alerting/event_log.ts | 2 +- .../spaces_only/tests/actions/execute.ts | 4 +- .../spaces_only/tests/alerting/event_log.ts | 28 ++++------ .../alerting/get_alert_instance_summary.ts | 2 +- 7 files changed, 59 insertions(+), 38 deletions(-) diff --git a/x-pack/test/alerting_api_integration/common/lib/get_event_log.ts b/x-pack/test/alerting_api_integration/common/lib/get_event_log.ts index aebcd854514b2..6336d834c3943 100644 --- a/x-pack/test/alerting_api_integration/common/lib/get_event_log.ts +++ b/x-pack/test/alerting_api_integration/common/lib/get_event_log.ts @@ -8,13 +8,26 @@ import { IValidatedEvent } from '../../../../plugins/event_log/server'; import { getUrlPrefix } from '.'; import { FtrProviderContext } from '../ftr_provider_context'; +interface GreaterThanEqualCondition { + gte: number; +} +interface EqualCondition { + equal: number; +} + +function isEqualConsition( + condition: GreaterThanEqualCondition | EqualCondition +): condition is EqualCondition { + return Number.isInteger((condition as EqualCondition).equal); +} + interface GetEventLogParams { getService: FtrProviderContext['getService']; spaceId: string; type: string; id: string; provider: string; - actions: string[]; + actions: Map; } // Return event log entries given the specified parameters; for the `actions` @@ -22,7 +35,6 @@ interface GetEventLogParams { export async function getEventLog(params: GetEventLogParams): Promise { const { getService, spaceId, type, id, provider, actions } = params; const supertest = getService('supertest'); - const actionsSet = new Set(actions); const spacePrefix = getUrlPrefix(spaceId); const url = `${spacePrefix}/api/event_log/${type}/${id}/_find?per_page=5000`; @@ -36,14 +48,35 @@ export async function getEventLog(params: GetEventLogParams): Promise event?.event?.provider === provider) .filter((event) => event?.event?.action) - .filter((event) => actionsSet.has(event?.event?.action!)); - const foundActions = new Set( - events.map((event) => event?.event?.action).filter((action) => !!action) - ); - - for (const action of actions) { - if (!foundActions.has(action)) { - throw new Error(`no event found with action "${action}"`); + .filter((event) => actions.has(event?.event?.action!)); + + const foundActions = events + .map((event) => event?.event?.action) + .reduce((actionsSum, action) => { + if (action) { + actionsSum.set(action, 1 + (actionsSum.get(action) ?? 0)); + } + return actionsSum; + }, new Map()); + + for (const [action, condition] of actions.entries()) { + if ( + !( + foundActions.has(action) && + (isEqualConsition(condition) + ? foundActions.get(action)! === condition.equal + : foundActions.get(action)! >= condition.gte) + ) + ) { + throw new Error( + `insufficient events found with action "${action}" (${ + foundActions.get(action) ?? 0 + } must be ${ + isEqualConsition(condition) + ? `equal to ${condition.equal}` + : `greater than or equal to ${condition.gte}` + })` + ); } } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts index 5c4eb5f5d4c54..9a3b2e7c137a4 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts @@ -518,12 +518,10 @@ export default function ({ getService }: FtrProviderContext) { type: 'action', id: actionId, provider: 'actions', - actions: ['execute'], + actions: new Map([['execute', { equal: 1 }]]), }); }); - expect(events.length).to.equal(1); - const event = events[0]; const duration = event?.event?.duration; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts index 0820b7642e99e..ba21df286fe6e 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts @@ -1096,12 +1096,10 @@ instanceStateValue: true type: 'alert', id: alertId, provider: 'alerting', - actions: ['execute'], + actions: new Map([['execute', { gte: 1 }]]), }); }); - expect(events.length).to.be.greaterThan(0); - const event = events[0]; const duration = event?.event?.duration; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/event_log.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/event_log.ts index 385d8bfca4a9a..459d214c8c993 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/event_log.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/event_log.ts @@ -56,7 +56,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { type: 'alert', id: alertId, provider: 'alerting', - actions: ['execute'], + actions: new Map([['execute', { gte: 1 }]]), }); const errorEvents = someEvents.filter( (event) => event?.kibana?.alerting?.status === 'error' diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts index 2316585d2d0f4..18ac7bfce4a69 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts @@ -270,12 +270,10 @@ export default function ({ getService }: FtrProviderContext) { type: 'action', id: actionId, provider: 'actions', - actions: ['execute'], + actions: new Map([['execute', { equal: 1 }]]), }); }); - expect(events.length).to.equal(1); - const event = events[0]; const duration = event?.event?.duration; diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts index 3766785680925..6d43c28138457 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts @@ -17,8 +17,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { const supertest = getService('supertest'); const retry = getService('retry'); - // FLAKY: https://github.com/elastic/kibana/issues/81668 - describe.skip('eventLog', () => { + describe('eventLog', () => { const objectRemover = new ObjectRemover(supertest); after(() => objectRemover.removeAll()); @@ -73,27 +72,22 @@ export default function eventLogTests({ getService }: FtrProviderContext) { type: 'alert', id: alertId, provider: 'alerting', - actions: [ - 'execute', - 'execute-action', - 'new-instance', - 'active-instance', - 'recovered-instance', - ], + actions: new Map([ + // make sure the counts of the # of events per type are as expected + ['execute', { gte: 4 }], + ['execute-action', { equal: 2 }], + ['new-instance', { equal: 1 }], + ['active-instance', { gte: 1 }], + ['recovered-instance', { equal: 1 }], + ]), }); }); - // make sure the counts of the # of events per type are as expected const executeEvents = getEventsByAction(events, 'execute'); const executeActionEvents = getEventsByAction(events, 'execute-action'); const newInstanceEvents = getEventsByAction(events, 'new-instance'); const recoveredInstanceEvents = getEventsByAction(events, 'recovered-instance'); - expect(executeEvents.length >= 4).to.be(true); - expect(executeActionEvents.length).to.be(2); - expect(newInstanceEvents.length).to.be(1); - expect(recoveredInstanceEvents.length).to.be(1); - // make sure the events are in the right temporal order const executeTimes = getTimestamps(executeEvents); const executeActionTimes = getTimestamps(executeActionEvents); @@ -137,7 +131,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { validateInstanceEvent(event, `created new instance: 'instance'`); break; case 'recovered-instance': - validateInstanceEvent(event, `recovered instance: 'instance'`); + validateInstanceEvent(event, `instance 'instance' has recovered`); break; case 'active-instance': validateInstanceEvent(event, `active instance: 'instance' in actionGroup: 'default'`); @@ -182,7 +176,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { type: 'alert', id: alertId, provider: 'alerting', - actions: ['execute'], + actions: new Map([['execute', { gte: 1 }]]), }); }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts index 404c6020fa237..a5791a900af7e 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts @@ -256,7 +256,7 @@ export default function createGetAlertInstanceSummaryTests({ getService }: FtrPr type: 'alert', id, provider: 'alerting', - actions, + actions: new Map(actions.map((action) => [action, { gte: 1 }])), }); }); }