Skip to content

Commit

Permalink
[RAM] fixes flaky tests for alerting bulk_edit HTTP API, adds additio…
Browse files Browse the repository at this point in the history 
…nal tests for alerting bulk_edit (#133635)

## Summary

Addresses #132195
Follow-up to #132637

- Fixes flaky tests `x-pack/test/alerting_api_integration/spaces_only/tests/alerting/bulk_edit.ts`
- Adds new tests in spaces_only/tests/alerting/bulk_edit.ts for the following operations: `schedule`, `notifyWhen`, `throttle`
- Adds units test in x-pack/plugins/alerting/server/rules_client/lib/apply_bulk_edit_operation.test.ts

### Details to fixing flaky tests

Despite having bulkEdit retry when 409 conflict happens during bulk update of SO, sometimes there was still failing test.
So, all tests in bulk_edit.ts were skipped 93ffd78

The reason, it happens I believe because mocked rule that used for test is enabled, its update after run clashes with `SavedObject.bulkUpdate` method that used inside `RulesClient.bulkEdit`. So, to fix it, I made this mocked rule disabled, which seems like fixed the flakiness

Here is flaky test runner builds with ENABLED rule: 
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/717
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/713

and with DISABLED:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/718
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/714

In both enabled runs there was test failure, but not for disabled.

Another possible way to fix:
use retry in test for `rules.bulkEdit` call and assertion. Let me know if it can more preferable way to fix it

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
@vitaliidm
vitaliidm authored Jun 15, 2022
1 parent 25ac988 commit 0d0c3d6
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 6 deletions.
54 changes: 54 additions & 0 deletions x-pack/plugins/alerting/server/rules_client/lib/apply_bulk_edit_operation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,58 @@ describe('applyBulkEditOperation', () => {
]);
});
});

describe('throttle operations', () => {
test('should rewrite throttle', () => {
const ruleMock = {
actions: [{ id: 'mock-action-id', group: 'default', params: {} }],
};
expect(
applyBulkEditOperation(
{
field: 'throttle',
value: '1d',
operation: 'set',
},
ruleMock
)
).toHaveProperty('throttle', '1d');
});
});

describe('notifyWhen operations', () => {
test('should rewrite notifyWhen', () => {
const ruleMock = {
actions: [{ id: 'mock-action-id', group: 'default', params: {} }],
};
expect(
applyBulkEditOperation(
{
field: 'notifyWhen',
value: 'onThrottleInterval',
operation: 'set',
},
ruleMock
)
).toHaveProperty('notifyWhen', 'onThrottleInterval');
});
});

describe('schedule operations', () => {
test('should rewrite schedule', () => {
const ruleMock = {
actions: [{ id: 'mock-action-id', group: 'default', params: {} }],
};
expect(
applyBulkEditOperation(
{
field: 'schedule',
value: { interval: '1d' },
operation: 'set',
},
ruleMock
)
).toHaveProperty('schedule', { interval: '1d' });
});
});
});
144 changes: 138 additions & 6 deletions x-pack/test/alerting_api_integration/spaces_only/tests/alerting/bulk_edit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ import { FtrProviderContext } from '../../../common/ftr_provider_context';
export default function createUpdateTests({ getService }: FtrProviderContext) {
const supertest = getService('supertest');

// FLAKY: https://github.com/elastic/kibana/issues/132195
describe.skip('bulkEdit', () => {
describe('bulkEdit', () => {
const objectRemover = new ObjectRemover(supertest);

after(() => objectRemover.removeAll());
Expand All @@ -25,7 +24,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) {
const { body: createdRule } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(getTestRuleData({ tags: ['default'] }));
.send(getTestRuleData({ enabled: false, tags: ['default'] }));

objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');

Expand Down Expand Up @@ -71,7 +70,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) {
supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(getTestRuleData({ tags: [`multiple-rules-edit`] }))
.send(getTestRuleData({ enabled: false, tags: [`multiple-rules-edit`] }))
.expect(200)
)
)
Expand Down Expand Up @@ -119,11 +118,140 @@ export default function createUpdateTests({ getService }: FtrProviderContext) {
});
});

it('should bulk edit rule with schedule operation', async () => {
const { body: createdRule } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(getTestRuleData({ enabled: false, schedule: { interval: '10m' } }));

objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');

const payload = {
ids: [createdRule.id],
operations: [
{
operation: 'set',
field: 'schedule',
value: { interval: '1h' },
},
],
};

const bulkEditResponse = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`)
.set('kbn-xsrf', 'foo')
.send(payload);

expect(bulkEditResponse.body.errors).to.have.length(0);
expect(bulkEditResponse.body.rules).to.have.length(1);
expect(bulkEditResponse.body.rules[0].schedule).to.eql({ interval: '1h' });

const { body: updatedRule } = await supertest
.get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`)
.set('kbn-xsrf', 'foo');

expect(updatedRule.schedule).to.eql({ interval: '1h' });

// Ensure AAD isn't broken
await checkAAD({
supertest,
spaceId: Spaces.space1.id,
type: 'alert',
id: createdRule.id,
});
});

it('should bulk edit rule with throttle operation', async () => {
const { body: createdRule } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(getTestRuleData({ enabled: false }));

objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');

const payload = {
ids: [createdRule.id],
operations: [
{
operation: 'set',
field: 'throttle',
value: '1h',
},
],
};

const bulkEditResponse = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`)
.set('kbn-xsrf', 'foo')
.send(payload);

expect(bulkEditResponse.body.errors).to.have.length(0);
expect(bulkEditResponse.body.rules).to.have.length(1);
expect(bulkEditResponse.body.rules[0]).property('throttle', '1h');

const { body: updatedRule } = await supertest
.get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`)
.set('kbn-xsrf', 'foo');

expect(updatedRule).property('throttle', '1h');

// Ensure AAD isn't broken
await checkAAD({
supertest,
spaceId: Spaces.space1.id,
type: 'alert',
id: createdRule.id,
});
});

it('should bulk edit rule with notifyWhen operation', async () => {
const { body: createdRule } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(getTestRuleData({ enabled: false, throttle: '1h' }));

objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');

const payload = {
ids: [createdRule.id],
operations: [
{
operation: 'set',
field: 'notifyWhen',
value: 'onActionGroupChange',
},
],
};

const bulkEditResponse = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`)
.set('kbn-xsrf', 'foo')
.send(payload);

expect(bulkEditResponse.body.errors).to.have.length(0);
expect(bulkEditResponse.body.rules).to.have.length(1);
expect(bulkEditResponse.body.rules[0]).property('notify_when', 'onActionGroupChange');

const { body: updatedRule } = await supertest
.get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`)
.set('kbn-xsrf', 'foo');

expect(updatedRule).property('notify_when', 'onActionGroupChange');

// Ensure AAD isn't broken
await checkAAD({
supertest,
spaceId: Spaces.space1.id,
type: 'alert',
id: createdRule.id,
});
});

it(`shouldn't bulk edit rule from another space`, async () => {
const { body: createdRule } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(getTestRuleData({ tags: ['default'] }));
.send(getTestRuleData({ enabled: false, tags: ['default'] }));

objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');

Expand All @@ -150,7 +278,11 @@ export default function createUpdateTests({ getService }: FtrProviderContext) {
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
.set('kbn-xsrf', 'foo')
.send(
getTestRuleData({ tags: ['default'], params: { risk_score: 40, severity: 'medium' } })
getTestRuleData({
enabled: false,
tags: ['default'],
params: { risk_score: 40, severity: 'medium' },
})
);

objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');
Expand Down

0 comments on commit 0d0c3d6

Please sign in to comment.