Skip to content

Commit

Permalink
[Reporting] Clean up test helpers and mocks (elastic#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 <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
tsullivan and kibanamachine committed Mar 1, 2021
1 parent 024bd24 commit 123240f
Show file tree
Hide file tree
Showing 15 changed files with 114 additions and 149 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
6 changes: 0 additions & 6 deletions x-pack/plugins/reporting/server/lib/store/report.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('Class Report', () => {
payload: { headers: 'payload_test_field', objectType: 'testOt', title: 'cool report' },
meta: { objectType: 'test' },
timeout: 30000,
priority: 1,
});

expect(report.toEsDocsJSON()).toMatchObject({
Expand All @@ -32,7 +31,6 @@ describe('Class Report', () => {
max_attempts: 50,
meta: { objectType: 'test' },
payload: { headers: 'payload_test_field', objectType: 'testOt' },
priority: 1,
started_at: undefined,
status: 'pending',
timeout: 30000,
Expand All @@ -47,7 +45,6 @@ describe('Class Report', () => {
max_attempts: 50,
payload: { headers: 'payload_test_field', objectType: 'testOt' },
meta: { objectType: 'test' },
priority: 1,
status: 'pending',
timeout: 30000,
});
Expand All @@ -65,7 +62,6 @@ describe('Class Report', () => {
payload: { headers: 'payload_test_field', objectType: 'testOt', title: 'hot report' },
meta: { objectType: 'stange' },
timeout: 30000,
priority: 1,
});

const metadata = {
Expand All @@ -88,7 +84,6 @@ describe('Class Report', () => {
max_attempts: 50,
meta: { objectType: 'stange' },
payload: { objectType: 'testOt' },
priority: 1,
started_at: undefined,
status: 'pending',
timeout: 30000,
Expand All @@ -105,7 +100,6 @@ describe('Class Report', () => {
max_attempts: 50,
meta: { objectType: 'stange' },
payload: { headers: 'payload_test_field', objectType: 'testOt' },
priority: 1,
started_at: undefined,
status: 'pending',
timeout: 30000,
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/reporting/server/lib/store/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export class Report implements Partial<ReportSource> {
public readonly started_at?: ReportSource['started_at'];
public readonly completed_at?: ReportSource['completed_at'];
public readonly process_expiration?: ReportSource['process_expiration'];
public readonly priority?: ReportSource['priority'];
public readonly timeout?: ReportSource['timeout'];

/*
Expand All @@ -63,7 +62,6 @@ export class Report implements Partial<ReportSource> {
this.created_by = opts.created_by || false;
this.meta = opts.meta || { objectType: 'unknown' };
this.browser_type = opts.browser_type;
this.priority = opts.priority;

this.status = opts.status || JOB_STATUSES.PENDING;
this.output = opts.output || null;
Expand Down Expand Up @@ -98,7 +96,6 @@ export class Report implements Partial<ReportSource> {
meta: this.meta,
timeout: this.timeout,
max_attempts: this.max_attempts,
priority: this.priority,
browser_type: this.browser_type,
status: this.status,
attempts: this.attempts,
Expand All @@ -124,7 +121,6 @@ export class Report implements Partial<ReportSource> {
meta: this.meta,
timeout: this.timeout,
max_attempts: this.max_attempts,
priority: this.priority,
browser_type: this.browser_type,
status: this.status,
attempts: this.attempts,
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/reporting/server/lib/store/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ describe('ReportingStore', () => {
browserTimezone: 'ABC',
},
timeout: 30000,
priority: 1,
});

await store.setReportClaimed(report, { testDoc: 'test' } as any);
Expand Down Expand Up @@ -244,7 +243,6 @@ describe('ReportingStore', () => {
browserTimezone: 'BCD',
},
timeout: 30000,
priority: 1,
});

await store.setReportFailed(report, { errors: 'yes' } as any);
Expand Down Expand Up @@ -285,7 +283,6 @@ describe('ReportingStore', () => {
browserTimezone: 'CDE',
},
timeout: 30000,
priority: 1,
});

await store.setReportCompleted(report, { certainly_completed: 'yes' } as any);
Expand Down Expand Up @@ -326,7 +323,6 @@ describe('ReportingStore', () => {
browserTimezone: 'utc',
},
timeout: 30000,
priority: 1,
});

await store.setReportCompleted(report, {
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
Loading

0 comments on commit 123240f

Please sign in to comment.