Change Jest preset to "node" for server-side UTs (#133017)

* change Jest preset to "node" for server-side UTs * small code enhancements, graceful stop
elastic · May 31, 2022 · 0052fd2 · 0052fd2
1 parent 7570715
commit 0052fd2
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 36 deletions.
diff --git a/packages/kbn-test/src/jest/run_check_jest_configs_cli.ts b/packages/kbn-test/src/jest/run_check_jest_configs_cli.ts
@@ -38,6 +38,7 @@ const roots: string[] = [
   'x-pack/plugins',
   'src/plugins',
   'test',
+  'src/core',
   'src',
   ...getAllRepoRelativeBazelPackageDirs(),
 ];

diff --git a/src/core/public/jest.config.js b/src/core/public/jest.config.js
@@ -0,0 +1,13 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+module.exports = {
+  preset: '@kbn/test',
+  rootDir: '../../..',
+  roots: ['<rootDir>/src/core/public'],
+};
diff --git a/src/core/jest.integration.config.js → src/core/public/jest.integration.config.js b/src/core/jest.integration.config.js → src/core/public/jest.integration.config.js
@@ -8,6 +8,6 @@
 
 module.exports = {
   preset: '@kbn/test/jest_integration',
-  rootDir: '../..',
-  roots: ['<rootDir>/src/core'],
+  rootDir: '../../..',
+  roots: ['<rootDir>/src/core/public'],
 };
diff --git a/src/core/server/elasticsearch/client/cluster_client.test.ts b/src/core/server/elasticsearch/client/cluster_client.test.ts
@@ -537,7 +537,7 @@ describe('ClusterClient', () => {
       expect(scopedClient.close).toHaveBeenCalledTimes(1);
     });
 
-    it('waits for both clients to close', async (done) => {
+    it('waits for both clients to close', (done) => {
       expect.assertions(4);
 
       const clusterClient = new ClusterClient({

diff --git a/src/core/server/elasticsearch/elasticsearch_service.test.ts b/src/core/server/elasticsearch/elasticsearch_service.test.ts
@@ -16,11 +16,14 @@ jest.mock('./version_check/ensure_es_version', () => ({
   pollEsNodesVersion: jest.fn(),
 }));
 
+// Mocking the module to disable caching for tests
+jest.mock('../ui_settings/cache');
+
 import { MockClusterClient, isScriptingEnabledMock } from './elasticsearch_service.test.mocks';
 
 import type { NodesVersionCompatibility } from './version_check/ensure_es_version';
-import { BehaviorSubject } from 'rxjs';
-import { first } from 'rxjs/operators';
+import { BehaviorSubject, firstValueFrom } from 'rxjs';
+import { first, concatMap } from 'rxjs/operators';
 import { REPO_ROOT } from '@kbn/utils';
 import { Env } from '../config';
 import { configServiceMock, getEnvOptions } from '../config/mocks';
@@ -85,10 +88,11 @@ beforeEach(() => {
   pollEsNodesVersionMocked.mockImplementation(pollEsNodesVersionActual);
 });
 
-afterEach(() => {
+afterEach(async () => {
   jest.clearAllMocks();
   MockClusterClient.mockClear();
   isScriptingEnabledMock.mockReset();
+  await elasticsearchService?.stop();
 });
 
 describe('#preboot', () => {
@@ -186,7 +190,7 @@ describe('#setup', () => {
     );
   });
 
-  it('esNodeVersionCompatibility$ only starts polling when subscribed to', async (done) => {
+  it('esNodeVersionCompatibility$ only starts polling when subscribed to', async () => {
     const mockedClient = mockClusterClientInstance.asInternalUser;
     mockedClient.nodes.info.mockImplementation(() =>
       elasticsearchClientMock.createErrorTransportRequestPromise(new Error())
@@ -196,13 +200,12 @@ describe('#setup', () => {
     await delay(10);
 
     expect(mockedClient.nodes.info).toHaveBeenCalledTimes(0);
-    setupContract.esNodesCompatibility$.subscribe(() => {
-      expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
-      done();
-    });
+
+    await firstValueFrom(setupContract.esNodesCompatibility$);
+    expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
   });
 
-  it('esNodeVersionCompatibility$ stops polling when unsubscribed from', async (done) => {
+  it('esNodeVersionCompatibility$ stops polling when unsubscribed from', async () => {
     const mockedClient = mockClusterClientInstance.asInternalUser;
     mockedClient.nodes.info.mockImplementation(() =>
       elasticsearchClientMock.createErrorTransportRequestPromise(new Error())
@@ -211,12 +214,10 @@ describe('#setup', () => {
     const setupContract = await elasticsearchService.setup(setupDeps);
 
     expect(mockedClient.nodes.info).toHaveBeenCalledTimes(0);
-    const sub = setupContract.esNodesCompatibility$.subscribe(async () => {
-      sub.unsubscribe();
-      await delay(100);
-      expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
-      done();
-    });
+
+    await firstValueFrom(setupContract.esNodesCompatibility$);
+    await delay(100);
+    expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
   });
 });
 
@@ -402,7 +403,7 @@ describe('#stop', () => {
     expect(mockClusterClientInstance.close).toHaveBeenCalledTimes(1);
   });
 
-  it('stops pollEsNodeVersions even if there are active subscriptions', async (done) => {
+  it('stops pollEsNodeVersions even if there are active subscriptions', async () => {
     expect.assertions(3);
 
     const mockedClient = mockClusterClientInstance.asInternalUser;
@@ -412,15 +413,18 @@ describe('#stop', () => {
 
     const setupContract = await elasticsearchService.setup(setupDeps);
 
-    setupContract.esNodesCompatibility$.subscribe(async () => {
-      expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
-      await delay(10);
-      expect(mockedClient.nodes.info).toHaveBeenCalledTimes(2);
-
-      await elasticsearchService.stop();
-      await delay(100);
-      expect(mockedClient.nodes.info).toHaveBeenCalledTimes(2);
-      done();
-    });
+    await firstValueFrom(
+      setupContract.esNodesCompatibility$.pipe(
+        concatMap(async () => {
+          expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
+          await delay(10);
+          expect(mockedClient.nodes.info).toHaveBeenCalledTimes(2);
+
+          await elasticsearchService.stop();
+          await delay(100);
+          expect(mockedClient.nodes.info).toHaveBeenCalledTimes(2);
+        })
+      )
+    );
   });
 });
diff --git a/src/core/server/execution_context/execution_context_container.test.ts b/src/core/server/execution_context/execution_context_container.test.ts
@@ -5,6 +5,8 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
+
+import { Blob } from 'buffer';
 import type { KibanaExecutionContext } from '../../types';
 
 import {

diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts
@@ -106,6 +106,7 @@ test('creates and sets up http server', async () => {
   await service.start();
   expect(httpServer.start).toHaveBeenCalled();
   expect(prebootHttpServer.stop).toHaveBeenCalled();
+  await service.stop();
 });
 
 test('spins up `preboot` server until started if configured with `autoListen:true`', async () => {
@@ -161,6 +162,7 @@ test('spins up `preboot` server until started if configured with `autoListen:tru
 
   expect(httpServer.start).toBeCalledTimes(1);
   expect(prebootHapiServer.stop).toBeCalledTimes(1);
+  await service.stop();
 });
 
 test('logs error if already set up', async () => {
@@ -189,6 +191,7 @@ test('logs error if already set up', async () => {
   await service.setup(setupDeps);
 
   expect(loggingSystemMock.collect(logger).warn).toMatchSnapshot();
+  await service.stop();
 });
 
 test('stops http server', async () => {
@@ -314,6 +317,7 @@ test('register route handler', async () => {
 
   expect(registerRouterMock).toHaveBeenCalledTimes(1);
   expect(registerRouterMock).toHaveBeenLastCalledWith(router);
+  await service.stop();
 });
 
 test('register preboot route handler on preboot', async () => {
@@ -340,6 +344,7 @@ test('register preboot route handler on preboot', async () => {
   const [[router]] = registerRoutesMock.mock.calls;
   expect(registerRouterMock).toHaveBeenCalledTimes(1);
   expect(registerRouterMock).toHaveBeenCalledWith(router);
+  await service.stop();
 });
 
 test('register preboot route handler on setup', async () => {
@@ -353,8 +358,14 @@ test('register preboot route handler on setup', async () => {
       }),
       start: noop,
       stop: noop,
+      isListening: jest.fn(),
     }))
-    .mockImplementationOnce(() => ({ setup: () => ({ server: {} }), start: noop, stop: noop }));
+    .mockImplementationOnce(() => ({
+      setup: () => ({ server: {} }),
+      start: noop,
+      stop: noop,
+      isListening: jest.fn(),
+    }));
 
   const service = new HttpService({ coreId, configService: createConfigService(), env, logger });
   await service.preboot(prebootDeps);
@@ -369,6 +380,7 @@ test('register preboot route handler on setup', async () => {
   const [[router]] = registerRoutesMock.mock.calls;
   expect(registerRouterMock).toHaveBeenCalledTimes(1);
   expect(registerRouterMock).toHaveBeenCalledWith(router);
+  await service.stop();
 });
 
 test('returns `preboot` http server contract on preboot', async () => {
@@ -400,6 +412,7 @@ test('returns `preboot` http server contract on preboot', async () => {
     registerStaticDir: expect.any(Function),
     getServerInfo: expect.any(Function),
   });
+  await service.stop();
 });
 
 test('returns http server contract on setup', async () => {
@@ -430,6 +443,7 @@ test('returns http server contract on setup', async () => {
     createRouter: expect.any(Function),
     registerPrebootRoutes: expect.any(Function),
   });
+  await service.stop();
 });
 
 test('does not start http server if configured with `autoListen:false`', async () => {
@@ -464,4 +478,5 @@ test('does not start http server if configured with `autoListen:false`', async (
   await service.start();
 
   expect(httpServer.start).not.toHaveBeenCalled();
+  await service.stop();
 });
diff --git a/src/core/server/jest.config.js b/src/core/server/jest.config.js
@@ -0,0 +1,13 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+module.exports = {
+  preset: '@kbn/test/jest_node',
+  rootDir: '../../..',
+  roots: ['<rootDir>/src/core/server'],
+};
diff --git a/src/core/server/jest.integration.config.js b/src/core/server/jest.integration.config.js
@@ -0,0 +1,17 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+module.exports = {
+  // TODO replace the line below with
+  // preset: '@kbn/test/jest_integration_node
+  // to do so, we must fix all integration tests first
+  // see https://github.com/elastic/kibana/pull/130255/
+  preset: '@kbn/test/jest_integration',
+  rootDir: '../../..',
+  roots: ['<rootDir>/src/core/server'],
+};
diff --git a/src/core/server/preboot/preboot_service.test.ts b/src/core/server/preboot/preboot_service.test.ts
@@ -6,7 +6,6 @@
  * Side Public License, v 1.
  */
 
-import { nextTick } from '@kbn/test-jest-helpers';
 import { REPO_ROOT } from '@kbn/utils';
 import { LoggerFactory } from '@kbn/logging';
 import { Env } from '@kbn/config';
@@ -15,6 +14,13 @@ import { configServiceMock, loggingSystemMock } from '../mocks';
 
 import { PrebootService } from './preboot_service';
 
+function nextTick() {
+  // we can't import { nextTick } from '@kbn/test-jest-helpers' on server-side tests
+  // because the package contains code that relies on jsdom (aka browser-like Jest presets)
+  // see https://github.com/elastic/kibana/pull/130255#discussion_r855033733
+  return new Promise((resolve) => setImmediate(resolve));
+}
+
 describe('PrebootService', () => {
   describe('#preboot()', () => {
     let service: PrebootService;

diff --git a/src/core/server/saved_objects/saved_objects_service.test.ts b/src/core/server/saved_objects/saved_objects_service.test.ts
@@ -8,6 +8,7 @@
 
 import { join } from 'path';
 import loadJsonFile from 'load-json-file';
+import { setImmediate } from 'timers/promises';
 
 import {
   clientProviderInstanceMock,
@@ -282,7 +283,7 @@ describe('SavedObjectsService', () => {
       expect(KibanaMigratorMock).toHaveBeenCalledWith(expect.objectContaining({ kibanaVersion }));
     });
 
-    it('waits for all es nodes to be compatible before running migrations', async (done) => {
+    it('waits for all es nodes to be compatible before running migrations', async () => {
       expect.assertions(2);
       const coreContext = createCoreContext({ skipMigration: false });
       const soService = new SavedObjectsService(coreContext);
@@ -307,10 +308,9 @@ describe('SavedObjectsService', () => {
         warningNodes: [],
         kibanaVersion: '8.0.0',
       });
-      setImmediate(() => {
-        expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(1);
-        done();
-      });
+
+      await setImmediate();
+      expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(1);
     });
 
     it('resolves with KibanaMigrator after waiting for migrations to complete', async () => {