Improve tests

src/plugins/discover/public/__mocks__/index_pattern.ts

@@ -78,7 +78,7 @@ const indexPattern = {
   getFieldByName: jest.fn(() => ({})),
   timeFieldName: '',
   docvalueFields: [],
-  getFormatterForField: () => ({ convert: (value: unknown) => value }),
+  getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })),
 } as unknown as IndexPattern;
 
 indexPattern.isTimeBased = () => !!indexPattern.timeFieldName;

src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx

@@ -22,15 +22,7 @@ import { indexPatternMock } from '../../../../../__mocks__/index_pattern';
 
 jest.mock('../../../../../kibana_services', () => ({
   ...jest.requireActual('../../../../../kibana_services'),
-  getServices: () => ({
-    fieldFormats: {
-      getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })),
-      getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })),
-    },
-    uiSettings: {
-      get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50),
-    },
-  }),
+  getServices: () => discoverServiceMock,
 }));
 
 setHeaderActionMenuMounter(jest.fn());

src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx

@@ -18,18 +18,11 @@ import { uiSettingsMock } from '../../../__mocks__/ui_settings';
 import { DiscoverServices } from '../../../build_services';
 import { ElasticSearchHit } from '../../doc_views/doc_views_types';
 import { getDocId } from './discover_grid_document_selection';
+import { discoverServiceMock } from '../../../__mocks__/services';
 
 jest.mock('../../../kibana_services', () => ({
   ...jest.requireActual('../../../kibana_services'),
-  getServices: () => ({
-    fieldFormats: {
-      getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })),
-      getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })),
-    },
-    uiSettings: {
-      get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50),
-    },
-  }),
+  getServices: () => discoverServiceMock,
 }));
 
 function getProps() {

src/plugins/discover/public/application/helpers/format_hit.ts

@@ -13,12 +13,19 @@ import { getServices } from '../../kibana_services';
 import { formatFieldValue } from './format_value';
 
 // TODO: Test coverage
-// TODO: documentation
 
 const formattedHitCache = new WeakMap<estypes.SearchHit, FormattedHit>();
 
 type FormattedHit = Array<[fieldName: string, formattedValue: string]>;
 
+/**
+ * Returns a formatted document in form of key/value pairs of the fields name and a formatted value.
+ * The value returned in each pair is an HTML string which is safe to be applied to the DOM, since
+ * it's formatted using field formatters.
+ * @param hit The hit to format
+ * @param dataView The corresponding data view
+ * @param fieldsToShow A list of fields that should be included in the document summary.
+ */
 export function formatHit(
   hit: estypes.SearchHit,
   dataView: DataView,
@@ -30,15 +37,23 @@ export function formatHit(
   }
 
   const highlights = hit.highlight ?? {};
-  // Keys are sorted in the hits object
+  // Flatten the object using the flattenHit implementation we use across Discover for flattening documents.
   const flattened = flattenHit(hit, dataView, { includeIgnoredValues: true, source: true });
 
   const highlightPairs: Array<[fieldName: string, formattedValue: string]> = [];
   const sourcePairs: Array<[fieldName: string, formattedValue: string]> = [];
+
+  // Add each flattened field into the corresponding array for highlighted or other fields,
+  // depending on whether the original hit had a highlight for it. That way we can later
+  // put highlighted fields first in the document summary.
   Object.entries(flattened).forEach(([key, val]) => {
+    // Retrieve the (display) name of the fields, if it's a mapped field on the data view
     const displayKey = dataView.fields.getByName(key)?.displayName;
     const pairs = highlights[key] ? highlightPairs : sourcePairs;
+    // Format the raw value using the regular field formatters for that field
     const formattedValue = formatFieldValue(val, hit, dataView, dataView.fields.getByName(key));
+    // If the field was a mapped field, we validate it against the fieldsToShow list, if not
+    // we always include it into the result.
     if (displayKey) {
       if (fieldsToShow.includes(key)) {
         pairs.push([displayKey, formattedValue]);

src/plugins/discover/public/application/helpers/format_value.test.ts

@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+import type { FieldFormat } from '../../../../field_formats/common';
+import { indexPatternMock } from '../../__mocks__/index_pattern';
+import { formatFieldValue } from './format_value';
+
+import { getServices } from '../../kibana_services';
+
+jest.mock('../../kibana_services', () => {
+  const services = {
+    fieldFormats: {
+      getDefaultInstance: jest.fn<FieldFormat, [string]>(
+        () => ({ convert: (value: unknown) => value } as FieldFormat)
+      ),
+    },
+  };
+  return { getServices: () => services };
+});
+
+const hit = {
+  _id: '1',
+  _index: 'index',
+  fields: {
+    message: 'foo',
+  },
+};
+
+describe('formatFieldValue', () => {
+  afterEach(() => {
+    (indexPatternMock.getFormatterForField as jest.Mock).mockReset();
+    (getServices().fieldFormats.getDefaultInstance as jest.Mock).mockReset();
+  });
+
+  it('should call correct fieldFormatter for field', () => {
+    const formatterForFieldMock = indexPatternMock.getFormatterForField as jest.Mock;
+    const convertMock = jest.fn((value: unknown) => `formatted:${value}`);
+    formatterForFieldMock.mockReturnValue({ convert: convertMock });
+    const field = indexPatternMock.fields.getByName('message');
+    expect(formatFieldValue('foo', hit, indexPatternMock, field)).toBe('formatted:foo');
+    expect(indexPatternMock.getFormatterForField).toHaveBeenCalledWith(field);
+    expect(convertMock).toHaveBeenCalledWith('foo', 'html', { field, hit });
+  });
+
+  it('should call default string formatter if no field specified', () => {
+    const convertMock = jest.fn((value: unknown) => `formatted:${value}`);
+    (getServices().fieldFormats.getDefaultInstance as jest.Mock).mockReturnValue({
+      convert: convertMock,
+    });
+    expect(formatFieldValue('foo', hit, indexPatternMock)).toBe('formatted:foo');
+    expect(getServices().fieldFormats.getDefaultInstance).toHaveBeenCalledWith('string');
+    expect(convertMock).toHaveBeenCalledWith('foo', 'html', { field: undefined, hit });
+  });
+
+  it('should call default string formatter if no indexPattern is specified', () => {
+    const convertMock = jest.fn((value: unknown) => `formatted:${value}`);
+    (getServices().fieldFormats.getDefaultInstance as jest.Mock).mockReturnValue({
+      convert: convertMock,
+    });
+    expect(formatFieldValue('foo', hit)).toBe('formatted:foo');
+    expect(getServices().fieldFormats.getDefaultInstance).toHaveBeenCalledWith('string');
+    expect(convertMock).toHaveBeenCalledWith('foo', 'html', { field: undefined, hit });
+  });
+});

src/plugins/discover/public/application/helpers/format_value.ts

@@ -10,8 +10,6 @@ import { estypes } from '@elastic/elasticsearch';
 import { DataView, DataViewField, KBN_FIELD_TYPES } from '../../../../data/common';
 import { getServices } from '../../kibana_services';
 
-// TODO: need more test coverage
-
 /**
  * Formats the value of a specific field using the appropriate field formatter if available
  * or the default string field formatter otherwise.