Skip to content

Commit

Permalink
[Reporting] Clean up test helpers and mocks (#92550)
Browse files Browse the repository at this point in the history
* [Reporting] Clean up logger instances and mocks

* revert logging changes, just keep test changes

* remove fluff

* clean up too much logger.clone

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
tsullivan and kibanamachine authored Mar 1, 2021
1 parent b5cd44e commit 348472c
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 139 deletions.
110 changes: 53 additions & 57 deletions x-pack/plugins/reporting/server/config/create_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,22 @@
* 2.0.
*/

import * as Rx from 'rxjs';
import { CoreSetup, PluginInitializerContext } from 'src/core/server';
import { coreMock } from 'src/core/server/mocks';
import { LevelLogger } from '../lib';
import { createMockConfigSchema } from '../test_helpers';
import { createConfig$ } from './create_config';
import { ReportingConfigType } from './schema';

interface KibanaServer {
hostname?: string;
port?: number;
protocol?: string;
}

const makeMockInitContext = (config: {
capture?: Partial<ReportingConfigType['capture']>;
encryptionKey?: string;
kibanaServer: Partial<ReportingConfigType['kibanaServer']>;
}): PluginInitializerContext =>
({
config: {
create: () =>
Rx.of({
...config,
capture: config.capture || { browser: { chromium: { disableSandbox: false } } },
kibanaServer: config.kibanaServer || {},
}),
},
} as PluginInitializerContext);

const makeMockCoreSetup = (serverInfo: KibanaServer): CoreSetup =>
({ http: { getServerInfo: () => serverInfo } } as any);

describe('Reporting server createConfig$', () => {
let mockCoreSetup: CoreSetup;
let mockInitContext: PluginInitializerContext;
let mockLogger: LevelLogger;

beforeEach(() => {
mockCoreSetup = makeMockCoreSetup({ hostname: 'kibanaHost', port: 5601, protocol: 'http' });
mockInitContext = makeMockInitContext({
kibanaServer: {},
});
mockCoreSetup = coreMock.createSetup();
mockInitContext = coreMock.createPluginInitializerContext(
createMockConfigSchema({ kibanaServer: {} })
);
mockLogger = ({
warn: jest.fn(),
debug: jest.fn(),
Expand All @@ -58,14 +33,18 @@ describe('Reporting server createConfig$', () => {
});

it('creates random encryption key and default config using host, protocol, and port from server info', async () => {
mockInitContext = coreMock.createPluginInitializerContext({
...createMockConfigSchema({ kibanaServer: {} }),
encryptionKey: undefined,
});
const mockConfig$: any = mockInitContext.config.create();
const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise();

expect(result.encryptionKey).toMatch(/\S{32,}/); // random 32 characters
expect(result.kibanaServer).toMatchInlineSnapshot(`
Object {
"hostname": "kibanaHost",
"port": 5601,
"hostname": "localhost",
"port": 80,
"protocol": "http",
}
`);
Expand All @@ -76,25 +55,28 @@ describe('Reporting server createConfig$', () => {
});

it('uses the user-provided encryption key', async () => {
mockInitContext = makeMockInitContext({
encryptionKey: 'iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii',
kibanaServer: {},
});
mockInitContext = coreMock.createPluginInitializerContext(
createMockConfigSchema({
encryptionKey: 'iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii',
})
);
const mockConfig$: any = mockInitContext.config.create();
const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise();
expect(result.encryptionKey).toMatch('iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii');
expect((mockLogger.warn as any).mock.calls.length).toBe(0);
});

it('uses the user-provided encryption key, reporting kibanaServer settings to override server info', async () => {
mockInitContext = makeMockInitContext({
encryptionKey: 'iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii',
kibanaServer: {
hostname: 'reportingHost',
port: 5677,
protocol: 'httpsa',
},
});
mockInitContext = coreMock.createPluginInitializerContext(
createMockConfigSchema({
encryptionKey: 'iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii',
kibanaServer: {
hostname: 'reportingHost',
port: 5677,
protocol: 'httpsa',
},
})
);
const mockConfig$: any = mockInitContext.config.create();
const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise();

Expand All @@ -103,26 +85,36 @@ describe('Reporting server createConfig$', () => {
"capture": Object {
"browser": Object {
"chromium": Object {
"disableSandbox": false,
"disableSandbox": true,
},
},
},
"csv": Object {},
"encryptionKey": "iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii",
"index": ".reporting",
"kibanaServer": Object {
"hostname": "reportingHost",
"port": 5677,
"protocol": "httpsa",
},
"queue": Object {
"indexInterval": "week",
"pollEnabled": true,
"pollInterval": 3000,
"timeout": 120000,
},
}
`);
expect((mockLogger.warn as any).mock.calls.length).toBe(0);
});

it('uses user-provided disableSandbox: false', async () => {
mockInitContext = makeMockInitContext({
encryptionKey: '888888888888888888888888888888888',
capture: { browser: { chromium: { disableSandbox: false } } },
} as ReportingConfigType);
mockInitContext = coreMock.createPluginInitializerContext(
createMockConfigSchema({
encryptionKey: '888888888888888888888888888888888',
capture: { browser: { chromium: { disableSandbox: false } } },
})
);
const mockConfig$: any = mockInitContext.config.create();
const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise();

Expand All @@ -131,10 +123,12 @@ describe('Reporting server createConfig$', () => {
});

it('uses user-provided disableSandbox: true', async () => {
mockInitContext = makeMockInitContext({
encryptionKey: '888888888888888888888888888888888',
capture: { browser: { chromium: { disableSandbox: true } } },
} as ReportingConfigType);
mockInitContext = coreMock.createPluginInitializerContext(
createMockConfigSchema({
encryptionKey: '888888888888888888888888888888888',
capture: { browser: { chromium: { disableSandbox: true } } },
})
);
const mockConfig$: any = mockInitContext.config.create();
const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise();

Expand All @@ -143,9 +137,11 @@ describe('Reporting server createConfig$', () => {
});

it('provides a default for disableSandbox', async () => {
mockInitContext = makeMockInitContext({
encryptionKey: '888888888888888888888888888888888',
} as ReportingConfigType);
mockInitContext = coreMock.createPluginInitializerContext(
createMockConfigSchema({
encryptionKey: '888888888888888888888888888888888',
})
);
const mockConfig$: any = mockInitContext.config.create();
const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,16 @@ export const runTaskFnFactory: RunTaskFnFactory<
> = function executeJobFactoryFn(reporting, parentLogger) {
const config = reporting.getConfig();
const encryptionKey = config.get('encryptionKey');
const logger = parentLogger.clone([PNG_JOB_TYPE, 'execute']);

return async function runTask(jobId, job, cancellationToken) {
const apmTrans = apm.startTransaction('reporting execute_job png', 'reporting');
const apmGetAssets = apmTrans?.startSpan('get_assets', 'setup');
let apmGeneratePng: { end: () => void } | null | undefined;

const generatePngObservable = await generatePngObservableFactory(reporting);
const jobLogger = logger.clone([jobId]);
const jobLogger = parentLogger.clone([PNG_JOB_TYPE, 'execute', jobId]);
const process$: Rx.Observable<TaskRunResult> = Rx.of(1).pipe(
mergeMap(() => decryptJobHeaders(encryptionKey, job.headers, logger)),
mergeMap(() => decryptJobHeaders(encryptionKey, job.headers, jobLogger)),
map((decryptedHeaders) => omitBlockedHeaders(decryptedHeaders)),
map((filteredHeaders) => getConditionalHeaders(config, filteredHeaders)),
mergeMap((conditionalHeaders) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,19 @@ export const runTaskFnFactory: RunTaskFnFactory<
const encryptionKey = config.get('encryptionKey');

return async function runTask(jobId, job, cancellationToken) {
const logger = parentLogger.clone([PDF_JOB_TYPE, 'execute-job', jobId]);
const jobLogger = parentLogger.clone([PDF_JOB_TYPE, 'execute-job', jobId]);
const apmTrans = apm.startTransaction('reporting execute_job pdf', 'reporting');
const apmGetAssets = apmTrans?.startSpan('get_assets', 'setup');
let apmGeneratePdf: { end: () => void } | null | undefined;

const generatePdfObservable = await generatePdfObservableFactory(reporting);

const jobLogger = logger.clone([jobId]);
const process$: Rx.Observable<TaskRunResult> = Rx.of(1).pipe(
mergeMap(() => decryptJobHeaders(encryptionKey, job.headers, logger)),
mergeMap(() => decryptJobHeaders(encryptionKey, job.headers, jobLogger)),
map((decryptedHeaders) => omitBlockedHeaders(decryptedHeaders)),
map((filteredHeaders) => getConditionalHeaders(config, filteredHeaders)),
mergeMap((conditionalHeaders) =>
getCustomLogo(reporting, conditionalHeaders, job.spaceId, logger)
getCustomLogo(reporting, conditionalHeaders, job.spaceId, jobLogger)
),
mergeMap(({ logo, conditionalHeaders }) => {
const urls = getFullUrls(config, job);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import { createInterface } from 'readline';
import { setupServer } from 'src/core/server/test_utils';
import supertest from 'supertest';
import { ReportingCore } from '../..';
import { createMockLevelLogger, createMockReportingCore } from '../../test_helpers';
import {
createMockLevelLogger,
createMockPluginSetup,
createMockReportingCore,
} from '../../test_helpers';
import { registerDiagnoseBrowser } from './browser';
import type { ReportingRequestHandlerContext } from '../../types';

Expand Down Expand Up @@ -55,12 +59,12 @@ describe('POST /diagnose/browser', () => {
() => ({})
);

const mockSetupDeps = ({
const mockSetupDeps = createMockPluginSetup({
elasticsearch: {
legacy: { client: { callAsInternalUser: jest.fn() } },
},
router: httpSetup.createRouter(''),
} as unknown) as any;
});

core = await createMockReportingCore(config, mockSetupDeps);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { UnwrapPromise } from '@kbn/utility-types';
import { setupServer } from 'src/core/server/test_utils';
import supertest from 'supertest';
import { ReportingCore } from '../..';
import { createMockReportingCore, createMockLevelLogger } from '../../test_helpers';
import {
createMockReportingCore,
createMockLevelLogger,
createMockPluginSetup,
} from '../../test_helpers';
import { registerDiagnoseConfig } from './config';
import type { ReportingRequestHandlerContext } from '../../types';

Expand All @@ -33,7 +37,7 @@ describe('POST /diagnose/config', () => {
() => ({})
);

mockSetupDeps = ({
mockSetupDeps = createMockPluginSetup({
elasticsearch: {
legacy: { client: { callAsInternalUser: jest.fn() } },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { UnwrapPromise } from '@kbn/utility-types';
import { setupServer } from 'src/core/server/test_utils';
import supertest from 'supertest';
import { ReportingCore } from '../..';
import { createMockReportingCore, createMockLevelLogger } from '../../test_helpers';
import {
createMockReportingCore,
createMockLevelLogger,
createMockPluginSetup,
} from '../../test_helpers';
import { registerDiagnoseScreenshot } from './screenshot';
import type { ReportingRequestHandlerContext } from '../../types';

Expand Down Expand Up @@ -52,12 +56,12 @@ describe('POST /diagnose/screenshot', () => {
() => ({})
);

const mockSetupDeps = ({
const mockSetupDeps = createMockPluginSetup({
elasticsearch: {
legacy: { client: { callAsInternalUser: jest.fn() } },
},
router: httpSetup.createRouter(''),
} as unknown) as any;
});

core = await createMockReportingCore(config, mockSetupDeps);
});
Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/reporting/server/routes/generation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import { setupServer } from 'src/core/server/test_utils';
import supertest from 'supertest';
import { ReportingCore } from '..';
import { ExportTypesRegistry } from '../lib/export_types_registry';
import { createMockReportingCore, createMockLevelLogger } from '../test_helpers';
import { createMockLevelLogger, createMockReportingCore } from '../test_helpers';
import { createMockPluginSetup } from '../test_helpers/create_mock_reportingplugin';
import { registerJobGenerationRoutes } from './generation';
import type { ReportingRequestHandlerContext } from '../types';

Expand All @@ -37,7 +38,7 @@ describe('POST /api/reporting/generate', () => {
case 'index':
return '.reporting';
case 'queue.pollEnabled':
return false;
return true;
default:
return;
}
Expand All @@ -56,7 +57,7 @@ describe('POST /api/reporting/generate', () => {

callClusterStub = sinon.stub().resolves({});

const mockSetupDeps = ({
const mockSetupDeps = createMockPluginSetup({
elasticsearch: {
legacy: { client: { callAsInternalUser: callClusterStub } },
},
Expand All @@ -68,7 +69,7 @@ describe('POST /api/reporting/generate', () => {
},
router: httpSetup.createRouter(''),
licensing: { license$: of({ isActive: true, isAvailable: true, type: 'gold' }) },
} as unknown) as any;
});

core = await createMockReportingCore(config, mockSetupDeps);

Expand Down
13 changes: 10 additions & 3 deletions x-pack/plugins/reporting/server/routes/jobs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ import supertest from 'supertest';
import { ReportingCore } from '..';
import { ReportingInternalSetup } from '../core';
import { ExportTypesRegistry } from '../lib/export_types_registry';
import { createMockConfig, createMockConfigSchema, createMockReportingCore } from '../test_helpers';
import {
createMockConfig,
createMockConfigSchema,
createMockPluginSetup,
createMockReportingCore,
} from '../test_helpers';
import { ExportTypeDefinition, ReportingRequestHandlerContext } from '../types';
import { registerJobInfoRoutes } from './jobs';

Expand Down Expand Up @@ -41,7 +46,7 @@ describe('GET /api/reporting/jobs/download', () => {
'reporting',
() => ({})
);
core = await createMockReportingCore(config, ({
const mockSetupDeps = createMockPluginSetup({
elasticsearch: {
legacy: { client: { callAsInternalUser: jest.fn() } },
},
Expand All @@ -65,7 +70,9 @@ describe('GET /api/reporting/jobs/download', () => {
type: 'gold',
}),
},
} as unknown) as ReportingInternalSetup);
});

core = await createMockReportingCore(config, mockSetupDeps);
// @ts-ignore
exportTypesRegistry = new ExportTypesRegistry();
exportTypesRegistry.register({
Expand Down
Loading

0 comments on commit 348472c

Please sign in to comment.