Skip to content

Commit

Permalink
Fix flaky test around search cancellation (elastic#193008)
Browse files Browse the repository at this point in the history
Resolves elastic#192914

In this PR, I'm fixing the flakiness in tests where sometimes rules fail
with a different message on timeout. This is expected as it's a race
condition between the Elasticsearch request timing out and the alerting
rule getting cancelled. So we can expect one of two messages.

Note: Test is not skipped as of PR creation
  • Loading branch information
mikecote authored Sep 16, 2024
1 parent 6680f35 commit 0c63a7b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,25 @@ export default function ruleTests({ getService }: FtrProviderContext) {
events.filter((event) => event?.event?.action === 'execute');
expect(events[0]?.event?.outcome).to.eql('failure');
expect(events[0]?.kibana?.alerting?.status).to.eql('error');
expect(events[0]?.error?.message).to.eql(
'Search has been aborted due to cancelled execution'
);
// Timeouts will encounter one of the following two messages
const expectedMessages = [
'Request timed out',
'Search has been aborted due to cancelled execution',
];
expect(expectedMessages.includes(events[0]?.error?.message || '')).to.be(true);

// rule execution status should be in error with reason timeout
const { status, body: rule } = await supertest.get(
`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule/${ruleId}`
);
expect(status).to.eql(200);
expect(rule.execution_status.status).to.eql('error');
expect(rule.execution_status.error.message).to.eql(
`test.cancellableRule:${ruleId}: execution cancelled due to timeout - exceeded rule type timeout of 3s`
);
expect(
[
'Request timed out',
`test.cancellableRule:${ruleId}: execution cancelled due to timeout - exceeded rule type timeout of 3s`,
].includes(rule.execution_status.error.message)
).to.eql(true);
expect(rule.execution_status.error.reason).to.eql('timeout');
});

Expand Down Expand Up @@ -183,9 +189,12 @@ export default function ruleTests({ getService }: FtrProviderContext) {
);
expect(status).to.eql(200);
expect(rule.execution_status.status).to.eql('error');
expect(rule.execution_status.error.message).to.eql(
`test.cancellableRule:${ruleId}: execution cancelled due to timeout - exceeded rule type timeout of 3s`
);
expect(
[
'Request timed out',
`test.cancellableRule:${ruleId}: execution cancelled due to timeout - exceeded rule type timeout of 3s`,
].includes(rule.execution_status.error.message)
).to.eql(true);
expect(rule.execution_status.error.reason).to.eql('timeout');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,12 @@ export default function ruleTests({ getService }: FtrProviderContext) {
expect(errorStatuses.length).to.be.greaterThan(0);
const lastErrorStatus = errorStatuses.pop();
expect(lastErrorStatus?.status).to.eql('error');
expect(lastErrorStatus?.error.message).to.eql(
`test.patternLongRunning.cancelAlertsOnRuleTimeout:${ruleId}: execution cancelled due to timeout - exceeded rule type timeout of 3s`
);
expect(
[
'Request timed out',
`test.patternLongRunning.cancelAlertsOnRuleTimeout:${ruleId}: execution cancelled due to timeout - exceeded rule type timeout of 3s`,
].includes(lastErrorStatus?.error.message || '')
).to.eql(true);
expect(lastErrorStatus?.error.reason).to.eql('timeout');
});

Expand Down

0 comments on commit 0c63a7b

Please sign in to comment.