From 050248255cb24ecc37eefb0b72bf412c9c34e1fe Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 2 Nov 2020 10:47:56 -0800 Subject: [PATCH 1/4] Refactor Remote Clusters client integration tests for readability. - Colocate helpers with test files. - Remove default API responses from HTTP response mocking functions to make behavior clearer at call sites. --- .../remote_clusters_add.helpers.js | 0 .../{ => add}/remote_clusters_add.test.js | 7 +++---- .../special_characters.js} | 8 ------- .../remote_clusters_edit.helpers.js | 8 ++++++- .../{ => edit}/remote_clusters_edit.test.js | 14 +++++++------ .../helpers/http_requests.js | 15 ++++--------- .../client_integration/helpers/index.js | 11 ---------- .../remote_clusters_list.helpers.js | 0 .../{ => list}/remote_clusters_list.test.js | 21 +++++++------------ 9 files changed, 29 insertions(+), 55 deletions(-) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{helpers => add}/remote_clusters_add.helpers.js (100%) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{ => add}/remote_clusters_add.test.js (97%) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{helpers/constants.js => add/special_characters.js} (83%) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{helpers => edit}/remote_clusters_edit.helpers.js (85%) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{ => edit}/remote_clusters_edit.test.js (87%) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{helpers => list}/remote_clusters_list.helpers.js (100%) rename x-pack/plugins/remote_clusters/__jest__/client_integration/{ => list}/remote_clusters_list.test.js (95%) diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_add.helpers.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/add/remote_clusters_add.helpers.js similarity index 100% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_add.helpers.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/add/remote_clusters_add.helpers.js diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_add.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/add/remote_clusters_add.test.js similarity index 97% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_add.test.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/add/remote_clusters_add.test.js index 6e1b2f6266584..05a4a2e330325 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_add.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/add/remote_clusters_add.test.js @@ -4,10 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { pageHelpers, nextTick, setupEnvironment } from './helpers'; -import { NON_ALPHA_NUMERIC_CHARS, ACCENTED_CHARS } from './helpers/constants'; - -const { setup } = pageHelpers.remoteClustersAdd; +import { nextTick, setupEnvironment } from '../helpers'; +import { NON_ALPHA_NUMERIC_CHARS, ACCENTED_CHARS } from './special_characters'; +import { setup } from './remote_clusters_add.helpers'; describe('Create Remote cluster', () => { describe('on component mount', () => { diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/constants.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/add/special_characters.js similarity index 83% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/constants.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/add/special_characters.js index bdc965b4037dd..d2ef598127572 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/constants.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/add/special_characters.js @@ -4,14 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -export const REMOTE_CLUSTER_EDIT_NAME = 'new-york'; - -export const REMOTE_CLUSTER_EDIT = { - name: REMOTE_CLUSTER_EDIT_NAME, - seeds: ['localhost:9400'], - skipUnavailable: true, -}; - export const NON_ALPHA_NUMERIC_CHARS = [ '#', '@', diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_edit.helpers.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/edit/remote_clusters_edit.helpers.js similarity index 85% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_edit.helpers.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/edit/remote_clusters_edit.helpers.js index 294861405b9d5..b5402f3b017f0 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_edit.helpers.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/edit/remote_clusters_edit.helpers.js @@ -11,7 +11,13 @@ import { RemoteClusterEdit } from '../../../public/application/sections/remote_c import { createRemoteClustersStore } from '../../../public/application/store'; import { registerRouter } from '../../../public/application/services/routing'; -import { REMOTE_CLUSTER_EDIT_NAME } from './constants'; +export const REMOTE_CLUSTER_EDIT_NAME = 'new-york'; + +export const REMOTE_CLUSTER_EDIT = { + name: REMOTE_CLUSTER_EDIT_NAME, + seeds: ['localhost:9400'], + skipUnavailable: true, +}; const testBedConfig = { store: createRemoteClustersStore, diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_edit.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/edit/remote_clusters_edit.test.js similarity index 87% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_edit.test.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/edit/remote_clusters_edit.test.js index a5905227f49b8..b0e0832cb0831 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_edit.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/edit/remote_clusters_edit.test.js @@ -6,12 +6,14 @@ import { act } from 'react-dom/test-utils'; -import { RemoteClusterForm } from '../../public/application/sections/components/remote_cluster_form'; -import { pageHelpers, setupEnvironment } from './helpers'; -import { REMOTE_CLUSTER_EDIT, REMOTE_CLUSTER_EDIT_NAME } from './helpers/constants'; - -const { setup } = pageHelpers.remoteClustersEdit; -const { setup: setupRemoteClustersAdd } = pageHelpers.remoteClustersAdd; +import { RemoteClusterForm } from '../../../public/application/sections/components/remote_cluster_form'; +import { setupEnvironment } from '../helpers'; +import { setup as setupRemoteClustersAdd } from '../add/remote_clusters_add.helpers'; +import { + setup, + REMOTE_CLUSTER_EDIT, + REMOTE_CLUSTER_EDIT_NAME, +} from './remote_clusters_edit.helpers'; describe('Edit Remote cluster', () => { let server; diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/http_requests.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/http_requests.js index 182700b0c8fbd..e174a8faa3c57 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/http_requests.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/http_requests.js @@ -8,29 +8,22 @@ import sinon from 'sinon'; // Register helpers to mock HTTP Requests const registerHttpRequestMockHelpers = (server) => { - const mockResponse = (defaultResponse, response) => [ + const mockResponse = (response) => [ 200, { 'Content-Type': 'application/json' }, - JSON.stringify({ ...defaultResponse, ...response }), + JSON.stringify(response), ]; const setLoadRemoteClustersResponse = (response) => { - const defaultResponse = []; - server.respondWith('GET', '/api/remote_clusters', [ 200, { 'Content-Type': 'application/json' }, - JSON.stringify(response ? response : defaultResponse), + JSON.stringify(response), ]); }; const setDeleteRemoteClusterResponse = (response) => { - const defaultResponse = { - itemsDeleted: [], - errors: [], - }; - - server.respondWith('DELETE', /api\/remote_clusters/, mockResponse(defaultResponse, response)); + server.respondWith('DELETE', /api\/remote_clusters/, mockResponse(response)); }; return { diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/index.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/index.js index d70ba2a21e176..6ef0e10b6ae15 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/index.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/index.js @@ -4,16 +4,5 @@ * you may not use this file except in compliance with the Elastic License. */ -import { setup as remoteClustersAddSetup } from './remote_clusters_add.helpers'; -import { setup as remoteClustersEditSetup } from './remote_clusters_edit.helpers'; -import { setup as remoteClustersListSetup } from './remote_clusters_list.helpers'; - export { nextTick, getRandomString, findTestSubject } from '../../../../../test_utils'; - export { setupEnvironment } from './setup_environment'; - -export const pageHelpers = { - remoteClustersAdd: { setup: remoteClustersAddSetup }, - remoteClustersEdit: { setup: remoteClustersEditSetup }, - remoteClustersList: { setup: remoteClustersListSetup }, -}; diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_list.helpers.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.helpers.js similarity index 100% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/helpers/remote_clusters_list.helpers.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.helpers.js diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js similarity index 95% rename from x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js rename to x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js index 9489eb86681b8..a715f244f84f9 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js @@ -5,20 +5,14 @@ */ import { act } from 'react-dom/test-utils'; -import { - pageHelpers, - setupEnvironment, - nextTick, - getRandomString, - findTestSubject, -} from './helpers'; +import { getRouter } from '../../../public/application/services'; +import { getRemoteClusterMock } from '../../../fixtures/remote_cluster'; -import { getRouter } from '../../public/application/services'; -import { getRemoteClusterMock } from '../../fixtures/remote_cluster'; +import { PROXY_MODE } from '../../../common/constants'; -import { PROXY_MODE } from '../../common/constants'; +import { setupEnvironment, nextTick, getRandomString, findTestSubject } from '../helpers'; -const { setup } = pageHelpers.remoteClustersList; +import { setup } from './remote_clusters_list.helpers'; describe('', () => { let server; @@ -33,8 +27,7 @@ describe('', () => { }); beforeEach(() => { - // Set "default" mock responses by not providing any arguments - httpRequestsMockHelpers.setLoadRemoteClustersResponse(); + httpRequestsMockHelpers.setLoadRemoteClustersResponse([]); }); describe('on component mount', () => { @@ -56,7 +49,7 @@ describe('', () => { beforeEach(async () => { ({ exists, component } = setup()); - await nextTick(100); // We need to wait next tick for the mock server response to kick in + // await nextTick(100); // We need to wait next tick for the mock server response to kick in component.update(); }); From 8ee1674f2852e8f0a102f9727907f71056d55f69 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 2 Nov 2020 11:30:36 -0800 Subject: [PATCH 2/4] Remove await nextTick, which was causing flakiness. --- .../list/remote_clusters_list.test.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js index a715f244f84f9..9944378fb0fa0 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js @@ -3,6 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ + import { act } from 'react-dom/test-utils'; import { getRouter } from '../../../public/application/services'; @@ -15,12 +16,7 @@ import { setupEnvironment, nextTick, getRandomString, findTestSubject } from '.. import { setup } from './remote_clusters_list.helpers'; describe('', () => { - let server; - let httpRequestsMockHelpers; - - beforeAll(() => { - ({ server, httpRequestsMockHelpers } = setupEnvironment()); - }); + const { server, httpRequestsMockHelpers } = setupEnvironment(); afterAll(() => { server.restore(); @@ -43,14 +39,13 @@ describe('', () => { }); describe('when there are no remote clusters', () => { + let waitFor; let exists; - let component; beforeEach(async () => { - ({ exists, component } = setup()); + ({ waitFor, exists } = setup()); - // await nextTick(100); // We need to wait next tick for the mock server response to kick in - component.update(); + await waitFor('remoteClusterListEmptyPrompt'); }); test('should display an empty prompt', async () => { From db222f43f375afc7fe58819b3c262caed525eeb7 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 3 Nov 2020 08:46:27 -0800 Subject: [PATCH 3/4] Revert "Remove await nextTick, which was causing flakiness." This reverts commit 8ee1674f2852e8f0a102f9727907f71056d55f69. --- .../list/remote_clusters_list.test.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js index 9944378fb0fa0..a715f244f84f9 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js @@ -3,7 +3,6 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ - import { act } from 'react-dom/test-utils'; import { getRouter } from '../../../public/application/services'; @@ -16,7 +15,12 @@ import { setupEnvironment, nextTick, getRandomString, findTestSubject } from '.. import { setup } from './remote_clusters_list.helpers'; describe('', () => { - const { server, httpRequestsMockHelpers } = setupEnvironment(); + let server; + let httpRequestsMockHelpers; + + beforeAll(() => { + ({ server, httpRequestsMockHelpers } = setupEnvironment()); + }); afterAll(() => { server.restore(); @@ -39,13 +43,14 @@ describe('', () => { }); describe('when there are no remote clusters', () => { - let waitFor; let exists; + let component; beforeEach(async () => { - ({ waitFor, exists } = setup()); + ({ exists, component } = setup()); - await waitFor('remoteClusterListEmptyPrompt'); + // await nextTick(100); // We need to wait next tick for the mock server response to kick in + component.update(); }); test('should display an empty prompt', async () => { From fe7ca760a51b8be15a27dc444a4175f455aaed20 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 3 Nov 2020 09:10:39 -0800 Subject: [PATCH 4/4] Uncomment line. --- .../client_integration/list/remote_clusters_list.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js index a715f244f84f9..d75921c5f49f2 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/list/remote_clusters_list.test.js @@ -49,7 +49,7 @@ describe('', () => { beforeEach(async () => { ({ exists, component } = setup()); - // await nextTick(100); // We need to wait next tick for the mock server response to kick in + await nextTick(100); // We need to wait next tick for the mock server response to kick in component.update(); });