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

[Task manager] Simplifies buffer tests to reduce flakiness #73024

Merged
merged 3 commits into from
Jul 27, 2020
Merged

[Task manager] Simplifies buffer tests to reduce flakiness #73024

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f10752b
simplified buffer tests to reduce flakyness
gmmorris Jul 23, 2020
b4e78e7
Merge branch 'master' into task-manager/fix-flaky-timers
gmmorris Jul 23, 2020
0e14656
Merge branch 'master' into task-manager/fix-flaky-timers
elasticmachine Jul 27, 2020
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
80 changes: 32 additions & 48 deletions x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ function errorAttempts(task: TaskInstance): Err<OperationError<TaskInstance, Err
});
}

// FLAKY: https://github.com/elastic/kibana/issues/72864
describe.skip('Bulk Operation Buffer', () => {
describe('Bulk Operation Buffer', () => {
describe('createBuffer()', () => {
test('batches up multiple Operation calls', async () => {
const bulkUpdate: jest.Mocked<BulkOperation<TaskInstance, Error>> = jest.fn(
Expand Down Expand Up @@ -67,8 +66,6 @@ describe.skip('Bulk Operation Buffer', () => {
const task2 = createTask();
const task3 = createTask();
const task4 = createTask();
const task5 = createTask();
const task6 = createTask();

return new Promise((resolve) => {
Promise.all([bufferedUpdate(task1), bufferedUpdate(task2)]).then((_) => {
Expand All @@ -79,22 +76,18 @@ describe.skip('Bulk Operation Buffer', () => {

setTimeout(() => {
// on next tick
setTimeout(() => {
// on next tick
expect(bulkUpdate).toHaveBeenCalledTimes(2);
Promise.all([bufferedUpdate(task5), bufferedUpdate(task6)]).then((_) => {
expect(bulkUpdate).toHaveBeenCalledTimes(3);
expect(bulkUpdate).toHaveBeenCalledWith([task5, task6]);
resolve();
});
}, bufferMaxDuration + 1);

expect(bulkUpdate).toHaveBeenCalledTimes(1);
Promise.all([bufferedUpdate(task3), bufferedUpdate(task4)]).then((_) => {
expect(bulkUpdate).toHaveBeenCalledTimes(2);
expect(bulkUpdate).toHaveBeenCalledWith([task3, task4]);
});
}, bufferMaxDuration + 1);

setTimeout(() => {
// on next tick
expect(bulkUpdate).toHaveBeenCalledTimes(2);
resolve();
}, bufferMaxDuration * 1.1);
}, bufferMaxDuration * 1.1);
});
});

Expand All @@ -103,8 +96,9 @@ describe.skip('Bulk Operation Buffer', () => {
return Promise.resolve(tasks.map(incrementAttempts));
});

const bufferMaxDuration = 1000;
const bufferedUpdate = createBuffer(bulkUpdate, {
bufferMaxDuration: 100,
bufferMaxDuration,
bufferMaxOperations: 2,
});

Expand All @@ -114,26 +108,19 @@ describe.skip('Bulk Operation Buffer', () => {
const task4 = createTask();
const task5 = createTask();

return new Promise((resolve) => {
bufferedUpdate(task1);
bufferedUpdate(task2);
bufferedUpdate(task3);
bufferedUpdate(task4);

setTimeout(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(2);
expect(bulkUpdate).toHaveBeenCalledWith([task1, task2]);
expect(bulkUpdate).toHaveBeenCalledWith([task3, task4]);

setTimeout(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(2);
bufferedUpdate(task5).then((_) => {
expect(bulkUpdate).toHaveBeenCalledTimes(3);
expect(bulkUpdate).toHaveBeenCalledWith([task5]);
resolve();
});
}, 50);
}, 50);
return Promise.all([
bufferedUpdate(task1),
bufferedUpdate(task2),
bufferedUpdate(task3),
bufferedUpdate(task4),
]).then(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(2);
expect(bulkUpdate).toHaveBeenCalledWith([task1, task2]);
expect(bulkUpdate).toHaveBeenCalledWith([task3, task4]);
return bufferedUpdate(task5).then((_) => {
expect(bulkUpdate).toHaveBeenCalledTimes(3);
expect(bulkUpdate).toHaveBeenCalledWith([task5]);
});
});
});

Expand All @@ -153,29 +140,26 @@ describe.skip('Bulk Operation Buffer', () => {
const task3 = createTask();
const task4 = createTask();

return new Promise((resolve) => {
bufferedUpdate(task1);
bufferedUpdate(task2);

setTimeout(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(1);
expect(bulkUpdate).toHaveBeenCalledWith([task1, task2]);
return Promise.all([bufferedUpdate(task1), bufferedUpdate(task2)]).then(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(1);
expect(bulkUpdate).toHaveBeenCalledWith([task1, task2]);

bufferedUpdate(task3);
bufferedUpdate(task4);
return new Promise((resolve) => {
const futureUpdates = Promise.all([bufferedUpdate(task3), bufferedUpdate(task4)]);

setTimeout(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(1);

setTimeout(() => {
futureUpdates.then(() => {
expect(bulkUpdate).toHaveBeenCalledTimes(2);
expect(bulkUpdate).toHaveBeenCalledWith([task3, task4]);
resolve();
}, bufferMaxDuration / 2);
});
}, bufferMaxDuration / 2);
}, bufferMaxDuration + 1);
});
});
});

test('handles both resolutions and rejections at individual task level', async (done) => {
const bulkUpdate: jest.Mocked<BulkOperation<TaskInstance, Error>> = jest.fn(
([task1, task2, task3]) => {
Expand Down