From 2dd0c613ae38dbda17f7f35183ffd94a8992cab1 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 6 Aug 2019 11:24:46 -0700 Subject: [PATCH] Fix flakiness in request lib tests (#42647) (#42711) * Remove flakiness from 'pollIntervalMs' test by making timing irrelevant. * Remove flakiness from 'state' tests by increasing wait time. * Fix flakiness with 'resets the pollIntervalMs' test by increasing the wait time. --- .../public/request/request.test.js | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/plugins/es_ui_shared/public/request/request.test.js b/src/plugins/es_ui_shared/public/request/request.test.js index 03bed758b120..60b05a8dc8a2 100644 --- a/src/plugins/es_ui_shared/public/request/request.test.js +++ b/src/plugins/es_ui_shared/public/request/request.test.js @@ -98,20 +98,19 @@ describe('request lib', () => { describe('path, method, body', () => { it('is used to send the request', async () => { initUseRequest({ ...successRequest }); - await wait(10); + await wait(50); expect(hook.data).toBe(successResponse.data); }); }); - // FLAKY: https://github.com/elastic/kibana/issues/42561 - describe.skip('pollIntervalMs', () => { + describe('pollIntervalMs', () => { it('sends another request after the specified time has elapsed', async () => { - initUseRequest({ ...successRequest, pollIntervalMs: 30 }); - await wait(5); - sinon.assert.calledOnce(sendPost); - - await wait(40); - sinon.assert.calledTwice(sendPost); + initUseRequest({ ...successRequest, pollIntervalMs: 10 }); + await wait(50); + // We just care that multiple requests have been sent out. We don't check the specific + // timing because that risks introducing flakiness into the tests, and it's unlikely + // we could break the implementation by getting the exact timing wrong. + expect(sendPost.callCount).toBeGreaterThan(1); // We have to manually clean up or else the interval will continue to fire requests, // interfering with other tests. @@ -132,14 +131,14 @@ describe('request lib', () => { initUseRequest({ ...successRequest, deserializer }); sinon.assert.notCalled(deserializer); - await wait(5); + await wait(50); sinon.assert.calledOnce(deserializer); sinon.assert.calledWith(deserializer, successResponse.data); }); it('processes data', async () => { initUseRequest({ ...successRequest, deserializer: () => 'intercepted' }); - await wait(5); + await wait(50); expect(hook.data).toBe('intercepted'); }); }); @@ -152,7 +151,7 @@ describe('request lib', () => { expect(hook.isInitialRequest).toBe(true); hook.sendRequest(); - await wait(5); + await wait(50); expect(hook.isInitialRequest).toBe(false); }); }); @@ -162,7 +161,7 @@ describe('request lib', () => { initUseRequest({ ...successRequest }); expect(hook.isLoading).toBe(true); - await wait(5); + await wait(50); expect(hook.isLoading).toBe(false); }); }); @@ -170,14 +169,13 @@ describe('request lib', () => { describe('error', () => { it('surfaces errors from requests', async () => { initUseRequest({ ...errorRequest }); - await wait(10); + await wait(50); expect(hook.error).toBe(errorResponse); }); - // FLAKY: https://github.com/elastic/kibana/issues/42563 - it.skip('persists while a request is in-flight', async () => { + it('persists while a request is in-flight', async () => { initUseRequest({ ...errorRequest }); - await wait(5); + await wait(50); hook.sendRequest(); expect(hook.isLoading).toBe(true); expect(hook.error).toBe(errorResponse); @@ -185,7 +183,7 @@ describe('request lib', () => { it('is undefined when the request is successful', async () => { initUseRequest({ ...successRequest }); - await wait(10); + await wait(50); expect(hook.isLoading).toBe(false); expect(hook.error).toBeUndefined(); }); @@ -194,30 +192,28 @@ describe('request lib', () => { describe('data', () => { it('surfaces payloads from requests', async () => { initUseRequest({ ...successRequest }); - await wait(10); + await wait(50); expect(hook.data).toBe(successResponse.data); }); it('persists while a request is in-flight', async () => { initUseRequest({ ...successRequest }); - await wait(5); + await wait(50); hook.sendRequest(); expect(hook.isLoading).toBe(true); expect(hook.data).toBe(successResponse.data); }); - // FLAKY: https://github.com/elastic/kibana/issues/42562 - it.skip('is undefined when the request fails', async () => { + it('is undefined when the request fails', async () => { initUseRequest({ ...errorRequest }); - await wait(10); + await wait(50); expect(hook.isLoading).toBe(false); expect(hook.data).toBeUndefined(); }); }); }); - // FLAKY: https://github.com/elastic/kibana/issues/42225 - describe.skip('callbacks', () => { + describe('callbacks', () => { describe('sendRequest', () => { it('sends the request', () => { initUseRequest({ ...successRequest }); @@ -227,19 +223,26 @@ describe('request lib', () => { }); it('resets the pollIntervalMs', async () => { - initUseRequest({ ...successRequest, pollIntervalMs: 30 }); - await wait(5); - sinon.assert.calledOnce(sendPost); + initUseRequest({ ...successRequest, pollIntervalMs: 800 }); + await wait(200); // 200ms + hook.sendRequest(); + expect(sendPost.callCount).toBe(2); - await wait(20); + await wait(200); // 400ms hook.sendRequest(); - // If the request didn't reset the interval, there would have been three requests sent by now. - await wait(20); - sinon.assert.calledTwice(sendPost); + await wait(200); // 600ms + hook.sendRequest(); - await wait(20); - sinon.assert.calledThrice(sendPost); + await wait(200); // 800ms + hook.sendRequest(); + + await wait(200); // 1000ms + hook.sendRequest(); + + // If sendRequest didn't reset the interval, the interval would have triggered another + // request by now, and the callCount would be 7. + expect(sendPost.callCount).toBe(6); // We have to manually clean up or else the interval will continue to fire requests, // interfering with other tests.