From a5c2603bc4d865681228b3a8f805d3a2ad955faa Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 6 Feb 2020 09:39:27 +0100 Subject: [PATCH 1/4] Expose core/public savedObjectsServiceMock --- src/core/public/legacy/legacy_service.test.ts | 4 ++-- src/core/public/mocks.ts | 5 +++-- src/core/public/plugins/plugins_service.test.ts | 4 ++-- src/core/public/saved_objects/saved_objects_service.mock.ts | 2 +- src/core/public/saved_objects/simple_saved_object.ts | 4 ++-- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/core/public/legacy/legacy_service.test.ts b/src/core/public/legacy/legacy_service.test.ts index d08c8b52e39c9..c3de645c6b17e 100644 --- a/src/core/public/legacy/legacy_service.test.ts +++ b/src/core/public/legacy/legacy_service.test.ts @@ -58,7 +58,7 @@ import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; import { LegacyPlatformService } from './legacy_service'; import { applicationServiceMock } from '../application/application_service.mock'; import { docLinksServiceMock } from '../doc_links/doc_links_service.mock'; -import { savedObjectsMock } from '../saved_objects/saved_objects_service.mock'; +import { savedObjectsServiceMock } from '../saved_objects/saved_objects_service.mock'; import { contextServiceMock } from '../context/context_service.mock'; const applicationSetup = applicationServiceMock.createInternalSetupContract(); @@ -97,7 +97,7 @@ const injectedMetadataStart = injectedMetadataServiceMock.createStartContract(); const notificationsStart = notificationServiceMock.createStartContract(); const overlayStart = overlayServiceMock.createStartContract(); const uiSettingsStart = uiSettingsServiceMock.createStartContract(); -const savedObjectsStart = savedObjectsMock.createStartContract(); +const savedObjectsStart = savedObjectsServiceMock.createStartContract(); const fatalErrorsStart = fatalErrorsServiceMock.createStartContract(); const mockStorage = { getItem: jest.fn() } as any; diff --git a/src/core/public/mocks.ts b/src/core/public/mocks.ts index ce90d49065ad4..3301d71e2cdaf 100644 --- a/src/core/public/mocks.ts +++ b/src/core/public/mocks.ts @@ -26,7 +26,7 @@ import { i18nServiceMock } from './i18n/i18n_service.mock'; import { notificationServiceMock } from './notifications/notifications_service.mock'; import { overlayServiceMock } from './overlays/overlay_service.mock'; import { uiSettingsServiceMock } from './ui_settings/ui_settings_service.mock'; -import { savedObjectsMock } from './saved_objects/saved_objects_service.mock'; +import { savedObjectsServiceMock } from './saved_objects/saved_objects_service.mock'; import { contextServiceMock } from './context/context_service.mock'; import { injectedMetadataServiceMock } from './injected_metadata/injected_metadata_service.mock'; @@ -40,6 +40,7 @@ export { legacyPlatformServiceMock } from './legacy/legacy_service.mock'; export { notificationServiceMock } from './notifications/notifications_service.mock'; export { overlayServiceMock } from './overlays/overlay_service.mock'; export { uiSettingsServiceMock } from './ui_settings/ui_settings_service.mock'; +export { savedObjectsServiceMock } from './saved_objects/saved_objects_service.mock'; function createCoreSetupMock({ basePath = '' } = {}) { const mock = { @@ -70,7 +71,7 @@ function createCoreStartMock({ basePath = '' } = {}) { notifications: notificationServiceMock.createStartContract(), overlays: overlayServiceMock.createStartContract(), uiSettings: uiSettingsServiceMock.createStartContract(), - savedObjects: savedObjectsMock.createStartContract(), + savedObjects: savedObjectsServiceMock.createStartContract(), injectedMetadata: { getInjectedVar: injectedMetadataServiceMock.createStartContract().getInjectedVar, }, diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index dbbcda8d60e12..688eaf4f2bfc7 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -44,7 +44,7 @@ import { injectedMetadataServiceMock } from '../injected_metadata/injected_metad import { httpServiceMock } from '../http/http_service.mock'; import { CoreSetup, CoreStart, PluginInitializerContext } from '..'; import { docLinksServiceMock } from '../doc_links/doc_links_service.mock'; -import { savedObjectsMock } from '../saved_objects/saved_objects_service.mock'; +import { savedObjectsServiceMock } from '../saved_objects/saved_objects_service.mock'; import { contextServiceMock } from '../context/context_service.mock'; export let mockPluginInitializers: Map; @@ -110,7 +110,7 @@ describe('PluginsService', () => { notifications: notificationServiceMock.createStartContract(), overlays: overlayServiceMock.createStartContract(), uiSettings: uiSettingsServiceMock.createStartContract(), - savedObjects: savedObjectsMock.createStartContract(), + savedObjects: savedObjectsServiceMock.createStartContract(), fatalErrors: fatalErrorsServiceMock.createStartContract(), }; mockStartContext = { diff --git a/src/core/public/saved_objects/saved_objects_service.mock.ts b/src/core/public/saved_objects/saved_objects_service.mock.ts index 247e684a24b92..855bdf8314ec8 100644 --- a/src/core/public/saved_objects/saved_objects_service.mock.ts +++ b/src/core/public/saved_objects/saved_objects_service.mock.ts @@ -45,7 +45,7 @@ const createMock = () => { return mocked; }; -export const savedObjectsMock = { +export const savedObjectsServiceMock = { create: createMock, createStartContract: createStartContractMock, }; diff --git a/src/core/public/saved_objects/simple_saved_object.ts b/src/core/public/saved_objects/simple_saved_object.ts index 7978708c9eabb..8e464680bcf17 100644 --- a/src/core/public/saved_objects/simple_saved_object.ts +++ b/src/core/public/saved_objects/simple_saved_object.ts @@ -19,7 +19,7 @@ import { get, has, set } from 'lodash'; import { SavedObject as SavedObjectType, SavedObjectAttributes } from '../../server'; -import { SavedObjectsClient } from './saved_objects_client'; +import { SavedObjectsClientContract } from './saved_objects_client'; /** * This class is a very simple wrapper for SavedObjects loaded from the server @@ -41,7 +41,7 @@ export class SimpleSavedObject { public references: SavedObjectType['references']; constructor( - private client: SavedObjectsClient, + private client: SavedObjectsClientContract, { id, type, version, attributes, error, references, migrationVersion }: SavedObjectType ) { this.id = id; From 999af600db6817f72c476119bf8683461b8dc7ec Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 6 Feb 2020 13:22:22 +0100 Subject: [PATCH 2/4] Test docs for Saved Objects unit and integration tests --- src/core/TESTING.md | 242 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 231 insertions(+), 11 deletions(-) diff --git a/src/core/TESTING.md b/src/core/TESTING.md index 4dfab8830a506..7992d90e4f5eb 100644 --- a/src/core/TESTING.md +++ b/src/core/TESTING.md @@ -2,15 +2,34 @@ This document outlines best practices and patterns for testing Kibana Plugins. -- [Strategy](#strategy) -- [Core Integrations](#core-integrations) - - [Core Mocks](#core-mocks) +- [Testing Kibana Plugins](#testing-kibana-plugins) + - [Strategy](#strategy) + - [New concerns in the Kibana Platform](#new-concerns-in-the-kibana-platform) + - [Core Integrations](#core-integrations) + - [Core Mocks](#core-mocks) + - [Example](#example) - [Strategies for specific Core APIs](#strategies-for-specific-core-apis) - - [HTTP Routes](#http-routes) - - [SavedObjects](#savedobjects) - - [Elasticsearch](#elasticsearch) -- [Plugin Integrations](#plugin-integrations) -- [Plugin Contracts](#plugin-contracts) + - [HTTP Routes](#http-routes) + - [Preconditions](#preconditions) + - [Unit testing](#unit-testing) + - [Example](#example-1) + - [Integration tests](#integration-tests) + - [Functional Test Runner](#functional-test-runner) + - [Example](#example-2) + - [TestUtils](#testutils) + - [Example](#example-3) + - [Applications](#applications) + - [Example](#example-4) + - [SavedObjects](#savedobjects) + - [Unit Tests](#unit-tests) + - [Integration Tests](#integration-tests-1) + - [Elasticsearch](#elasticsearch) + - [Plugin integrations](#plugin-integrations) + - [Preconditions](#preconditions-1) + - [Testing dependencies usages](#testing-dependencies-usages) + - [Testing components consuming the dependencies](#testing-components-consuming-the-dependencies) + - [Testing optional plugin dependencies](#testing-optional-plugin-dependencies) + - [Plugin Contracts](#plugin-contracts) ## Strategy @@ -540,11 +559,212 @@ describe('renderApp', () => { }); ``` -#### SavedObjects +### SavedObjects -_How to test SO operations_ +#### Unit Tests -#### Elasticsearch +To unit test code that uses the Saved Objects client mock the client methods +and make assertions against the behaviour you would expect to see. + +Since the Saved Objects client makes network requests to an external +Elasticsearch cluster, it's important to include failure scenarios in your +test cases. + +When writing a view with which a user might interact, it's important to ensure +your code can recover from exceptions and provide a way for the user to +proceed. This behaviour should be tested as well. + +Below is an example of a Jest Unit test suite that mocks the server-side Saved +Objects client: + +```typescript +// src/plugins/myplugin/server/lib/short_url_lookup.ts +import crypto from 'crypto'; +import { SavedObjectsClientContract } from 'kibana/server'; + +export const shortUrlLookup = { + generateUrlId(url: string, savedObjectsClient: SavedObjectsClientContract) { + const id = crypto + .createHash('md5') + .update(url) + .digest('hex'); + + return savedObjectsClient + .create( + 'url', + { + url, + accessCount: 0, + createDate: new Date().valueOf(), + accessDate: new Date().valueOf(), + }, + { id } + ) + .then(doc => doc.id) + .catch(err => { + if (savedObjectsClient.errors.isConflictError(err)) { + return id; + } else { + throw err; + } + }); + }, +}; + +``` + +```typescript +// src/plugins/myplugin/server/lib/short_url_lookup.test.ts +import { shortUrlLookup } from './short_url_lookup'; +import { savedObjectsClientMock } from '../../../../../core/server/mocks'; + +describe('shortUrlLookup', () => { + const ID = 'bf00ad16941fc51420f91a93428b27a0'; + const TYPE = 'url'; + const URL = 'http://elastic.co'; + + const mockSavedObjectsClient = savedObjectsClientMock.create(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('generateUrlId', () => { + it('provides correct arguments to savedObjectsClient', async () => { + const ATTRIBUTES = { + url: URL, + accessCount: 0, + createDate: new Date().valueOf(), + accessDate: new Date().valueOf(), + }; + mockSavedObjectsClient.create.mockResolvedValueOnce({ + id: ID, + type: TYPE, + references: [], + attributes: ATTRIBUTES, + }); + await shortUrlLookup.generateUrlId(URL, mockSavedObjectsClient); + + expect(mockSavedObjectsClient.create).toHaveBeenCalledTimes(1); + const [type, attributes, options] = mockSavedObjectsClient.create.mock.calls[0]; + expect(type).toBe(TYPE); + expect(attributes).toStrictEqual(ATTRIBUTES); + expect(options).toStrictEqual({ id: ID }); + }); + + it('ignores version conflict and returns id', async () => { + mockSavedObjectsClient.create.mockRejectedValueOnce( + mockSavedObjectsClient.errors.decorateConflictError(new Error()) + ); + const id = await shortUrlLookup.generateUrlId(URL, mockSavedObjectsClient); + expect(id).toEqual(ID); + }); + + it('rejects with passed through savedObjectsClient errors', () => { + const error = new Error('oops'); + mockSavedObjectsClient.create.mockRejectedValueOnce(error); + return expect(shortUrlLookup.generateUrlId(URL, mockSavedObjectsClient)).rejects.toBe(error); + }); + }); +}); +``` + +The following is an example of a public saved object unit test. The biggest +difference with the server-side test is the slightly different Saved Objects +client API which returns `SimpleSavedObject` instances which needs to be +reflected in the mock. + +```typescript +// src/plugins/myplugin/public/saved_query_service.ts +import { SavedObjectsClientContract, SavedObjectAttributes } from 'src/core/public'; + +export const createSavedQueryService = (savedObjectsClient: SavedObjectsClientContract) => { + const saveQuery = async (attributes: SavedObjectAttributes) => { + try { + return await savedObjectsClient.create('query', attributes, { + id: attributes.title as string, + }); + } catch (err) { + throw new Error('Unable to create saved query, please try again.'); + } + }; + + return { + saveQuery, + }; +}; +``` + +```typescript +// src/plugins/myplugin/public/saved_query_service.test.ts +import { createSavedQueryService } from './saved_query_service'; +import { savedObjectsServiceMock } from '../../../core/public/mocks'; +import { SavedObjectsClientContract, SimpleSavedObject } from '../../../core/public'; + +describe('saved query service', () => { + const savedQueryAttributes = { + title: 'foo', + description: 'bar', + query: { + language: 'kuery', + query: 'response:200', + }, + }; + + const mockSavedObjectsClient = savedObjectsServiceMock.createStartContract() + .client as jest.Mocked; + + const savedQueryService = createSavedQueryService(mockSavedObjectsClient); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('saveQuery', function() { + it('should create a saved object for the given attributes', async () => { + // The public Saved Objects client returns instances of + // SimpleSavedObject, so we create an instance to return from our mock. + const mockReturnValue = new SimpleSavedObject(mockSavedObjectsClient, { + type: 'query', + id: 'foo', + attributes: savedQueryAttributes, + references: [], + }); + mockSavedObjectsClient.create.mockResolvedValue(mockReturnValue); + + const response = await savedQueryService.saveQuery(savedQueryAttributes); + expect(mockSavedObjectsClient.create).toHaveBeenCalledWith('query', savedQueryAttributes, { + id: 'foo', + }); + expect(response).toBe(mockReturnValue); + }); + + it('should reject with an error when saved objects client errors', () => { + mockSavedObjectsClient.create.mockRejectedValue(new Error('timeout')); + + return expect( + savedQueryService.saveQuery(savedQueryAttributes) + ).rejects.toMatchInlineSnapshot(`[Error: Unable to create saved query, please try again.]`); + }); + }); +}); +``` + +#### Integration Tests +To get the highest confidence in how your code behaves when using the Saved +Objects client, you should write at least a few integration tests which loads +data into and queries a real Elasticsearch database. + +To do that we'll write a Jest integration test using `TestUtils` to start +Kibana and esArchiver to load fixture data into Elasticsearch. + +1. Create the fixtures data you need in Elasticsearch +2. Create a fixtures archive with `node scripts/es_archiver save [index patterns...]` +3. Load the fixtures in your test using esArchiver `esArchiver.load('name')`; + +_todo: fully worked out example_ + +### Elasticsearch _How to test ES clients_ From bbc14393f5ec5ccb01cf51fefdcdb5d9e744c370 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 6 Feb 2020 13:55:28 +0100 Subject: [PATCH 3/4] Review comments --- src/core/TESTING.md | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/core/TESTING.md b/src/core/TESTING.md index 7992d90e4f5eb..aac54a4a14680 100644 --- a/src/core/TESTING.md +++ b/src/core/TESTING.md @@ -676,12 +676,27 @@ reflected in the mock. ```typescript // src/plugins/myplugin/public/saved_query_service.ts -import { SavedObjectsClientContract, SavedObjectAttributes } from 'src/core/public'; +import { + SavedObjectsClientContract, + SavedObjectAttributes, + SimpleSavedObject, +} from 'src/core/public'; + +export type SavedQueryAttributes = SavedObjectAttributes & { + title: string; + description: 'bar'; + query: { + language: 'kuery'; + query: 'response:200'; + }; +}; export const createSavedQueryService = (savedObjectsClient: SavedObjectsClientContract) => { - const saveQuery = async (attributes: SavedObjectAttributes) => { + const saveQuery = async ( + attributes: SavedQueryAttributes + ): Promise> => { try { - return await savedObjectsClient.create('query', attributes, { + return await savedObjectsClient.create('query', attributes, { id: attributes.title as string, }); } catch (err) { @@ -697,12 +712,12 @@ export const createSavedQueryService = (savedObjectsClient: SavedObjectsClientCo ```typescript // src/plugins/myplugin/public/saved_query_service.test.ts -import { createSavedQueryService } from './saved_query_service'; -import { savedObjectsServiceMock } from '../../../core/public/mocks'; -import { SavedObjectsClientContract, SimpleSavedObject } from '../../../core/public'; +import { createSavedQueryService, SavedQueryAttributes } from './saved_query_service'; +import { savedObjectsServiceMock } from '../../../../../core/public/mocks'; +import { SavedObjectsClientContract, SimpleSavedObject } from '../../../../../core/public'; describe('saved query service', () => { - const savedQueryAttributes = { + const savedQueryAttributes: SavedQueryAttributes = { title: 'foo', description: 'bar', query: { @@ -739,12 +754,17 @@ describe('saved query service', () => { expect(response).toBe(mockReturnValue); }); - it('should reject with an error when saved objects client errors', () => { + it('should reject with an error when saved objects client errors', async done => { mockSavedObjectsClient.create.mockRejectedValue(new Error('timeout')); - return expect( - savedQueryService.saveQuery(savedQueryAttributes) - ).rejects.toMatchInlineSnapshot(`[Error: Unable to create saved query, please try again.]`); + try { + await savedQueryService.saveQuery(savedQueryAttributes); + } catch (err) { + expect(err).toMatchInlineSnapshot( + `[Error: Unable to create saved query, please try again.]` + ); + done(); + } }); }); }); From d19dfc0f0099f9e36d4bab6e0e4725c52bef7346 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 6 Feb 2020 16:08:21 +0100 Subject: [PATCH 4/4] Update api types / docs --- .../kibana-plugin-public.simplesavedobject._constructor_.md | 4 ++-- src/core/public/public.api.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/development/core/public/kibana-plugin-public.simplesavedobject._constructor_.md b/docs/development/core/public/kibana-plugin-public.simplesavedobject._constructor_.md index f0769c0124d63..87d317da7a936 100644 --- a/docs/development/core/public/kibana-plugin-public.simplesavedobject._constructor_.md +++ b/docs/development/core/public/kibana-plugin-public.simplesavedobject._constructor_.md @@ -9,13 +9,13 @@ Constructs a new instance of the `SimpleSavedObject` class Signature: ```typescript -constructor(client: SavedObjectsClient, { id, type, version, attributes, error, references, migrationVersion }: SavedObjectType); +constructor(client: SavedObjectsClientContract, { id, type, version, attributes, error, references, migrationVersion }: SavedObjectType); ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| client | SavedObjectsClient | | +| client | SavedObjectsClientContract | | | { id, type, version, attributes, error, references, migrationVersion } | SavedObjectType<T> | | diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 91b4e9be58973..fb48524c20fb9 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -1170,7 +1170,7 @@ export interface SavedObjectsUpdateOptions { // @public export class SimpleSavedObject { - constructor(client: SavedObjectsClient, { id, type, version, attributes, error, references, migrationVersion }: SavedObject); + constructor(client: SavedObjectsClientContract, { id, type, version, attributes, error, references, migrationVersion }: SavedObject); // (undocumented) attributes: T; // (undocumented)