Saved Objects testing (#56965)

* Expose core/public savedObjectsServiceMock

* Test docs for Saved Objects unit and integration tests

* Review comments

* Update api types / docs

docs/development/core/public/kibana-plugin-public.simplesavedobject._constructor_.md

@@ -9,13 +9,13 @@ Constructs a new instance of the `SimpleSavedObject` class
 <b>Signature:</b>
 
 ```typescript
-constructor(client: SavedObjectsClient, { id, type, version, attributes, error, references, migrationVersion }: SavedObjectType<T>);
+constructor(client: SavedObjectsClientContract, { id, type, version, attributes, error, references, migrationVersion }: SavedObjectType<T>);
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
-|  client | <code>SavedObjectsClient</code> |  |
+|  client | <code>SavedObjectsClientContract</code> |  |
 |  { id, type, version, attributes, error, references, migrationVersion } | <code>SavedObjectType&lt;T&gt;</code> |  |
 

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,232 @@ 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,
+  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: SavedQueryAttributes
+  ): Promise<SimpleSavedObject<SavedQueryAttributes>> => {
+    try {
+      return await savedObjectsClient.create<SavedQueryAttributes>('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, SavedQueryAttributes } from './saved_query_service';
+import { savedObjectsServiceMock } from '../../../../../core/public/mocks';
+import { SavedObjectsClientContract, SimpleSavedObject } from '../../../../../core/public';
+
+describe('saved query service', () => {
+  const savedQueryAttributes: SavedQueryAttributes = {
+    title: 'foo',
+    description: 'bar',
+    query: {
+      language: 'kuery',
+      query: 'response:200',
+    },
+  };
+
+  const mockSavedObjectsClient = savedObjectsServiceMock.createStartContract()
+    .client as jest.Mocked<SavedObjectsClientContract>;
+
+  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', async done => {
+      mockSavedObjectsClient.create.mockRejectedValue(new Error('timeout'));
+
+      try {
+        await savedQueryService.saveQuery(savedQueryAttributes);
+      } catch (err) {
+        expect(err).toMatchInlineSnapshot(
+          `[Error: Unable to create saved query, please try again.]`
+        );
+        done();
+      }
+    });
+  });
+});
+```
+
+#### 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 <name> [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_
 

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;
 

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,
     },

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<PluginName, MockedPluginInitializer>;
@@ -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 = {

src/core/public/public.api.md

@@ -1170,7 +1170,7 @@ export interface SavedObjectsUpdateOptions {
 
 // @public
 export class SimpleSavedObject<T extends SavedObjectAttributes> {
-    constructor(client: SavedObjectsClient, { id, type, version, attributes, error, references, migrationVersion }: SavedObject<T>);
+    constructor(client: SavedObjectsClientContract, { id, type, version, attributes, error, references, migrationVersion }: SavedObject<T>);
     // (undocumented)
     attributes: T;
     // (undocumented)

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,
 };

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<T extends SavedObjectAttributes> {
   public references: SavedObjectType<T>['references'];
 
   constructor(
-    private client: SavedObjectsClient,
+    private client: SavedObjectsClientContract,
     { id, type, version, attributes, error, references, migrationVersion }: SavedObjectType<T>
   ) {
     this.id = id;