From 9e654f1106110bfae0d71f9e3ce6e54bf4b70d27 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 4 Jun 2020 16:30:55 -0700 Subject: [PATCH 1/8] [Reporting] Convert plugin setup and start to synchronous --- .../browsers/chromium/driver_factory/index.ts | 9 +- .../server/browsers/chromium/index.ts | 16 ++-- .../browsers/create_browser_driver_factory.ts | 45 ---------- .../browsers/download/ensure_downloaded.ts | 2 +- .../reporting/server/browsers/index.ts | 32 +++++-- .../reporting/server/browsers/install.ts | 71 ++++++++++------ .../plugins/reporting/server/config/config.ts | 15 ++-- .../reporting/server/config/create_config.ts | 3 +- .../plugins/reporting/server/config/index.ts | 1 - x-pack/plugins/reporting/server/core.ts | 11 ++- .../reporting/server/lib/validate/index.ts | 5 +- x-pack/plugins/reporting/server/plugin.ts | 83 ++++++++++--------- .../create_mock_browserdriverfactory.ts | 10 ++- 13 files changed, 161 insertions(+), 142 deletions(-) delete mode 100644 x-pack/plugins/reporting/server/browsers/create_browser_driver_factory.ts diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts index 246c605f4bfe6..3ce5329e42517 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts @@ -28,22 +28,25 @@ import { getChromeLogLocation } from '../paths'; import { puppeteerLaunch } from '../puppeteer'; import { args } from './args'; -type binaryPath = string; type BrowserConfig = CaptureConfig['browser']['chromium']; type ViewportConfig = CaptureConfig['viewport']; export class HeadlessChromiumDriverFactory { - private binaryPath: binaryPath; + private binaryPath: string; private captureConfig: CaptureConfig; private browserConfig: BrowserConfig; private userDataDir: string; private getChromiumArgs: (viewport: ViewportConfig) => string[]; - constructor(binaryPath: binaryPath, logger: LevelLogger, captureConfig: CaptureConfig) { + constructor(binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) { this.binaryPath = binaryPath; this.captureConfig = captureConfig; this.browserConfig = captureConfig.browser.chromium; + if (this.browserConfig.disableSandbox) { + logger.warning(`Enabling the Chromium sandbox provides an additional layer of protection.`); + } + this.userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chromium-')); this.getChromiumArgs = (viewport: ViewportConfig) => args({ diff --git a/x-pack/plugins/reporting/server/browsers/chromium/index.ts b/x-pack/plugins/reporting/server/browsers/chromium/index.ts index 5f89662c94da2..cebcd228b01c3 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/index.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/index.ts @@ -4,16 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ +import { BrowserDownload } from '../'; import { CaptureConfig } from '../../../server/types'; import { LevelLogger } from '../../lib'; import { HeadlessChromiumDriverFactory } from './driver_factory'; +import { paths } from './paths'; -export { paths } from './paths'; - -export async function createDriverFactory( - binaryPath: string, - logger: LevelLogger, - captureConfig: CaptureConfig -): Promise { - return new HeadlessChromiumDriverFactory(binaryPath, logger, captureConfig); -} +export const chromium: BrowserDownload = { + paths, + createDriverFactory: (binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) => + new HeadlessChromiumDriverFactory(binaryPath, captureConfig, logger), +}; diff --git a/x-pack/plugins/reporting/server/browsers/create_browser_driver_factory.ts b/x-pack/plugins/reporting/server/browsers/create_browser_driver_factory.ts deleted file mode 100644 index f3486a48ba7b1..0000000000000 --- a/x-pack/plugins/reporting/server/browsers/create_browser_driver_factory.ts +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { first } from 'rxjs/operators'; -import { ReportingConfig } from '../'; -import { LevelLogger } from '../lib'; -import { HeadlessChromiumDriverFactory } from './chromium/driver_factory'; -import { ensureBrowserDownloaded } from './download'; -import { chromium } from './index'; -import { installBrowser } from './install'; - -export async function createBrowserDriverFactory( - config: ReportingConfig, - logger: LevelLogger -): Promise { - const captureConfig = config.get('capture'); - const browserConfig = captureConfig.browser.chromium; - const browserAutoDownload = captureConfig.browser.autoDownload; - const browserType = captureConfig.browser.type; - const dataDir = await config.kbnConfig.get('path', 'data').pipe(first()).toPromise(); - - if (browserConfig.disableSandbox) { - logger.warning(`Enabling the Chromium sandbox provides an additional layer of protection.`); - } - if (browserAutoDownload) { - await ensureBrowserDownloaded(browserType, logger); - } - - try { - const { binaryPath } = await installBrowser(logger, chromium, dataDir); - return chromium.createDriverFactory(binaryPath, logger, captureConfig); - } catch (error) { - if (error.cause && ['EACCES', 'EEXIST'].includes(error.cause.code)) { - logger.error( - `Error code ${error.cause.code}: Insufficient permissions for extracting the browser archive. ` + - `Make sure the Kibana data directory (path.data) is owned by the same user that is running Kibana.` - ); - } - - throw error; // reject the promise with the original error - } -} diff --git a/x-pack/plugins/reporting/server/browsers/download/ensure_downloaded.ts b/x-pack/plugins/reporting/server/browsers/download/ensure_downloaded.ts index b334510d71947..add14448e2f1d 100644 --- a/x-pack/plugins/reporting/server/browsers/download/ensure_downloaded.ts +++ b/x-pack/plugins/reporting/server/browsers/download/ensure_downloaded.ts @@ -56,7 +56,7 @@ async function ensureDownloaded(browsers: BrowserDownload[], logger: LevelLogger const path = resolvePath(archivesPath, archiveFilename); if (existsSync(path) && (await md5(path)) === archiveChecksum) { - logger.info(`Browser archive exists in ${path}`); + logger.debug(`Browser archive exists in ${path}`); return; } diff --git a/x-pack/plugins/reporting/server/browsers/index.ts b/x-pack/plugins/reporting/server/browsers/index.ts index 7f6e40fb433b6..966b000f83ea3 100644 --- a/x-pack/plugins/reporting/server/browsers/index.ts +++ b/x-pack/plugins/reporting/server/browsers/index.ts @@ -4,20 +4,28 @@ * you may not use this file except in compliance with the Elastic License. */ -import * as chromiumDefinition from './chromium'; +import { first } from 'rxjs/operators'; +import { LevelLogger } from '../lib'; +import { CaptureConfig } from '../types'; +import { chromium } from './chromium'; +import { HeadlessChromiumDriverFactory } from './chromium/driver_factory'; +import { BrowserInstaller } from './install'; +import { ReportingConfig } from '..'; +export { BrowserInstaller } from './install'; export { ensureAllBrowsersDownloaded } from './download'; -export { createBrowserDriverFactory } from './create_browser_driver_factory'; - export { HeadlessChromiumDriver } from './chromium/driver'; export { HeadlessChromiumDriverFactory } from './chromium/driver_factory'; +export { chromium } from './chromium'; -export const chromium = { - paths: chromiumDefinition.paths, - createDriverFactory: chromiumDefinition.createDriverFactory, -}; +type CreateDriverFactory = ( + binaryPath: string, + captureConfig: CaptureConfig, + logger: LevelLogger +) => HeadlessChromiumDriverFactory; export interface BrowserDownload { + createDriverFactory: CreateDriverFactory; paths: { archivesPath: string; baseUrl: string; @@ -30,3 +38,13 @@ export interface BrowserDownload { }>; }; } + +export const initializeBrowserDriverFactory = async ( + config: ReportingConfig, + logger: LevelLogger +) => { + const { binaryPath$ } = new BrowserInstaller(chromium, config, logger); + const binaryPath = await binaryPath$.pipe(first()).toPromise(); + const captureConfig = config.get('capture'); + return chromium.createDriverFactory(binaryPath, captureConfig, logger); +}; diff --git a/x-pack/plugins/reporting/server/browsers/install.ts b/x-pack/plugins/reporting/server/browsers/install.ts index 01526af307022..ca51df5730162 100644 --- a/x-pack/plugins/reporting/server/browsers/install.ts +++ b/x-pack/plugins/reporting/server/browsers/install.ts @@ -6,9 +6,13 @@ import fs from 'fs'; import path from 'path'; +import * as Rx from 'rxjs'; +import { first } from 'rxjs/operators'; import { promisify } from 'util'; +import { ReportingConfig } from '../'; import { LevelLogger } from '../lib'; import { BrowserDownload } from './'; +import { ensureBrowserDownloaded } from './download'; // @ts-ignore import { md5 } from './download/checksum'; // @ts-ignore @@ -19,37 +23,56 @@ const chmod = promisify(fs.chmod); interface Package { platforms: string[]; } -interface PathResponse { - binaryPath: string; -} /** * "install" a browser by type into installs path by extracting the downloaded * archive. If there is an error extracting the archive an `ExtractError` is thrown */ -export async function installBrowser( - logger: LevelLogger, - browser: BrowserDownload, - installsPath: string -): Promise { - const pkg = browser.paths.packages.find((p: Package) => p.platforms.includes(process.platform)); - - if (!pkg) { - throw new Error(`Unsupported platform: ${JSON.stringify(browser, null, 2)}`); +export class BrowserInstaller { + public readonly binaryPath$: Rx.Subject; + + constructor(browser: BrowserDownload, config: ReportingConfig, logger: LevelLogger) { + this.binaryPath$ = new Rx.Subject(); + this.backgroundInstall(browser, config, logger); } - const binaryPath = path.join(installsPath, pkg.binaryRelativePath); - const binaryChecksum = await md5(binaryPath).catch(() => ''); + async backgroundInstall(browser: BrowserDownload, config: ReportingConfig, logger: LevelLogger) { + const captureConfig = config.get('capture'); + const { autoDownload, type: browserType } = captureConfig.browser; + if (autoDownload) { + await ensureBrowserDownloaded(browserType, logger); + } - if (binaryChecksum !== pkg.binaryChecksum) { - const archive = path.join(browser.paths.archivesPath, pkg.archiveFilename); - logger.debug(`Extracting [${archive}] to [${binaryPath}]`); - await extract(archive, installsPath); - await chmod(binaryPath, '755'); - } + const pkg = browser.paths.packages.find((p: Package) => p.platforms.includes(process.platform)); + if (!pkg) { + throw new Error(`Unsupported platform: ${JSON.stringify(browser, null, 2)}`); + } + + const dataDir = await config.kbnConfig.get('path', 'data').pipe(first()).toPromise(); + const binaryPath = path.join(dataDir, pkg.binaryRelativePath); + + try { + const binaryChecksum = await md5(binaryPath).catch(() => ''); - logger.debug(`Browser installed at ${binaryPath}`); - return { - binaryPath, - }; + if (binaryChecksum !== pkg.binaryChecksum) { + const archive = path.join(browser.paths.archivesPath, pkg.archiveFilename); + logger.info(`Extracting [${archive}] to [${binaryPath}]`); + await extract(archive, dataDir); + await chmod(binaryPath, '755'); + } + } catch (error) { + if (error.cause && ['EACCES', 'EEXIST'].includes(error.cause.code)) { + logger.error( + `Error code ${error.cause.code}: Insufficient permissions for extracting the browser archive. ` + + `Make sure the Kibana data directory (path.data) is owned by the same user that is running Kibana.` + ); + } + + throw error; // reject the promise with the original error + } + + logger.debug(`Browser executable: ${binaryPath}`); + + this.binaryPath$.next(binaryPath); // subscribers wait for download and extract to complete + } } diff --git a/x-pack/plugins/reporting/server/config/config.ts b/x-pack/plugins/reporting/server/config/config.ts index 4142ab6f0ae43..2a09ebea9619c 100644 --- a/x-pack/plugins/reporting/server/config/config.ts +++ b/x-pack/plugins/reporting/server/config/config.ts @@ -4,10 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Observable } from 'rxjs'; import { get } from 'lodash'; -import { map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; +import { first, map } from 'rxjs/operators'; import { CoreSetup, PluginInitializerContext } from 'src/core/server'; +import { LevelLogger } from '../lib'; +import { createConfig$ } from './create_config'; import { ReportingConfigType } from './schema'; // make config.get() aware of the value type it returns @@ -55,11 +57,12 @@ export interface ReportingConfig extends Config { kbnConfig: Config; } -export const buildConfig = ( +export const buildConfig = async ( initContext: PluginInitializerContext, core: CoreSetup, - reportingConfig: ReportingConfigType -): ReportingConfig => { + logger: LevelLogger +): Promise => { + const config$ = initContext.config.create(); const { http } = core; const serverInfo = http.getServerInfo(); @@ -77,6 +80,8 @@ export const buildConfig = ( }, }; + const reportingConfig$ = createConfig$(core, config$, logger); + const reportingConfig = await reportingConfig$.pipe(first()).toPromise(); return { get: (...keys: string[]) => get(reportingConfig, keys.join('.'), null), // spreading arguments as an array allows the return type to be known by the compiler kbnConfig: { diff --git a/x-pack/plugins/reporting/server/config/create_config.ts b/x-pack/plugins/reporting/server/config/create_config.ts index 67df7dacc77ab..5c66bd599dd9a 100644 --- a/x-pack/plugins/reporting/server/config/create_config.ts +++ b/x-pack/plugins/reporting/server/config/create_config.ts @@ -23,8 +23,9 @@ import { ReportingConfigType } from './schema'; export function createConfig$( core: CoreSetup, config$: Observable, - logger: LevelLogger + parentLogger: LevelLogger ) { + const logger = parentLogger.clone(['config']); return config$.pipe( map((config) => { // encryption key diff --git a/x-pack/plugins/reporting/server/config/index.ts b/x-pack/plugins/reporting/server/config/index.ts index caa64a7414005..a89b952702e1b 100644 --- a/x-pack/plugins/reporting/server/config/index.ts +++ b/x-pack/plugins/reporting/server/config/index.ts @@ -7,7 +7,6 @@ import { PluginConfigDescriptor } from 'kibana/server'; import { ConfigSchema, ReportingConfigType } from './schema'; export { buildConfig } from './config'; -export { createConfig$ } from './create_config'; export { ConfigSchema, ReportingConfigType }; export const config: PluginConfigDescriptor = { diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index e7786b3b753fb..94b138ffcae0b 100644 --- a/x-pack/plugins/reporting/server/core.ts +++ b/x-pack/plugins/reporting/server/core.ts @@ -26,7 +26,6 @@ import { ESQueueInstance } from './lib/create_queue'; import { EnqueueJobFn } from './lib/enqueue_job'; export interface ReportingInternalSetup { - browserDriverFactory: HeadlessChromiumDriverFactory; elasticsearch: ElasticsearchServiceSetup; licensing: LicensingPluginSetup; basePath: BasePath['get']; @@ -44,6 +43,7 @@ interface ReportingInternalStart { export class ReportingCore { private pluginSetupDeps?: ReportingInternalSetup; private pluginStartDeps?: ReportingInternalStart; + private browserDriverFactory?: HeadlessChromiumDriverFactory; private readonly pluginSetup$ = new Rx.ReplaySubject(); private readonly pluginStart$ = new Rx.ReplaySubject(); private exportTypesRegistry = getExportTypesRegistry(); @@ -63,6 +63,10 @@ export class ReportingCore { return this.pluginStart$.pipe(first(), mapTo(true)).toPromise(); } + public setBrowserDriverFactory(browserDriverFactory: HeadlessChromiumDriverFactory) { + this.browserDriverFactory = browserDriverFactory; + } + /* * Internal module dependencies */ @@ -93,7 +97,10 @@ export class ReportingCore { } public getScreenshotsObservable(): ScreenshotsObservableFn { - const { browserDriverFactory } = this.getPluginSetupDeps(); + const { browserDriverFactory } = this; + if (!browserDriverFactory) { + throw new Error(`"browserDriverFactory" dependency hasn't initialized yet`); + } return screenshotsObservableFactory(this.config.get('capture'), browserDriverFactory); } diff --git a/x-pack/plugins/reporting/server/lib/validate/index.ts b/x-pack/plugins/reporting/server/lib/validate/index.ts index 404cbcda31a09..7c439d6023d5f 100644 --- a/x-pack/plugins/reporting/server/lib/validate/index.ts +++ b/x-pack/plugins/reporting/server/lib/validate/index.ts @@ -7,8 +7,8 @@ import { i18n } from '@kbn/i18n'; import { ElasticsearchServiceSetup } from 'kibana/server'; import { ReportingConfig } from '../../'; -import { LevelLogger } from '../../lib'; import { HeadlessChromiumDriverFactory } from '../../browsers/chromium/driver_factory'; +import { LevelLogger } from '../../lib'; import { validateBrowser } from './validate_browser'; import { validateMaxContentLength } from './validate_max_content_length'; @@ -16,8 +16,9 @@ export async function runValidations( config: ReportingConfig, elasticsearch: ElasticsearchServiceSetup, browserFactory: HeadlessChromiumDriverFactory, - logger: LevelLogger + parentLogger: LevelLogger ) { + const logger = parentLogger.clone(['validations']); try { await Promise.all([ validateBrowser(browserFactory, logger), diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index d0d25f6d9e0ae..ed4939fcc74f3 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -4,13 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Observable } from 'rxjs'; -import { first } from 'rxjs/operators'; import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/server'; -import { ReportingCore } from './core'; -import { ReportingConfigType } from './config'; -import { createBrowserDriverFactory } from './browsers'; -import { buildConfig, createConfig$ } from './config'; +import { ReportingCore } from './'; +import { initializeBrowserDriverFactory } from './browsers'; +import { buildConfig, ReportingConfigType } from './config'; import { createQueueFactory, enqueueJobFactory, LevelLogger, runValidations } from './lib'; import { registerRoutes } from './routes'; import { setFieldFormats } from './services'; @@ -22,61 +19,69 @@ export class ReportingPlugin private readonly initializerContext: PluginInitializerContext; private logger: LevelLogger; private reportingCore?: ReportingCore; - private config$: Observable; constructor(context: PluginInitializerContext) { this.logger = new LevelLogger(context.logger.get()); this.initializerContext = context; - this.config$ = context.config.create(); } - public async setup(core: CoreSetup, plugins: ReportingSetupDeps) { + public setup(core: CoreSetup, plugins: ReportingSetupDeps) { const { elasticsearch, http } = core; const { licensing, security } = plugins; const { initializerContext: initContext } = this; const router = http.createRouter(); const basePath = http.basePath.get; - const coreConfig = await createConfig$(core, this.config$, this.logger) - .pipe(first()) - .toPromise(); // apply computed defaults to config - const reportingConfig = buildConfig(initContext, core, coreConfig); // combine kbnServer configs - this.reportingCore = new ReportingCore(reportingConfig); - - const browserDriverFactory = await createBrowserDriverFactory(reportingConfig, this.logger); - - this.reportingCore.pluginSetup({ - browserDriverFactory, - elasticsearch, - licensing, - basePath, - router, - security, - }); + // async background setup + buildConfig(initContext, core, this.logger).then((config) => { + const reportingCore = new ReportingCore(config); + + reportingCore.pluginSetup({ + elasticsearch, + licensing, + basePath, + router, + security, + }); + + registerReportingUsageCollector(reportingCore, plugins); + registerRoutes(reportingCore, this.logger); + this.reportingCore = reportingCore; - runValidations(reportingConfig, elasticsearch, browserDriverFactory, this.logger); - registerReportingUsageCollector(this.reportingCore, plugins); - registerRoutes(this.reportingCore, this.logger); + this.logger.debug('Setup complete'); + }); return {}; } - public async start(core: CoreStart, plugins: ReportingStartDeps) { + public start(core: CoreStart, plugins: ReportingStartDeps) { + // use data plugin for csv formats + setFieldFormats(plugins.data.fieldFormats); + const { logger } = this; const reportingCore = this.getReportingCore(); + const config = reportingCore.getConfig(); + const { elasticsearch } = reportingCore.getPluginSetupDeps(); - const esqueue = await createQueueFactory(reportingCore, logger); - const enqueueJob = enqueueJobFactory(reportingCore, logger); + // async background start + initializeBrowserDriverFactory(config, logger).then(async (browserDriverFactory) => { + reportingCore.setBrowserDriverFactory(browserDriverFactory); - reportingCore.pluginStart({ - savedObjects: core.savedObjects, - uiSettings: core.uiSettings, - esqueue, - enqueueJob, - }); + const esqueue = await createQueueFactory(reportingCore, logger); + const enqueueJob = enqueueJobFactory(reportingCore, logger); - setFieldFormats(plugins.data.fieldFormats); - logger.info('reporting plugin started'); + reportingCore.pluginStart({ + savedObjects: core.savedObjects, + uiSettings: core.uiSettings, + esqueue, + enqueueJob, + }); + + // run self-check validations + runValidations(config, elasticsearch, browserDriverFactory, this.logger); + + this.logger.debug('Start complete'); + }); return {}; } diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts index 5b0d740e031ab..0e8be7578e988 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts @@ -7,7 +7,7 @@ import { Page } from 'puppeteer'; import * as Rx from 'rxjs'; import { HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../browsers'; -import { createDriverFactory } from '../browsers/chromium'; +import { chromium } from '../browsers'; import * as contexts from '../export_types/common/lib/screenshots/constants'; import { LevelLogger } from '../lib'; import { CaptureConfig, ElementsPositionAndAttribute } from '../types'; @@ -113,8 +113,12 @@ export const createMockBrowserDriverFactory = async ( maxAttempts: 1, }; - const binaryPath = '/usr/local/share/common/secure/'; - const mockBrowserDriverFactory = await createDriverFactory(binaryPath, logger, captureConfig); + const binaryPath = '/usr/local/share/common/secure/super_awesome_binary'; + const mockBrowserDriverFactory = await chromium.createDriverFactory( + binaryPath, + captureConfig, + logger + ); const mockPage = {} as Page; const mockBrowserDriver = new HeadlessChromiumDriver(mockPage, { inspect: true, From d2dd2e8eaba3c2d35d7f30a442c71d117580872a Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 5 Jun 2020 10:35:50 -0700 Subject: [PATCH 2/8] revert class conversion --- .../reporting/server/browsers/index.ts | 5 ++-- .../reporting/server/browsers/install.ts | 23 ++++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/reporting/server/browsers/index.ts b/x-pack/plugins/reporting/server/browsers/index.ts index 966b000f83ea3..be5b869ba523b 100644 --- a/x-pack/plugins/reporting/server/browsers/index.ts +++ b/x-pack/plugins/reporting/server/browsers/index.ts @@ -9,10 +9,9 @@ import { LevelLogger } from '../lib'; import { CaptureConfig } from '../types'; import { chromium } from './chromium'; import { HeadlessChromiumDriverFactory } from './chromium/driver_factory'; -import { BrowserInstaller } from './install'; +import { installBrowser } from './install'; import { ReportingConfig } from '..'; -export { BrowserInstaller } from './install'; export { ensureAllBrowsersDownloaded } from './download'; export { HeadlessChromiumDriver } from './chromium/driver'; export { HeadlessChromiumDriverFactory } from './chromium/driver_factory'; @@ -43,7 +42,7 @@ export const initializeBrowserDriverFactory = async ( config: ReportingConfig, logger: LevelLogger ) => { - const { binaryPath$ } = new BrowserInstaller(chromium, config, logger); + const { binaryPath$ } = installBrowser(chromium, config, logger); const binaryPath = await binaryPath$.pipe(first()).toPromise(); const captureConfig = config.get('capture'); return chromium.createDriverFactory(binaryPath, captureConfig, logger); diff --git a/x-pack/plugins/reporting/server/browsers/install.ts b/x-pack/plugins/reporting/server/browsers/install.ts index ca51df5730162..0c1803a46a9ec 100644 --- a/x-pack/plugins/reporting/server/browsers/install.ts +++ b/x-pack/plugins/reporting/server/browsers/install.ts @@ -28,15 +28,13 @@ interface Package { * "install" a browser by type into installs path by extracting the downloaded * archive. If there is an error extracting the archive an `ExtractError` is thrown */ -export class BrowserInstaller { - public readonly binaryPath$: Rx.Subject; - - constructor(browser: BrowserDownload, config: ReportingConfig, logger: LevelLogger) { - this.binaryPath$ = new Rx.Subject(); - this.backgroundInstall(browser, config, logger); - } - - async backgroundInstall(browser: BrowserDownload, config: ReportingConfig, logger: LevelLogger) { +export function installBrowser( + browser: BrowserDownload, + config: ReportingConfig, + logger: LevelLogger +): { binaryPath$: Rx.Subject } { + const binaryPath$ = new Rx.Subject(); + const backgroundInstall = async () => { const captureConfig = config.get('capture'); const { autoDownload, type: browserType } = captureConfig.browser; if (autoDownload) { @@ -73,6 +71,9 @@ export class BrowserInstaller { logger.debug(`Browser executable: ${binaryPath}`); - this.binaryPath$.next(binaryPath); // subscribers wait for download and extract to complete - } + binaryPath$.next(binaryPath); // subscribers wait for download and extract to complete + }; + + backgroundInstall(); + return { binaryPath$ }; } From 02e78d676b8a962da7333130b0c3c4f01919629b Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 5 Jun 2020 10:48:49 -0700 Subject: [PATCH 3/8] diff prettify --- x-pack/plugins/reporting/server/browsers/install.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/reporting/server/browsers/install.ts b/x-pack/plugins/reporting/server/browsers/install.ts index 0c1803a46a9ec..49361b7b6014d 100644 --- a/x-pack/plugins/reporting/server/browsers/install.ts +++ b/x-pack/plugins/reporting/server/browsers/install.ts @@ -75,5 +75,8 @@ export function installBrowser( }; backgroundInstall(); - return { binaryPath$ }; + + return { + binaryPath$, + }; } From 10ccc38fcc0c9d9aa02109f815e3d467c4fdb179 Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Fri, 5 Jun 2020 11:30:53 -0700 Subject: [PATCH 4/8] Fix test mocks + add some plugin async helpers where needed --- .../server/config/create_config.test.ts | 6 +++++- x-pack/plugins/reporting/server/plugin.ts | 19 +++++++++++++++++++ .../create_mock_browserdriverfactory.ts | 3 +-- .../create_mock_reportingplugin.ts | 18 +++++++++++++++--- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.test.ts b/x-pack/plugins/reporting/server/config/create_config.test.ts index 1c4c840cfa312..8ad8042a93105 100644 --- a/x-pack/plugins/reporting/server/config/create_config.test.ts +++ b/x-pack/plugins/reporting/server/config/create_config.test.ts @@ -45,7 +45,11 @@ describe('Reporting server createConfig$', () => { mockInitContext = makeMockInitContext({ kibanaServer: {}, }); - mockLogger = ({ warn: jest.fn(), debug: jest.fn() } as unknown) as LevelLogger; + mockLogger = ({ + warn: jest.fn(), + debug: jest.fn(), + clone: jest.fn().mockImplementation(() => mockLogger), + } as unknown) as LevelLogger; }); afterEach(() => { diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index ed4939fcc74f3..5eadcfab7ed28 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -20,9 +20,26 @@ export class ReportingPlugin private logger: LevelLogger; private reportingCore?: ReportingCore; + private setupDone: (val?: unknown) => void = () => {}; + private startDone: (val?: unknown) => void = () => {}; + private setupPromise: Promise; + private startPromise: Promise; + constructor(context: PluginInitializerContext) { this.logger = new LevelLogger(context.logger.get()); this.initializerContext = context; + + // Setup some promises for modules that need to await setup/start + this.setupPromise = new Promise((r) => (this.setupDone = r)); + this.startPromise = new Promise((r) => (this.startDone = r)); + } + + public async setupReady() { + return this.setupPromise; + } + + public async startReady() { + return this.startPromise; } public setup(core: CoreSetup, plugins: ReportingSetupDeps) { @@ -49,6 +66,7 @@ export class ReportingPlugin this.reportingCore = reportingCore; this.logger.debug('Setup complete'); + this.setupDone(); }); return {}; @@ -81,6 +99,7 @@ export class ReportingPlugin runValidations(config, elasticsearch, browserDriverFactory, this.logger); this.logger.debug('Start complete'); + this.startDone(); }); return {}; diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts index 0e8be7578e988..97e22e2ca2863 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts @@ -6,8 +6,7 @@ import { Page } from 'puppeteer'; import * as Rx from 'rxjs'; -import { HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../browsers'; -import { chromium } from '../browsers'; +import { chromium, HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../browsers'; import * as contexts from '../export_types/common/lib/screenshots/constants'; import { LevelLogger } from '../lib'; import { CaptureConfig, ElementsPositionAndAttribute } from '../types'; diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts index b04e697d0a118..085f4c9995fab 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts @@ -7,18 +7,28 @@ jest.mock('../routes'); jest.mock('../usage'); jest.mock('../browsers'); -jest.mock('../browsers'); jest.mock('../lib/create_queue'); jest.mock('../lib/enqueue_job'); jest.mock('../lib/validate'); import { of } from 'rxjs'; import { coreMock } from 'src/core/server/mocks'; +import { + initializeBrowserDriverFactory, + HeadlessChromiumDriverFactory, + chromium, +} from '../browsers'; import { ReportingConfig, ReportingCore } from '../'; import { ReportingInternalSetup } from '../core'; import { ReportingPlugin } from '../plugin'; import { ReportingSetupDeps, ReportingStartDeps } from '../types'; +(initializeBrowserDriverFactory as jest.Mock< + Promise +>).mockImplementation(() => Promise.resolve({} as HeadlessChromiumDriverFactory)); + +(chromium as any).createDriverFactory.mockImplementation(() => ({})); + const createMockSetupDeps = (setupMock?: any): ReportingSetupDeps => { return { security: setupMock.security, @@ -57,8 +67,10 @@ const createMockReportingPlugin = async (config: ReportingConfig): Promise Date: Fri, 5 Jun 2020 14:26:10 -0700 Subject: [PATCH 5/8] no setupReady startReady needed --- x-pack/plugins/reporting/server/plugin.ts | 24 +++++-------------- .../create_mock_reportingplugin.ts | 11 +++++---- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index 5eadcfab7ed28..4ed7e494b378d 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import * as Rx from 'rxjs'; import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/server'; import { ReportingCore } from './'; import { initializeBrowserDriverFactory } from './browsers'; @@ -20,26 +21,13 @@ export class ReportingPlugin private logger: LevelLogger; private reportingCore?: ReportingCore; - private setupDone: (val?: unknown) => void = () => {}; - private startDone: (val?: unknown) => void = () => {}; - private setupPromise: Promise; - private startPromise: Promise; + // Setup some observables for modules that need to await setup/start + public readonly setup$ = new Rx.Subject(); + public readonly start$ = new Rx.Subject(); constructor(context: PluginInitializerContext) { this.logger = new LevelLogger(context.logger.get()); this.initializerContext = context; - - // Setup some promises for modules that need to await setup/start - this.setupPromise = new Promise((r) => (this.setupDone = r)); - this.startPromise = new Promise((r) => (this.startDone = r)); - } - - public async setupReady() { - return this.setupPromise; - } - - public async startReady() { - return this.startPromise; } public setup(core: CoreSetup, plugins: ReportingSetupDeps) { @@ -66,7 +54,7 @@ export class ReportingPlugin this.reportingCore = reportingCore; this.logger.debug('Setup complete'); - this.setupDone(); + this.setup$.next(true); }); return {}; @@ -99,7 +87,7 @@ export class ReportingPlugin runValidations(config, elasticsearch, browserDriverFactory, this.logger); this.logger.debug('Start complete'); - this.startDone(); + this.start$.next(true); }); return {}; diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts index 085f4c9995fab..33edbed3002c4 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts @@ -12,13 +12,14 @@ jest.mock('../lib/enqueue_job'); jest.mock('../lib/validate'); import { of } from 'rxjs'; +import { first } from 'rxjs/operators'; import { coreMock } from 'src/core/server/mocks'; +import { ReportingConfig, ReportingCore } from '../'; import { - initializeBrowserDriverFactory, - HeadlessChromiumDriverFactory, chromium, + HeadlessChromiumDriverFactory, + initializeBrowserDriverFactory, } from '../browsers'; -import { ReportingConfig, ReportingCore } from '../'; import { ReportingInternalSetup } from '../core'; import { ReportingPlugin } from '../plugin'; import { ReportingSetupDeps, ReportingStartDeps } from '../types'; @@ -68,9 +69,9 @@ const createMockReportingPlugin = async (config: ReportingConfig): Promise Date: Tue, 9 Jun 2020 12:48:38 -0700 Subject: [PATCH 6/8] Add plugin test for sync/error cases --- .../plugins/reporting/server/plugin.test.ts | 115 ++++++++++++++++++ x-pack/plugins/reporting/server/plugin.ts | 14 ++- .../plugins/reporting/server/routes/jobs.ts | 2 +- .../create_mock_reportingplugin.ts | 37 +++--- .../reporting/server/test_helpers/index.ts | 2 +- 5 files changed, 150 insertions(+), 20 deletions(-) create mode 100644 x-pack/plugins/reporting/server/plugin.test.ts diff --git a/x-pack/plugins/reporting/server/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts new file mode 100644 index 0000000000000..80a3706823ab6 --- /dev/null +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -0,0 +1,115 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +jest.mock('./browsers/install', () => ({ + installBrowser: jest.fn().mockImplementation(() => ({ + binaryPath$: { + pipe: jest.fn().mockImplementation(() => ({ + toPromise: () => Promise.resolve(), + })), + }, + })), +})); + +import { coreMock } from 'src/core/server/mocks'; +import { ReportingPlugin } from './plugin'; +import { createMockConfig } from './test_helpers'; + +const sleep = (time: number) => new Promise((r) => setTimeout(r, time)); + +describe('Reporting Plugin', () => { + let config: any; + let initContext: any; + let coreSetup: any; + let coreStart: any; + let pluginSetup: any; + let pluginStart: any; + + beforeEach(async () => { + config = createMockConfig(); + initContext = coreMock.createPluginInitializerContext(config); + coreSetup = await coreMock.createSetup(config); + coreStart = await coreMock.createStart(); + pluginSetup = ({ + licensing: {}, + usageCollection: { + makeUsageCollector: jest.fn(), + registerCollector: jest.fn(), + }, + security: { + authc: { + getCurrentUser: () => ({ + id: '123', + roles: ['superuser'], + username: 'Tom Riddle', + }), + }, + }, + } as unknown) as any; + pluginStart = ({ + data: { + fieldFormats: {}, + }, + } as unknown) as any; + }); + + it('has a sync setup process', () => { + const plugin = new ReportingPlugin(initContext); + + expect(plugin.setup(coreSetup, pluginSetup)).not.toHaveProperty('then'); + }); + + it('logs setup issues', async () => { + const plugin = new ReportingPlugin(initContext); + // @ts-ignore overloading error logger + plugin.logger.error = jest.fn(); + coreSetup.elasticsearch = null; + plugin.setup(coreSetup, pluginSetup); + + await sleep(5); + + // @ts-ignore overloading error logger + expect(plugin.logger.error.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "Error in Reporting setup, reporting may not function properly + TypeError: Cannot read property 'legacy' of null + at jobsQueryFactory (/Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/lib/jobs_query.ts:50:48) + at registerJobInfoRoutes (/Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/routes/jobs.ts:30:21) + at registerRoutes (/Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/routes/index.ts:14:3) + at /Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/plugin.ts:54:7 + at process._tickCallback (internal/process/next_tick.js:68:7)", + ], + ] + `); + }); + + it('has a sync startup process', async () => { + const plugin = new ReportingPlugin(initContext); + plugin.setup(coreSetup, pluginSetup); + await sleep(5); + expect(plugin.start(coreStart, pluginStart)).not.toHaveProperty('then'); + }); + + it('logs start issues', async () => { + const plugin = new ReportingPlugin(initContext); + // @ts-ignore overloading error logger + plugin.logger.error = jest.fn(); + plugin.setup(coreSetup, pluginSetup); + await sleep(5); + plugin.start(null as any, pluginStart); + await sleep(10); + // @ts-ignore overloading error logger + expect(plugin.logger.error.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "Error in Reporting startup, reporting may not function properly + TypeError: Cannot read property 'savedObjects' of null + at /Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/plugin.ts:86:28", + ], + ] + `); + }); +}); diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index 4ed7e494b378d..ca77af9754c52 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -38,7 +38,8 @@ export class ReportingPlugin const basePath = http.basePath.get; // async background setup - buildConfig(initContext, core, this.logger).then((config) => { + (async () => { + const config = await buildConfig(initContext, core, this.logger); const reportingCore = new ReportingCore(config); reportingCore.pluginSetup({ @@ -55,6 +56,10 @@ export class ReportingPlugin this.logger.debug('Setup complete'); this.setup$.next(true); + })().catch((e) => { + this.logger.error( + `Error in Reporting setup, reporting may not function properly\n` + e.stack + ); }); return {}; @@ -70,7 +75,8 @@ export class ReportingPlugin const { elasticsearch } = reportingCore.getPluginSetupDeps(); // async background start - initializeBrowserDriverFactory(config, logger).then(async (browserDriverFactory) => { + (async () => { + const browserDriverFactory = await initializeBrowserDriverFactory(config, logger); reportingCore.setBrowserDriverFactory(browserDriverFactory); const esqueue = await createQueueFactory(reportingCore, logger); @@ -88,6 +94,10 @@ export class ReportingPlugin this.logger.debug('Start complete'); this.start$.next(true); + })().catch((e) => { + this.logger.error( + `Error in Reporting startup, reporting may not function properly\n` + e.stack + ); }); return {}; diff --git a/x-pack/plugins/reporting/server/routes/jobs.ts b/x-pack/plugins/reporting/server/routes/jobs.ts index 8c35f79ec0fb4..29cf55bc5c72e 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.ts @@ -22,7 +22,7 @@ interface ListQuery { } const MAIN_ENTRY = `${API_BASE_URL}/jobs`; -export async function registerJobInfoRoutes(reporting: ReportingCore) { +export function registerJobInfoRoutes(reporting: ReportingCore) { const config = reporting.getConfig(); const setupDeps = reporting.getPluginSetupDeps(); const userHandler = authorizedUserPreRoutingFactory(reporting); diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts index 33edbed3002c4..ca6e999cba360 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts @@ -36,30 +36,35 @@ const createMockSetupDeps = (setupMock?: any): ReportingSetupDeps => { licensing: { license$: of({ isAvailable: true, isActive: true, type: 'basic' }), } as any, - usageCollection: {} as any, + usageCollection: { + makeUsageCollector: jest.fn(), + registerCollector: jest.fn(), + } as any, }; }; +export const createMockConfig = (overrides?: any) => ({ + index: '.reporting', + kibanaServer: { + hostname: 'localhost', + port: '80', + }, + capture: { + browser: { + chromium: { + disableSandbox: true, + }, + }, + }, + ...overrides, +}); + export const createMockStartDeps = (startMock?: any): ReportingStartDeps => ({ data: startMock.data, }); const createMockReportingPlugin = async (config: ReportingConfig): Promise => { - const mockConfig = { - index: '.reporting', - kibanaServer: { - hostname: 'localhost', - port: '80', - }, - capture: { - browser: { - chromium: { - disableSandbox: true, - }, - }, - }, - ...config, - }; + const mockConfig = createMockConfig(config); const plugin = new ReportingPlugin(coreMock.createPluginInitializerContext(mockConfig)); const setupMock = coreMock.createSetup(); const coreStartMock = coreMock.createStart(); diff --git a/x-pack/plugins/reporting/server/test_helpers/index.ts b/x-pack/plugins/reporting/server/test_helpers/index.ts index 491d390c370b9..fc513cf58579a 100644 --- a/x-pack/plugins/reporting/server/test_helpers/index.ts +++ b/x-pack/plugins/reporting/server/test_helpers/index.ts @@ -5,6 +5,6 @@ */ export { createMockServer } from './create_mock_server'; -export { createMockReportingCore } from './create_mock_reportingplugin'; +export { createMockReportingCore, createMockConfig } from './create_mock_reportingplugin'; export { createMockBrowserDriverFactory } from './create_mock_browserdriverfactory'; export { createMockLayoutInstance } from './create_mock_layoutinstance'; From 6d3a8a9d4f7ae8ad7f7e180339f59e059448dc0d Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Tue, 9 Jun 2020 13:35:53 -0700 Subject: [PATCH 7/8] PR feedback on tests/setup logs --- .../plugins/reporting/server/plugin.test.ts | 32 ++++++------------- x-pack/plugins/reporting/server/plugin.ts | 10 +++--- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/reporting/server/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts index 80a3706823ab6..c656a749ecaf8 100644 --- a/x-pack/plugins/reporting/server/plugin.test.ts +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -71,19 +71,11 @@ describe('Reporting Plugin', () => { await sleep(5); // @ts-ignore overloading error logger - expect(plugin.logger.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - "Error in Reporting setup, reporting may not function properly - TypeError: Cannot read property 'legacy' of null - at jobsQueryFactory (/Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/lib/jobs_query.ts:50:48) - at registerJobInfoRoutes (/Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/routes/jobs.ts:30:21) - at registerRoutes (/Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/routes/index.ts:14:3) - at /Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/plugin.ts:54:7 - at process._tickCallback (internal/process/next_tick.js:68:7)", - ], - ] - `); + expect(plugin.logger.error.mock.calls[0][0]).toMatch( + /Error in Reporting setup, reporting may not function properly/ + ); + // @ts-ignore overloading error logger + expect(plugin.logger.error).toHaveBeenCalledTimes(2); }); it('has a sync startup process', async () => { @@ -102,14 +94,10 @@ describe('Reporting Plugin', () => { plugin.start(null as any, pluginStart); await sleep(10); // @ts-ignore overloading error logger - expect(plugin.logger.error.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - "Error in Reporting startup, reporting may not function properly - TypeError: Cannot read property 'savedObjects' of null - at /Users/joelgriffith/Projects/kibana/x-pack/plugins/reporting/server/plugin.ts:86:28", - ], - ] - `); + expect(plugin.logger.error.mock.calls[0][0]).toMatch( + /Error in Reporting start, reporting may not function properly/ + ); + // @ts-ignore overloading error logger + expect(plugin.logger.error).toHaveBeenCalledTimes(2); }); }); diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index ca77af9754c52..a3c89c7b8a8ce 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -57,9 +57,8 @@ export class ReportingPlugin this.logger.debug('Setup complete'); this.setup$.next(true); })().catch((e) => { - this.logger.error( - `Error in Reporting setup, reporting may not function properly\n` + e.stack - ); + this.logger.error(`Error in Reporting setup, reporting may not function properly`); + this.logger.error(e); }); return {}; @@ -95,9 +94,8 @@ export class ReportingPlugin this.logger.debug('Start complete'); this.start$.next(true); })().catch((e) => { - this.logger.error( - `Error in Reporting startup, reporting may not function properly\n` + e.stack - ); + this.logger.error(`Error in Reporting start, reporting may not function properly`); + this.logger.error(e); }); return {}; From 4f1f4a6a1d2c97b7a657193e84ef8f5c63b91c50 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 9 Jun 2020 13:54:14 -0700 Subject: [PATCH 8/8] rename symbol --- x-pack/plugins/reporting/server/plugin.test.ts | 10 +++++----- .../server/test_helpers/create_mock_reportingplugin.ts | 6 +++--- x-pack/plugins/reporting/server/test_helpers/index.ts | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/reporting/server/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts index c656a749ecaf8..b2bcd6b9c97ce 100644 --- a/x-pack/plugins/reporting/server/plugin.test.ts +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -15,12 +15,12 @@ jest.mock('./browsers/install', () => ({ import { coreMock } from 'src/core/server/mocks'; import { ReportingPlugin } from './plugin'; -import { createMockConfig } from './test_helpers'; +import { createMockConfigSchema } from './test_helpers'; const sleep = (time: number) => new Promise((r) => setTimeout(r, time)); describe('Reporting Plugin', () => { - let config: any; + let configSchema: any; let initContext: any; let coreSetup: any; let coreStart: any; @@ -28,9 +28,9 @@ describe('Reporting Plugin', () => { let pluginStart: any; beforeEach(async () => { - config = createMockConfig(); - initContext = coreMock.createPluginInitializerContext(config); - coreSetup = await coreMock.createSetup(config); + configSchema = createMockConfigSchema(); + initContext = coreMock.createPluginInitializerContext(configSchema); + coreSetup = await coreMock.createSetup(configSchema); coreStart = await coreMock.createStart(); pluginSetup = ({ licensing: {}, diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts index ca6e999cba360..669381a92c522 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts @@ -43,7 +43,7 @@ const createMockSetupDeps = (setupMock?: any): ReportingSetupDeps => { }; }; -export const createMockConfig = (overrides?: any) => ({ +export const createMockConfigSchema = (overrides?: any) => ({ index: '.reporting', kibanaServer: { hostname: 'localhost', @@ -64,8 +64,8 @@ export const createMockStartDeps = (startMock?: any): ReportingStartDeps => ({ }); const createMockReportingPlugin = async (config: ReportingConfig): Promise => { - const mockConfig = createMockConfig(config); - const plugin = new ReportingPlugin(coreMock.createPluginInitializerContext(mockConfig)); + const mockConfigSchema = createMockConfigSchema(config); + const plugin = new ReportingPlugin(coreMock.createPluginInitializerContext(mockConfigSchema)); const setupMock = coreMock.createSetup(); const coreStartMock = coreMock.createStart(); const startMock = { diff --git a/x-pack/plugins/reporting/server/test_helpers/index.ts b/x-pack/plugins/reporting/server/test_helpers/index.ts index fc513cf58579a..b37b447dc05a9 100644 --- a/x-pack/plugins/reporting/server/test_helpers/index.ts +++ b/x-pack/plugins/reporting/server/test_helpers/index.ts @@ -5,6 +5,6 @@ */ export { createMockServer } from './create_mock_server'; -export { createMockReportingCore, createMockConfig } from './create_mock_reportingplugin'; +export { createMockReportingCore, createMockConfigSchema } from './create_mock_reportingplugin'; export { createMockBrowserDriverFactory } from './create_mock_browserdriverfactory'; export { createMockLayoutInstance } from './create_mock_layoutinstance';