From a4fca06c1c67610d272e2611fe8bc1f0f79dea73 Mon Sep 17 00:00:00 2001 From: Spencer Date: Thu, 12 Dec 2019 06:36:27 -0700 Subject: [PATCH] =?UTF-8?q?[7.5]=20[kbnClient]=20Retry=20uiSettings.replac?= =?UTF-8?q?e()=20calls=20up=20to=205=20tim=E2=80=A6=20(#52780)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [kbnClient] Retry uiSettings.replace() calls up to 5 times (#52601) * [kbn/dev-utils] target ES2019 to transpile ?? * Retry uiSettings.replace() calls up to 5 times * share logic for selecting junit report name to ensure they are unique * convert to junit report path helper # Conflicts: # packages/kbn-test/src/mocha/__tests__/junit_report_generation.js # packages/kbn-test/src/mocha/junit_report_generation.js # src/dev/jest/junit_reporter.js * remove ?? syntax * fix more null-ish syntax not supported in 7.5 * fix imports --- .../src/kbn_client/kbn_client_requester.ts | 70 ++++++++++--------- .../src/kbn_client/kbn_client_ui_settings.ts | 30 ++++---- packages/kbn-dev-utils/tsconfig.json | 1 + packages/kbn-test/src/index.ts | 2 + packages/kbn-test/src/junit_report_path.ts | 32 +++++++++ .../integration_tests/junit_reporter.test.js | 3 +- src/dev/jest/junit_reporter.js | 9 +-- .../__tests__/junit_report_generation.js | 13 +--- src/dev/mocha/junit_report_generation.js | 11 +-- tasks/config/karma.js | 5 +- 10 files changed, 99 insertions(+), 77 deletions(-) create mode 100644 packages/kbn-test/src/junit_report_path.ts diff --git a/packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts b/packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts index 25962c91a896d7..f33b2e070c4992 100644 --- a/packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts +++ b/packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts @@ -62,8 +62,7 @@ export interface ReqOptions { query?: Record; method: 'GET' | 'POST' | 'PUT' | 'DELETE'; body?: any; - attempt?: number; - maxAttempts?: number; + retries?: number; } const delay = (ms: number) => @@ -87,44 +86,47 @@ export class KbnClientRequester { async request(options: ReqOptions): Promise { const url = Url.resolve(this.pickUrl(), options.path); const description = options.description || `${options.method} ${url}`; - const attempt = options.attempt === undefined ? 1 : options.attempt; - const maxAttempts = - options.maxAttempts === undefined ? DEFAULT_MAX_ATTEMPTS : options.maxAttempts; - - try { - const response = await Axios.request({ - method: options.method, - url, - data: options.body, - params: options.query, - headers: { - 'kbn-xsrf': 'kbn-client', - }, - }); - - return response.data; - } catch (error) { - let retryErrorMsg: string | undefined; - if (isAxiosRequestError(error)) { - retryErrorMsg = `[${description}] request failed (attempt=${attempt})`; - } else if (isConcliftOnGetError(error)) { - retryErrorMsg = `Conflict on GET (path=${options.path}, attempt=${attempt})`; - } + let attempt = 0; + const maxAttempts = options.retries === undefined ? DEFAULT_MAX_ATTEMPTS : options.retries; + + while (true) { + attempt += 1; + + try { + const response = await Axios.request({ + method: options.method, + url, + data: options.body, + params: options.query, + headers: { + 'kbn-xsrf': 'kbn-client', + }, + }); + + return response.data; + } catch (error) { + const conflictOnGet = isConcliftOnGetError(error); + const requestedRetries = options.retries !== undefined; + const failedToGetResponse = isAxiosRequestError(error); + + let errorMessage; + if (conflictOnGet) { + errorMessage = `Conflict on GET (path=${options.path}, attempt=${attempt}/${maxAttempts})`; + this.log.error(errorMessage); + } else if (requestedRetries || failedToGetResponse) { + errorMessage = `[${description}] request failed (attempt=${attempt}/${maxAttempts})`; + this.log.error(errorMessage); + } else { + throw error; + } - if (retryErrorMsg) { if (attempt < maxAttempts) { - this.log.error(retryErrorMsg); await delay(1000 * attempt); - return await this.request({ - ...options, - attempt: attempt + 1, - }); + continue; } - throw new Error(retryErrorMsg + ' and ran out of retries'); + throw new Error(`${errorMessage} -- and ran out of retries`); } - - throw error; } } } diff --git a/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts b/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts index 03033bc5c2ccc4..6e6eeb0d35238b 100644 --- a/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts +++ b/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts @@ -40,7 +40,7 @@ export class KbnClientUiSettings { async get(setting: string) { const all = await this.getAll(); - const value = all.settings[setting] ? all.settings[setting].userValue : undefined; + const value = all[setting] === undefined ? undefined : all[setting].userValue; this.log.verbose('uiSettings.value: %j', value); return value; @@ -68,24 +68,24 @@ export class KbnClientUiSettings { * with some defaults */ async replace(doc: UiSettingValues) { - const all = await this.getAll(); - for (const [name, { isOverridden }] of Object.entries(all.settings)) { - if (!isOverridden) { - await this.unset(name); + this.log.debug('replacing kibana config doc: %j', doc); + + const changes: Record = { + ...this.defaults, + ...doc, + }; + + for (const [name, { isOverridden }] of Object.entries(await this.getAll())) { + if (!isOverridden && !changes.hasOwnProperty(name)) { + changes[name] = null; } } - this.log.debug('replacing kibana config doc: %j', doc); - await this.requester.request({ method: 'POST', path: '/api/kibana/settings', - body: { - changes: { - ...this.defaults, - ...doc, - }, - }, + body: { changes }, + retries: 5, }); } @@ -105,9 +105,11 @@ export class KbnClientUiSettings { } private async getAll() { - return await this.requester.request({ + const resp = await this.requester.request({ path: '/api/kibana/settings', method: 'GET', }); + + return resp.settings; } } diff --git a/packages/kbn-dev-utils/tsconfig.json b/packages/kbn-dev-utils/tsconfig.json index 40a3bd475f1c1d..4c519a609d86fe 100644 --- a/packages/kbn-dev-utils/tsconfig.json +++ b/packages/kbn-dev-utils/tsconfig.json @@ -2,6 +2,7 @@ "extends": "../../tsconfig.json", "compilerOptions": { "outDir": "target", + "target": "ES2019", "declaration": true }, "include": [ diff --git a/packages/kbn-test/src/index.ts b/packages/kbn-test/src/index.ts index f39fde0d82c555..d078fb23c958ae 100644 --- a/packages/kbn-test/src/index.ts +++ b/packages/kbn-test/src/index.ts @@ -42,3 +42,5 @@ export { readConfigFile } from './functional_test_runner/lib/config/read_config_ export { runFtrCli } from './functional_test_runner/cli'; export { runFailedTestsReporterCli } from './failed_tests_reporter'; + +export { makeJunitReportPath } from './junit_report_path'; diff --git a/packages/kbn-test/src/junit_report_path.ts b/packages/kbn-test/src/junit_report_path.ts new file mode 100644 index 00000000000000..11eaf3d2b14a5e --- /dev/null +++ b/packages/kbn-test/src/junit_report_path.ts @@ -0,0 +1,32 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { resolve } from 'path'; + +const job = process.env.JOB ? `job-${process.env.JOB}-` : ''; +const num = process.env.CI_WORKER_NUMBER ? `worker-${process.env.CI_WORKER_NUMBER}-` : ''; + +export function makeJunitReportPath(rootDirectory: string, reportName: string) { + return resolve( + rootDirectory, + 'target/junit', + process.env.JOB || '.', + `TEST-${job}${num}${reportName}.xml` + ); +} diff --git a/src/dev/jest/integration_tests/junit_reporter.test.js b/src/dev/jest/integration_tests/junit_reporter.test.js index c26c8cfad80258..186a13ed9b8e93 100644 --- a/src/dev/jest/integration_tests/junit_reporter.test.js +++ b/src/dev/jest/integration_tests/junit_reporter.test.js @@ -24,12 +24,13 @@ import { readFileSync } from 'fs'; import del from 'del'; import execa from 'execa'; import xml2js from 'xml2js'; +import { makeJunitReportPath } from '@kbn/test'; const MINUTE = 1000 * 60; const ROOT_DIR = resolve(__dirname, '../../../../'); const FIXTURE_DIR = resolve(__dirname, '__fixtures__'); const TARGET_DIR = resolve(FIXTURE_DIR, 'target'); -const XML_PATH = resolve(TARGET_DIR, 'junit', process.env.JOB || '.', `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}Jest Tests.xml`); +const XML_PATH = makeJunitReportPath(FIXTURE_DIR, 'Jest Tests'); afterAll(async () => { await del(TARGET_DIR); diff --git a/src/dev/jest/junit_reporter.js b/src/dev/jest/junit_reporter.js index d2d8d8f43e3b26..a3b0eeeaba4377 100644 --- a/src/dev/jest/junit_reporter.js +++ b/src/dev/jest/junit_reporter.js @@ -23,6 +23,7 @@ import { writeFileSync, mkdirSync } from 'fs'; import xmlBuilder from 'xmlbuilder'; import { escapeCdata } from '../xml'; +import { makeJunitReportPath } from '@kbn/test'; const ROOT_DIR = dirname(require.resolve('../../../package.json')); @@ -102,13 +103,7 @@ export default class JestJUnitReporter { }); }); - const reportPath = resolve( - rootDirectory, - 'target/junit', - process.env.JOB || '.', - `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml` - ); - + const reportPath = makeJunitReportPath(rootDirectory, reportName); const reportXML = root.end(); mkdirSync(dirname(reportPath), { recursive: true }); writeFileSync(reportPath, reportXML, 'utf8'); diff --git a/src/dev/mocha/__tests__/junit_report_generation.js b/src/dev/mocha/__tests__/junit_report_generation.js index b27f2fcf47a5a6..7472e271bd1e9c 100644 --- a/src/dev/mocha/__tests__/junit_report_generation.js +++ b/src/dev/mocha/__tests__/junit_report_generation.js @@ -25,6 +25,7 @@ import { parseString } from 'xml2js'; import del from 'del'; import Mocha from 'mocha'; import expect from '@kbn/expect'; +import { makeJunitReportPath } from '@kbn/test'; import { setupJUnitReportGeneration } from '../junit_report_generation'; @@ -50,17 +51,7 @@ describe('dev/mocha/junit report generation', () => { mocha.addFile(resolve(PROJECT_DIR, 'test.js')); await new Promise(resolve => mocha.run(resolve)); const report = await fcb(cb => - parseString( - readFileSync( - resolve( - PROJECT_DIR, - 'target/junit', - process.env.JOB || '.', - `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}test.xml` - ) - ), - cb - ) + parseString(readFileSync(makeJunitReportPath(PROJECT_DIR, 'test')), cb) ); // test case results are wrapped in diff --git a/src/dev/mocha/junit_report_generation.js b/src/dev/mocha/junit_report_generation.js index f10ed541c83038..b3885a4f6fbc66 100644 --- a/src/dev/mocha/junit_report_generation.js +++ b/src/dev/mocha/junit_report_generation.js @@ -17,11 +17,12 @@ * under the License. */ -import { resolve, dirname, relative } from 'path'; +import { dirname, relative } from 'path'; import { writeFileSync, mkdirSync } from 'fs'; import { inspect } from 'util'; import xmlBuilder from 'xmlbuilder'; +import { makeJunitReportPath } from '@kbn/test'; import { getSnapshotOfRunnableLogs } from './log_cache'; import { escapeCdata } from '../xml'; @@ -137,13 +138,7 @@ export function setupJUnitReportGeneration(runner, options = {}) { } }); - const reportPath = resolve( - rootDirectory, - 'target/junit', - process.env.JOB || '.', - `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml` - ); - + const reportPath = makeJunitReportPath(rootDirectory, reportName); const reportXML = builder.end(); mkdirSync(dirname(reportPath), { recursive: true }); writeFileSync(reportPath, reportXML, 'utf8'); diff --git a/tasks/config/karma.js b/tasks/config/karma.js index 16947a97a3d146..3fb7b72abc6d29 100644 --- a/tasks/config/karma.js +++ b/tasks/config/karma.js @@ -17,8 +17,9 @@ * under the License. */ -import { resolve, dirname } from 'path'; +import { dirname } from 'path'; import { times } from 'lodash'; +import { makeJunitReportPath } from '@kbn/test'; const TOTAL_CI_SHARDS = 4; const ROOT = dirname(require.resolve('../../package.json')); @@ -67,7 +68,7 @@ module.exports = function (grunt) { reporters: process.env.CI ? ['dots', 'junit'] : ['progress'], junitReporter: { - outputFile: resolve(ROOT, 'target/junit', process.env.JOB || '.', `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}karma.xml`), + outputFile: makeJunitReportPath(ROOT, 'karma'), useBrowserName: false, nameFormatter: (browser, result) => [...result.suite, result.description].join(' '), classNameFormatter: (browser, result) => {