From 9cead7efe97454cd1babe938da7f4e5c45872e04 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Fri, 3 Apr 2020 10:49:25 -0400 Subject: [PATCH 01/10] Add per-test metrics tracking to FTR --- .../functional_test_runner.ts | 2 + .../lib/config/config.ts | 3 + .../src/functional_test_runner/lib/index.ts | 1 + .../lib/mocha/decorate_mocha_ui.js | 4 +- .../lib/mocha/load_test_files.js | 12 +- .../lib/mocha/setup_mocha.js | 1 + .../lib/test_tracker.ts | 123 ++++++++++++++++++ 7 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts diff --git a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts index 03075dd4081fd..19e18f9831b70 100644 --- a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts +++ b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts @@ -30,11 +30,13 @@ import { setupMocha, runTests, Config, + TestTracker, } from './lib'; export class FunctionalTestRunner { public readonly lifecycle = new Lifecycle(); public readonly failureMetadata = new FailureMetadata(this.lifecycle); + public readonly testTracker = new TestTracker(this.lifecycle); private closed = false; constructor( diff --git a/packages/kbn-test/src/functional_test_runner/lib/config/config.ts b/packages/kbn-test/src/functional_test_runner/lib/config/config.ts index ad9247523797a..33adadf3c7dae 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/config/config.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/config/config.ts @@ -35,6 +35,7 @@ interface Options { export class Config { private [$values]: Record; + public path: string; constructor(options: Options) { const { settings = {}, primary = false, path = null } = options || {}; @@ -43,6 +44,8 @@ export class Config { throw new TypeError('path is a required option'); } + this.path = path; + const { error, value } = schema.validate(settings, { abortEarly: false, context: { diff --git a/packages/kbn-test/src/functional_test_runner/lib/index.ts b/packages/kbn-test/src/functional_test_runner/lib/index.ts index 8940eccad503a..4ba0077c2f008 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/index.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/index.ts @@ -23,3 +23,4 @@ export { readConfigFile, Config } from './config'; export { readProviderSpec, ProviderCollection, Provider } from './providers'; export { runTests, setupMocha } from './mocha'; export { FailureMetadata } from './failure_metadata'; +export { TestTracker } from './test_tracker'; diff --git a/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js b/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js index 1cac852a7e713..ebd85327495be 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js +++ b/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js @@ -22,7 +22,7 @@ import { createAssignmentProxy } from './assignment_proxy'; import { wrapFunction } from './wrap_function'; import { wrapRunnableArgs } from './wrap_runnable_args'; -export function decorateMochaUi(lifecycle, context) { +export function decorateMochaUi(lifecycle, context, config) { // incremented at the start of each suite, decremented after // so that in each non-suite call we can know if we are within // a suite, or that when a suite is defined it is within a suite @@ -70,6 +70,8 @@ export function decorateMochaUi(lifecycle, context) { this.tags(relativeFilePath); this.suiteTag = relativeFilePath; // The tag that uniquely targets this suite/file + this.ftrConfig = config; + provider.call(this); after(async () => { diff --git a/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js b/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js index 6ee65b1b7e394..153a83920f483 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js +++ b/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js @@ -31,7 +31,15 @@ import { decorateMochaUi } from './decorate_mocha_ui'; * @param {String} path * @return {undefined} - mutates mocha, no return value */ -export const loadTestFiles = ({ mocha, log, lifecycle, providers, paths, updateBaselines }) => { +export const loadTestFiles = ({ + config, + mocha, + log, + lifecycle, + providers, + paths, + updateBaselines, +}) => { const innerLoadTestFile = path => { if (typeof path !== 'string' || !isAbsolute(path)) { throw new TypeError('loadTestFile() only accepts absolute paths'); @@ -55,7 +63,7 @@ export const loadTestFiles = ({ mocha, log, lifecycle, providers, paths, updateB loadTracer(provider, `testProvider[${path}]`, () => { // mocha.suite hocus-pocus comes from: https://git.io/vDnXO - const context = decorateMochaUi(lifecycle, global); + const context = decorateMochaUi(lifecycle, global, config); mocha.suite.emit('pre-require', context, path, mocha); const returnVal = provider({ diff --git a/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js b/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js index 61851cece0e8f..34e1290c22de9 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js +++ b/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js @@ -51,6 +51,7 @@ export async function setupMocha(lifecycle, log, config, providers) { log, lifecycle, providers, + config, paths: config.get('testFiles'), updateBaselines: config.get('updateBaselines'), }); diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts new file mode 100644 index 0000000000000..bdc5b1078bea0 --- /dev/null +++ b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts @@ -0,0 +1,123 @@ +/* + * 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 fs from 'fs'; +import { dirname, resolve, sep } from 'path'; + +import { REPO_ROOT } from '@kbn/dev-utils'; + +import { Lifecycle } from './lifecycle'; + +export interface TrackedSuiteMetadata { + startTime?: Date; + endTime?: Date; + duration?: number; + success?: boolean; +} + +export interface TrackedSuite { + config: string; + file: string; + tag: string; + title: string; + startTime: Date; + endTime: Date; + duration: number; + success: boolean; + leafSuite: boolean; +} + +const getTestMetadataPath = () => { + return process.env.TEST_METADATA_PATH || resolve(REPO_ROOT, 'target', 'test_metadata.json'); +}; + +export class TestTracker { + lifecycle: Lifecycle; + allSuites: Record> = {}; + trackedSuites: Map = new Map(); + + getTracked(suite: object): TrackedSuiteMetadata { + if (!this.trackedSuites.has(suite)) { + this.trackedSuites.set(suite, { success: true } as TrackedSuiteMetadata); + } + return this.trackedSuites.get(suite) || ({} as TrackedSuiteMetadata); + } + + constructor(lifecycle: Lifecycle) { + this.lifecycle = lifecycle; + + if (fs.existsSync(getTestMetadataPath())) { + fs.unlinkSync(getTestMetadataPath()); + } else { + fs.mkdirSync(dirname(getTestMetadataPath()), { recursive: true }); + } + + lifecycle.beforeTestSuite.add(suite => { + const tracked = this.getTracked(suite); + tracked.startTime = new Date(); + tracked.success = true; + }); + + const handleFailure = (error: any, test: any) => { + let parent = test.parent; + for (let i = 0; i < 100 && parent; i++) { + if (this.trackedSuites.has(parent)) { + this.getTracked(parent).success = false; + } + parent = parent.parent; + } + }; + + lifecycle.testFailure.add(handleFailure); + lifecycle.testHookFailure.add(handleFailure); + + lifecycle.afterTestSuite.add(suite => { + const tracked = this.getTracked(suite); + tracked.endTime = new Date(); + tracked.duration = tracked.endTime.getTime() - (tracked.startTime || new Date()).getTime(); + tracked.duration = Math.floor(tracked.duration / 1000); + + const config = suite.ftrConfig.path.replace(REPO_ROOT + sep, ''); + const file = suite.file.replace(REPO_ROOT + sep, ''); + + // TODO should non-leaf suite (e.g. index files) still be included here? is it confusing? + this.allSuites[config] = this.allSuites[config] || {}; + this.allSuites[config][file] = { + ...tracked, + config, + file, + tag: suite.suiteTag, + title: suite.title, + leafSuite: !!( + (suite.tests && suite.tests.length) || + (this.allSuites[config][file] && this.allSuites[config][file].leafSuite) + ), + } as TrackedSuite; + }); + + // TODO gate behind a CI env var or something + lifecycle.cleanup.add(() => { + const flattened: TrackedSuite[] = []; + Object.values(this.allSuites).forEach((x: Record) => + Object.values(x).forEach((y: TrackedSuite) => flattened.push(y)) + ); + flattened.sort((a, b) => b.duration - a.duration); + fs.writeFileSync(getTestMetadataPath(), JSON.stringify(flattened, null, 2)); + }); + } +} From 3895ccd2a5b57fe360991681a1915b3f2180a92b Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Fri, 3 Apr 2020 12:21:34 -0400 Subject: [PATCH 02/10] WIP finishing up test tracker and adding tests --- .../lib/test_tracker.test.ts | 82 +++++++++++++++++++ .../lib/test_tracker.ts | 55 ++++++++----- 2 files changed, 116 insertions(+), 21 deletions(-) create mode 100644 packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts new file mode 100644 index 0000000000000..eddf0548deb1f --- /dev/null +++ b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts @@ -0,0 +1,82 @@ +/* + * 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 fs from 'fs'; +import { join, resolve } from 'path'; + +jest.mock('fs'); +jest.mock('@kbn/dev-utils', () => { + return { REPO_ROOT: '/dev/null/root' }; +}); + +import { REPO_ROOT } from '@kbn/dev-utils'; +import { Lifecycle } from './lifecycle'; +import { TestTracker } from './test_tracker'; + +const MOCK_CONFIG_PATH = join('test', 'config.js'); +const MOCK_TEST_PATH = join('test', 'apps', 'test.js'); + +describe('TestTracker', () => { + let mocks: Record; + + const createMock = (overrides = {}) => { + return { + ftrConfig: { + path: resolve(REPO_ROOT, MOCK_CONFIG_PATH), + }, + file: resolve(REPO_ROOT, MOCK_TEST_PATH), + title: 'A Test', + suiteTag: MOCK_TEST_PATH, + ...overrides, + }; + }; + + beforeEach(() => { + mocks = { + WITH_TESTS: createMock({ tests: [{}] }), + WITHOUT_TESTS: createMock(), + }; + }); + + it('collects metadata for the current test', async () => { + const lifecycle = new Lifecycle(); + const testTracker = new TestTracker(lifecycle); + + const mockSuite = mocks.WITH_TESTS; + await lifecycle.beforeTestSuite.trigger(mockSuite); + await lifecycle.afterTestSuite.trigger(mockSuite); + // await lifecycle.cleanup.trigger(); + + const suites = testTracker.getAllFinishedSuites(); + expect(suites.length).toBe(1); + const suite = suites[0]; + + // [{"config": "test/config.js", "duration": 0, "endTime": 2020-04-03T16:10:59.115Z, "file": "test/apps/test.js", + // "leafSuite": true, "startTime": 2020-04-03T16:10:59.115Z, "success": true, "tag": undefined, "title": undefined}] + + // expect(fs.writeFileSync).toHaveBeenCalledWith(1); + expect(suite).toMatchObject({ + config: MOCK_CONFIG_PATH, + file: MOCK_TEST_PATH, + tag: MOCK_TEST_PATH, + leafSuite: true, + success: true, + }); + }); +}); diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts index bdc5b1078bea0..e8564896049d1 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts @@ -17,7 +17,7 @@ * under the License. */ import fs from 'fs'; -import { dirname, resolve, sep } from 'path'; +import { dirname, relative, resolve } from 'path'; import { REPO_ROOT } from '@kbn/dev-utils'; @@ -48,14 +48,14 @@ const getTestMetadataPath = () => { export class TestTracker { lifecycle: Lifecycle; - allSuites: Record> = {}; - trackedSuites: Map = new Map(); + finishedSuitesByConfig: Record> = {}; + inProgressSuites: Map = new Map(); getTracked(suite: object): TrackedSuiteMetadata { - if (!this.trackedSuites.has(suite)) { - this.trackedSuites.set(suite, { success: true } as TrackedSuiteMetadata); + if (!this.inProgressSuites.has(suite)) { + this.inProgressSuites.set(suite, { success: true } as TrackedSuiteMetadata); } - return this.trackedSuites.get(suite) || ({} as TrackedSuiteMetadata); + return this.inProgressSuites.get(suite) || ({} as TrackedSuiteMetadata); } constructor(lifecycle: Lifecycle) { @@ -73,10 +73,14 @@ export class TestTracker { tracked.success = true; }); + // If a test fails, we want to make sure all of the ancestors, all the way up to the root, get marked as failed + // This information is not available on the mocha objects without traversing all descendants of a given node const handleFailure = (error: any, test: any) => { let parent = test.parent; - for (let i = 0; i < 100 && parent; i++) { - if (this.trackedSuites.has(parent)) { + + // Infinite loop protection, just in case + for (let i = 0; i < 500 && parent; i++) { + if (this.inProgressSuites.has(parent)) { this.getTracked(parent).success = false; } parent = parent.parent; @@ -92,12 +96,14 @@ export class TestTracker { tracked.duration = tracked.endTime.getTime() - (tracked.startTime || new Date()).getTime(); tracked.duration = Math.floor(tracked.duration / 1000); - const config = suite.ftrConfig.path.replace(REPO_ROOT + sep, ''); - const file = suite.file.replace(REPO_ROOT + sep, ''); + const config = relative(REPO_ROOT, suite.ftrConfig.path); + const file = relative(REPO_ROOT, suite.file); + + this.finishedSuitesByConfig[config] = this.finishedSuitesByConfig[config] || {}; - // TODO should non-leaf suite (e.g. index files) still be included here? is it confusing? - this.allSuites[config] = this.allSuites[config] || {}; - this.allSuites[config][file] = { + // This will get called multiple times for a test file that has multiple describes in it or similar + // This is okay, because the last one that fires is always the root of the file, which is is the one we ultimately want + this.finishedSuitesByConfig[config][file] = { ...tracked, config, file, @@ -105,19 +111,26 @@ export class TestTracker { title: suite.title, leafSuite: !!( (suite.tests && suite.tests.length) || - (this.allSuites[config][file] && this.allSuites[config][file].leafSuite) + (this.finishedSuitesByConfig[config][file] && + this.finishedSuitesByConfig[config][file].leafSuite) ), } as TrackedSuite; }); - // TODO gate behind a CI env var or something lifecycle.cleanup.add(() => { - const flattened: TrackedSuite[] = []; - Object.values(this.allSuites).forEach((x: Record) => - Object.values(x).forEach((y: TrackedSuite) => flattened.push(y)) - ); - flattened.sort((a, b) => b.duration - a.duration); - fs.writeFileSync(getTestMetadataPath(), JSON.stringify(flattened, null, 2)); + const suites = this.getAllFinishedSuites(); + + fs.writeFileSync(getTestMetadataPath(), JSON.stringify(suites, null, 2)); }); } + + getAllFinishedSuites() { + const flattened: TrackedSuite[] = []; + Object.values(this.finishedSuitesByConfig).forEach((byFile: Record) => + Object.values(byFile).forEach((suite: TrackedSuite) => flattened.push(suite)) + ); + + flattened.sort((a, b) => b.duration - a.duration); + return flattened; + } } From 754ef194c8bbf79dc10810b4ff1a9dcd8ab1b534 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Tue, 7 Apr 2020 11:14:28 -0400 Subject: [PATCH 03/10] Add more ftr test tracker tests --- .../lib/test_tracker.test.ts | 83 +++++++++++++++---- 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts index eddf0548deb1f..8b26f20bef709 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts @@ -29,11 +29,36 @@ import { REPO_ROOT } from '@kbn/dev-utils'; import { Lifecycle } from './lifecycle'; import { TestTracker } from './test_tracker'; +const DEFAULT_TEST_METADATA_PATH = join(REPO_ROOT, 'target', 'test_metadata.json'); const MOCK_CONFIG_PATH = join('test', 'config.js'); const MOCK_TEST_PATH = join('test', 'apps', 'test.js'); +const ENVS_TO_RESET = ['TEST_METADATA_PATH']; describe('TestTracker', () => { - let mocks: Record; + const originalEnvs: Record = {}; + + beforeEach(() => { + for (const env of ENVS_TO_RESET) { + if (env in process.env) { + originalEnvs[env] = process.env[env] || ''; + delete process.env[env]; + } + } + }); + + afterEach(() => { + for (const env of ENVS_TO_RESET) { + delete process.env[env]; + } + + for (const env of Object.keys(originalEnvs)) { + process.env[env] = originalEnvs[env]; + } + + jest.resetAllMocks(); + }); + + let MOCKS: Record; const createMock = (overrides = {}) => { return { @@ -47,30 +72,35 @@ describe('TestTracker', () => { }; }; + const runLifecycleWithMocks = async (mocks: object[]) => { + const lifecycle = new Lifecycle(); + const testTracker = new TestTracker(lifecycle); + + for (const mock of mocks) { + await lifecycle.beforeTestSuite.trigger(mock); + } + + for (const mock of mocks.reverse()) { + await lifecycle.afterTestSuite.trigger(mock); + } + + return { lifecycle, testTracker }; + }; + beforeEach(() => { - mocks = { - WITH_TESTS: createMock({ tests: [{}] }), - WITHOUT_TESTS: createMock(), + MOCKS = { + WITH_TESTS: createMock({ tests: [{}] }), // i.e. a describe with tests in it + WITHOUT_TESTS: createMock(), // i.e. a describe with only other describes in it }; }); - it('collects metadata for the current test', async () => { - const lifecycle = new Lifecycle(); - const testTracker = new TestTracker(lifecycle); - - const mockSuite = mocks.WITH_TESTS; - await lifecycle.beforeTestSuite.trigger(mockSuite); - await lifecycle.afterTestSuite.trigger(mockSuite); - // await lifecycle.cleanup.trigger(); + it('collects metadata for a single suite with multiple describe()s', async () => { + const { testTracker } = await runLifecycleWithMocks([MOCKS.WITHOUT_TESTS, MOCKS.WITH_TESTS]); const suites = testTracker.getAllFinishedSuites(); expect(suites.length).toBe(1); const suite = suites[0]; - // [{"config": "test/config.js", "duration": 0, "endTime": 2020-04-03T16:10:59.115Z, "file": "test/apps/test.js", - // "leafSuite": true, "startTime": 2020-04-03T16:10:59.115Z, "success": true, "tag": undefined, "title": undefined}] - - // expect(fs.writeFileSync).toHaveBeenCalledWith(1); expect(suite).toMatchObject({ config: MOCK_CONFIG_PATH, file: MOCK_TEST_PATH, @@ -79,4 +109,25 @@ describe('TestTracker', () => { success: true, }); }); + + it('writes metadata to a file when cleanup is triggered', async () => { + const { lifecycle, testTracker } = await runLifecycleWithMocks([MOCKS.WITH_TESTS]); + await lifecycle.cleanup.trigger(); + + const suites = testTracker.getAllFinishedSuites(); + + const call = (fs.writeFileSync as jest.Mock).mock.calls[0]; + expect(call[0]).toEqual(DEFAULT_TEST_METADATA_PATH); + expect(call[1]).toEqual(JSON.stringify(suites, null, 2)); + }); + + it('respects TEST_METADATA_PATH env var for metadata target override', async () => { + process.env.TEST_METADATA_PATH = '/dev/null/fake-test-path'; + const { lifecycle } = await runLifecycleWithMocks([MOCKS.WITH_TESTS]); + await lifecycle.cleanup.trigger(); + + expect((fs.writeFileSync as jest.Mock).mock.calls[0][0]).toEqual( + process.env.TEST_METADATA_PATH + ); + }); }); From a0b4834fedcc8c0003fa26c887897e9b3961d9aa Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Tue, 7 Apr 2020 17:21:31 -0400 Subject: [PATCH 04/10] Add some more tests --- .../lib/test_tracker.test.ts | 68 ++++++++++++++++++- .../lib/test_tracker.ts | 2 +- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts index 8b26f20bef709..a90615f3ea08e 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts @@ -72,19 +72,25 @@ describe('TestTracker', () => { }; }; - const runLifecycleWithMocks = async (mocks: object[]) => { + const runLifecycleWithMocks = async (mocks: object[], fn: (objs: any) => any = () => {}) => { const lifecycle = new Lifecycle(); const testTracker = new TestTracker(lifecycle); + const ret = { lifecycle, testTracker }; + for (const mock of mocks) { await lifecycle.beforeTestSuite.trigger(mock); } + if (fn) { + fn(ret); + } + for (const mock of mocks.reverse()) { await lifecycle.afterTestSuite.trigger(mock); } - return { lifecycle, testTracker }; + return ret; }; beforeEach(() => { @@ -130,4 +136,62 @@ describe('TestTracker', () => { process.env.TEST_METADATA_PATH ); }); + + it('identifies suites with tests as leaf suites', async () => { + const root = createMock({ title: 'root', file: join(REPO_ROOT, 'root.js') }); + const parent = createMock({ parent: root }); + const withTests = createMock({ parent, tests: [{}] }); + + const { testTracker } = await runLifecycleWithMocks([root, parent, withTests]); + const suites = testTracker.getAllFinishedSuites(); + + const finishedRoot = suites.find(s => s.title === 'root'); + const finishedWithTests = suites.find(s => s.title !== 'root'); + + expect(finishedRoot).toBeTruthy(); + expect(finishedRoot?.leafSuite).toBeFalsy(); + expect(finishedWithTests?.leafSuite).toBe(true); + }); + + describe('with a failing suite', () => { + let root: any; + let parent: any; + let failed: any; + + beforeEach(() => { + root = createMock({ file: join(REPO_ROOT, 'root.js') }); + parent = createMock({ parent: root }); + failed = createMock({ parent, tests: [{}] }); + }); + + it('marks parent suites as not successful when a test fails', async () => { + const { testTracker } = await runLifecycleWithMocks( + [root, parent, failed], + async ({ lifecycle }) => { + await lifecycle.testFailure.trigger(Error('test'), { parent: failed }); + } + ); + + const suites = testTracker.getAllFinishedSuites(); + expect(suites.length).toBe(2); + for (const suite of suites) { + expect(suite.success).toBeFalsy(); + } + }); + + it('marks parent suites as not successful when a test hook fails', async () => { + const { testTracker } = await runLifecycleWithMocks( + [root, parent, failed], + async ({ lifecycle }) => { + await lifecycle.testHookFailure.trigger(Error('test'), { parent: failed }); + } + ); + + const suites = testTracker.getAllFinishedSuites(); + expect(suites.length).toBe(2); + for (const suite of suites) { + expect(suite.success).toBeFalsy(); + } + }); + }); }); diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts index e8564896049d1..b1fee25143b5e 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts @@ -75,7 +75,7 @@ export class TestTracker { // If a test fails, we want to make sure all of the ancestors, all the way up to the root, get marked as failed // This information is not available on the mocha objects without traversing all descendants of a given node - const handleFailure = (error: any, test: any) => { + const handleFailure = (_: any, test: any) => { let parent = test.parent; // Infinite loop protection, just in case From f75d4ca882920e704db0ab603f0b4a5af4d08ba2 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Wed, 8 Apr 2020 22:39:47 -0400 Subject: [PATCH 05/10] A few small re-factors --- .../functional_test_runner.ts | 4 +-- .../src/functional_test_runner/lib/index.ts | 2 +- ..._tracker.test.ts => suite_tracker.test.ts} | 28 +++++++++---------- .../lib/{test_tracker.ts => suite_tracker.ts} | 28 ++++++++++--------- 4 files changed, 32 insertions(+), 30 deletions(-) rename packages/kbn-test/src/functional_test_runner/lib/{test_tracker.test.ts => suite_tracker.test.ts} (86%) rename packages/kbn-test/src/functional_test_runner/lib/{test_tracker.ts => suite_tracker.ts} (84%) diff --git a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts index 19e18f9831b70..aa71fd96bc5c6 100644 --- a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts +++ b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts @@ -30,13 +30,13 @@ import { setupMocha, runTests, Config, - TestTracker, + SuiteTracker, } from './lib'; export class FunctionalTestRunner { public readonly lifecycle = new Lifecycle(); public readonly failureMetadata = new FailureMetadata(this.lifecycle); - public readonly testTracker = new TestTracker(this.lifecycle); + public readonly testTracker = new SuiteTracker(this.lifecycle); private closed = false; constructor( diff --git a/packages/kbn-test/src/functional_test_runner/lib/index.ts b/packages/kbn-test/src/functional_test_runner/lib/index.ts index 4ba0077c2f008..2e534974e1d76 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/index.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/index.ts @@ -23,4 +23,4 @@ export { readConfigFile, Config } from './config'; export { readProviderSpec, ProviderCollection, Provider } from './providers'; export { runTests, setupMocha } from './mocha'; export { FailureMetadata } from './failure_metadata'; -export { TestTracker } from './test_tracker'; +export { SuiteTracker } from './suite_tracker'; diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts similarity index 86% rename from packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts rename to packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts index a90615f3ea08e..9aeb0554a91e3 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.test.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts @@ -27,14 +27,14 @@ jest.mock('@kbn/dev-utils', () => { import { REPO_ROOT } from '@kbn/dev-utils'; import { Lifecycle } from './lifecycle'; -import { TestTracker } from './test_tracker'; +import { SuiteTracker } from './suite_tracker'; const DEFAULT_TEST_METADATA_PATH = join(REPO_ROOT, 'target', 'test_metadata.json'); const MOCK_CONFIG_PATH = join('test', 'config.js'); const MOCK_TEST_PATH = join('test', 'apps', 'test.js'); const ENVS_TO_RESET = ['TEST_METADATA_PATH']; -describe('TestTracker', () => { +describe('SuiteTracker', () => { const originalEnvs: Record = {}; beforeEach(() => { @@ -74,9 +74,9 @@ describe('TestTracker', () => { const runLifecycleWithMocks = async (mocks: object[], fn: (objs: any) => any = () => {}) => { const lifecycle = new Lifecycle(); - const testTracker = new TestTracker(lifecycle); + const suiteTracker = new SuiteTracker(lifecycle); - const ret = { lifecycle, testTracker }; + const ret = { lifecycle, suiteTracker }; for (const mock of mocks) { await lifecycle.beforeTestSuite.trigger(mock); @@ -101,9 +101,9 @@ describe('TestTracker', () => { }); it('collects metadata for a single suite with multiple describe()s', async () => { - const { testTracker } = await runLifecycleWithMocks([MOCKS.WITHOUT_TESTS, MOCKS.WITH_TESTS]); + const { suiteTracker } = await runLifecycleWithMocks([MOCKS.WITHOUT_TESTS, MOCKS.WITH_TESTS]); - const suites = testTracker.getAllFinishedSuites(); + const suites = suiteTracker.getAllFinishedSuites(); expect(suites.length).toBe(1); const suite = suites[0]; @@ -117,10 +117,10 @@ describe('TestTracker', () => { }); it('writes metadata to a file when cleanup is triggered', async () => { - const { lifecycle, testTracker } = await runLifecycleWithMocks([MOCKS.WITH_TESTS]); + const { lifecycle, suiteTracker } = await runLifecycleWithMocks([MOCKS.WITH_TESTS]); await lifecycle.cleanup.trigger(); - const suites = testTracker.getAllFinishedSuites(); + const suites = suiteTracker.getAllFinishedSuites(); const call = (fs.writeFileSync as jest.Mock).mock.calls[0]; expect(call[0]).toEqual(DEFAULT_TEST_METADATA_PATH); @@ -142,8 +142,8 @@ describe('TestTracker', () => { const parent = createMock({ parent: root }); const withTests = createMock({ parent, tests: [{}] }); - const { testTracker } = await runLifecycleWithMocks([root, parent, withTests]); - const suites = testTracker.getAllFinishedSuites(); + const { suiteTracker } = await runLifecycleWithMocks([root, parent, withTests]); + const suites = suiteTracker.getAllFinishedSuites(); const finishedRoot = suites.find(s => s.title === 'root'); const finishedWithTests = suites.find(s => s.title !== 'root'); @@ -165,14 +165,14 @@ describe('TestTracker', () => { }); it('marks parent suites as not successful when a test fails', async () => { - const { testTracker } = await runLifecycleWithMocks( + const { suiteTracker } = await runLifecycleWithMocks( [root, parent, failed], async ({ lifecycle }) => { await lifecycle.testFailure.trigger(Error('test'), { parent: failed }); } ); - const suites = testTracker.getAllFinishedSuites(); + const suites = suiteTracker.getAllFinishedSuites(); expect(suites.length).toBe(2); for (const suite of suites) { expect(suite.success).toBeFalsy(); @@ -180,14 +180,14 @@ describe('TestTracker', () => { }); it('marks parent suites as not successful when a test hook fails', async () => { - const { testTracker } = await runLifecycleWithMocks( + const { suiteTracker } = await runLifecycleWithMocks( [root, parent, failed], async ({ lifecycle }) => { await lifecycle.testHookFailure.trigger(Error('test'), { parent: failed }); } ); - const suites = testTracker.getAllFinishedSuites(); + const suites = suiteTracker.getAllFinishedSuites(); expect(suites.length).toBe(2); for (const suite of suites) { expect(suite.success).toBeFalsy(); diff --git a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts similarity index 84% rename from packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts rename to packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts index b1fee25143b5e..8cdb631a706bc 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/test_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts @@ -23,14 +23,14 @@ import { REPO_ROOT } from '@kbn/dev-utils'; import { Lifecycle } from './lifecycle'; -export interface TrackedSuiteMetadata { +export interface SuiteInProgress { startTime?: Date; endTime?: Date; duration?: number; success?: boolean; } -export interface TrackedSuite { +export interface SuiteWithMetadata { config: string; file: string; tag: string; @@ -46,16 +46,16 @@ const getTestMetadataPath = () => { return process.env.TEST_METADATA_PATH || resolve(REPO_ROOT, 'target', 'test_metadata.json'); }; -export class TestTracker { +export class SuiteTracker { lifecycle: Lifecycle; - finishedSuitesByConfig: Record> = {}; - inProgressSuites: Map = new Map(); + finishedSuitesByConfig: Record> = {}; + inProgressSuites: Map = new Map(); - getTracked(suite: object): TrackedSuiteMetadata { + getTracked(suite: object): SuiteInProgress { if (!this.inProgressSuites.has(suite)) { - this.inProgressSuites.set(suite, { success: true } as TrackedSuiteMetadata); + this.inProgressSuites.set(suite, { success: true } as SuiteInProgress); } - return this.inProgressSuites.get(suite) || ({} as TrackedSuiteMetadata); + return this.inProgressSuites.get(suite) || ({} as SuiteInProgress); } constructor(lifecycle: Lifecycle) { @@ -114,7 +114,7 @@ export class TestTracker { (this.finishedSuitesByConfig[config][file] && this.finishedSuitesByConfig[config][file].leafSuite) ), - } as TrackedSuite; + } as SuiteWithMetadata; }); lifecycle.cleanup.add(() => { @@ -125,10 +125,12 @@ export class TestTracker { } getAllFinishedSuites() { - const flattened: TrackedSuite[] = []; - Object.values(this.finishedSuitesByConfig).forEach((byFile: Record) => - Object.values(byFile).forEach((suite: TrackedSuite) => flattened.push(suite)) - ); + const flattened: SuiteWithMetadata[] = []; + for (const byFile of Object.values(this.finishedSuitesByConfig)) { + for (const suite of Object.values(byFile)) { + flattened.push(suite); + } + } flattened.sort((a, b) => b.duration - a.duration); return flattened; From e93837cdbecb16e63cc3599c8465b34291148eee Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Wed, 8 Apr 2020 22:44:23 -0400 Subject: [PATCH 06/10] Remove duration from interface, unnecessary --- .../src/functional_test_runner/lib/suite_tracker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts index 8cdb631a706bc..076bb40acad1b 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts @@ -26,7 +26,6 @@ import { Lifecycle } from './lifecycle'; export interface SuiteInProgress { startTime?: Date; endTime?: Date; - duration?: number; success?: boolean; } @@ -93,8 +92,8 @@ export class SuiteTracker { lifecycle.afterTestSuite.add(suite => { const tracked = this.getTracked(suite); tracked.endTime = new Date(); - tracked.duration = tracked.endTime.getTime() - (tracked.startTime || new Date()).getTime(); - tracked.duration = Math.floor(tracked.duration / 1000); + let duration = tracked.endTime.getTime() - (tracked.startTime || new Date()).getTime(); + duration = Math.floor(duration / 1000); const config = relative(REPO_ROOT, suite.ftrConfig.path); const file = relative(REPO_ROOT, suite.file); @@ -105,6 +104,7 @@ export class SuiteTracker { // This is okay, because the last one that fires is always the root of the file, which is is the one we ultimately want this.finishedSuitesByConfig[config][file] = { ...tracked, + duration, config, file, tag: suite.suiteTag, From 8833f68f3d0b05d088e678517dd4f969df317f45 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Wed, 8 Apr 2020 22:46:51 -0400 Subject: [PATCH 07/10] Fix variable name --- .../src/functional_test_runner/functional_test_runner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts index aa71fd96bc5c6..51e76d5cc025a 100644 --- a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts +++ b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts @@ -36,7 +36,7 @@ import { export class FunctionalTestRunner { public readonly lifecycle = new Lifecycle(); public readonly failureMetadata = new FailureMetadata(this.lifecycle); - public readonly testTracker = new SuiteTracker(this.lifecycle); + public readonly suiteTracker = new SuiteTracker(this.lifecycle); private closed = false; constructor( From 953ff5e52b4a964349eb85362b2866b6c4d8c392 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Apr 2020 14:02:04 -0400 Subject: [PATCH 08/10] Refactors based on PR feedback --- .../functional_test_runner.ts | 3 ++- .../lib/config/config.ts | 3 --- .../lib/mocha/decorate_mocha_ui.js | 4 +--- .../lib/mocha/load_test_files.js | 12 ++-------- .../lib/mocha/setup_mocha.js | 1 - .../lib/suite_tracker.test.ts | 8 +++---- .../lib/suite_tracker.ts | 24 ++++++++++++------- 7 files changed, 25 insertions(+), 30 deletions(-) diff --git a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts index 51e76d5cc025a..3a66ba22ccf3d 100644 --- a/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts +++ b/packages/kbn-test/src/functional_test_runner/functional_test_runner.ts @@ -36,7 +36,6 @@ import { export class FunctionalTestRunner { public readonly lifecycle = new Lifecycle(); public readonly failureMetadata = new FailureMetadata(this.lifecycle); - public readonly suiteTracker = new SuiteTracker(this.lifecycle); private closed = false; constructor( @@ -54,6 +53,8 @@ export class FunctionalTestRunner { async run() { return await this._run(async (config, coreProviders) => { + SuiteTracker.startTracking(this.lifecycle, this.configFile); + const providers = new ProviderCollection(this.log, [ ...coreProviders, ...readProviderSpec('Service', config.get('services')), diff --git a/packages/kbn-test/src/functional_test_runner/lib/config/config.ts b/packages/kbn-test/src/functional_test_runner/lib/config/config.ts index 33adadf3c7dae..ad9247523797a 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/config/config.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/config/config.ts @@ -35,7 +35,6 @@ interface Options { export class Config { private [$values]: Record; - public path: string; constructor(options: Options) { const { settings = {}, primary = false, path = null } = options || {}; @@ -44,8 +43,6 @@ export class Config { throw new TypeError('path is a required option'); } - this.path = path; - const { error, value } = schema.validate(settings, { abortEarly: false, context: { diff --git a/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js b/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js index ebd85327495be..1cac852a7e713 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js +++ b/packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js @@ -22,7 +22,7 @@ import { createAssignmentProxy } from './assignment_proxy'; import { wrapFunction } from './wrap_function'; import { wrapRunnableArgs } from './wrap_runnable_args'; -export function decorateMochaUi(lifecycle, context, config) { +export function decorateMochaUi(lifecycle, context) { // incremented at the start of each suite, decremented after // so that in each non-suite call we can know if we are within // a suite, or that when a suite is defined it is within a suite @@ -70,8 +70,6 @@ export function decorateMochaUi(lifecycle, context, config) { this.tags(relativeFilePath); this.suiteTag = relativeFilePath; // The tag that uniquely targets this suite/file - this.ftrConfig = config; - provider.call(this); after(async () => { diff --git a/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js b/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js index 153a83920f483..6ee65b1b7e394 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js +++ b/packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js @@ -31,15 +31,7 @@ import { decorateMochaUi } from './decorate_mocha_ui'; * @param {String} path * @return {undefined} - mutates mocha, no return value */ -export const loadTestFiles = ({ - config, - mocha, - log, - lifecycle, - providers, - paths, - updateBaselines, -}) => { +export const loadTestFiles = ({ mocha, log, lifecycle, providers, paths, updateBaselines }) => { const innerLoadTestFile = path => { if (typeof path !== 'string' || !isAbsolute(path)) { throw new TypeError('loadTestFile() only accepts absolute paths'); @@ -63,7 +55,7 @@ export const loadTestFiles = ({ loadTracer(provider, `testProvider[${path}]`, () => { // mocha.suite hocus-pocus comes from: https://git.io/vDnXO - const context = decorateMochaUi(lifecycle, global, config); + const context = decorateMochaUi(lifecycle, global); mocha.suite.emit('pre-require', context, path, mocha); const returnVal = provider({ diff --git a/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js b/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js index 34e1290c22de9..61851cece0e8f 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js +++ b/packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js @@ -51,7 +51,6 @@ export async function setupMocha(lifecycle, log, config, providers) { log, lifecycle, providers, - config, paths: config.get('testFiles'), updateBaselines: config.get('updateBaselines'), }); diff --git a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts index 9aeb0554a91e3..6f1cf3d8f2961 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts @@ -62,9 +62,6 @@ describe('SuiteTracker', () => { const createMock = (overrides = {}) => { return { - ftrConfig: { - path: resolve(REPO_ROOT, MOCK_CONFIG_PATH), - }, file: resolve(REPO_ROOT, MOCK_TEST_PATH), title: 'A Test', suiteTag: MOCK_TEST_PATH, @@ -74,7 +71,10 @@ describe('SuiteTracker', () => { const runLifecycleWithMocks = async (mocks: object[], fn: (objs: any) => any = () => {}) => { const lifecycle = new Lifecycle(); - const suiteTracker = new SuiteTracker(lifecycle); + const suiteTracker = SuiteTracker.startTracking( + lifecycle, + resolve(REPO_ROOT, MOCK_CONFIG_PATH) + ); const ret = { lifecycle, suiteTracker }; diff --git a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts index 076bb40acad1b..f912e71302766 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts @@ -46,30 +46,33 @@ const getTestMetadataPath = () => { }; export class SuiteTracker { - lifecycle: Lifecycle; finishedSuitesByConfig: Record> = {}; inProgressSuites: Map = new Map(); + static startTracking(lifecycle: Lifecycle, configPath: string): SuiteTracker { + const suiteTracker = new SuiteTracker(lifecycle, configPath); + return suiteTracker; + } + getTracked(suite: object): SuiteInProgress { if (!this.inProgressSuites.has(suite)) { - this.inProgressSuites.set(suite, { success: true } as SuiteInProgress); + this.inProgressSuites.set(suite, {} as SuiteInProgress); } - return this.inProgressSuites.get(suite) || ({} as SuiteInProgress); + return this.inProgressSuites.get(suite)!; } - constructor(lifecycle: Lifecycle) { - this.lifecycle = lifecycle; - + constructor(lifecycle: Lifecycle, configPathAbsolute: string) { if (fs.existsSync(getTestMetadataPath())) { fs.unlinkSync(getTestMetadataPath()); } else { fs.mkdirSync(dirname(getTestMetadataPath()), { recursive: true }); } + const config = relative(REPO_ROOT, configPathAbsolute); + lifecycle.beforeTestSuite.add(suite => { const tracked = this.getTracked(suite); tracked.startTime = new Date(); - tracked.success = true; }); // If a test fails, we want to make sure all of the ancestors, all the way up to the root, get marked as failed @@ -92,10 +95,15 @@ export class SuiteTracker { lifecycle.afterTestSuite.add(suite => { const tracked = this.getTracked(suite); tracked.endTime = new Date(); + + // The suite ended without any children failing, so we can mark it as successful + if (!('success' in tracked)) { + tracked.success = true; + } + let duration = tracked.endTime.getTime() - (tracked.startTime || new Date()).getTime(); duration = Math.floor(duration / 1000); - const config = relative(REPO_ROOT, suite.ftrConfig.path); const file = relative(REPO_ROOT, suite.file); this.finishedSuitesByConfig[config] = this.finishedSuitesByConfig[config] || {}; From af7b217482baf6c95c45140322be0cadc3a8e90d Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Apr 2020 17:15:17 -0400 Subject: [PATCH 09/10] Make the success:undefined check more explicit --- .../kbn-test/src/functional_test_runner/lib/suite_tracker.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts index f912e71302766..0891d7dbe8b2f 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts @@ -56,7 +56,7 @@ export class SuiteTracker { getTracked(suite: object): SuiteInProgress { if (!this.inProgressSuites.has(suite)) { - this.inProgressSuites.set(suite, {} as SuiteInProgress); + this.inProgressSuites.set(suite, { success: undefined } as SuiteInProgress); } return this.inProgressSuites.get(suite)!; } @@ -97,7 +97,7 @@ export class SuiteTracker { tracked.endTime = new Date(); // The suite ended without any children failing, so we can mark it as successful - if (!('success' in tracked)) { + if (typeof tracked.success === 'undefined') { tracked.success = true; } From 527752404af24ab1645767d709eb3f1500c43001 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Fri, 17 Apr 2020 11:10:23 -0400 Subject: [PATCH 10/10] A little more PR feedback --- .../src/functional_test_runner/lib/suite_tracker.test.ts | 8 ++++---- .../src/functional_test_runner/lib/suite_tracker.ts | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts index 6f1cf3d8f2961..b6c2c0a6d511d 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.test.ts @@ -111,7 +111,7 @@ describe('SuiteTracker', () => { config: MOCK_CONFIG_PATH, file: MOCK_TEST_PATH, tag: MOCK_TEST_PATH, - leafSuite: true, + hasTests: true, success: true, }); }); @@ -128,7 +128,7 @@ describe('SuiteTracker', () => { }); it('respects TEST_METADATA_PATH env var for metadata target override', async () => { - process.env.TEST_METADATA_PATH = '/dev/null/fake-test-path'; + process.env.TEST_METADATA_PATH = resolve(REPO_ROOT, '../fake-test-path'); const { lifecycle } = await runLifecycleWithMocks([MOCKS.WITH_TESTS]); await lifecycle.cleanup.trigger(); @@ -149,8 +149,8 @@ describe('SuiteTracker', () => { const finishedWithTests = suites.find(s => s.title !== 'root'); expect(finishedRoot).toBeTruthy(); - expect(finishedRoot?.leafSuite).toBeFalsy(); - expect(finishedWithTests?.leafSuite).toBe(true); + expect(finishedRoot?.hasTests).toBeFalsy(); + expect(finishedWithTests?.hasTests).toBe(true); }); describe('with a failing suite', () => { diff --git a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts index 0891d7dbe8b2f..8967251ea78de 100644 --- a/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts +++ b/packages/kbn-test/src/functional_test_runner/lib/suite_tracker.ts @@ -38,7 +38,7 @@ export interface SuiteWithMetadata { endTime: Date; duration: number; success: boolean; - leafSuite: boolean; + hasTests: boolean; } const getTestMetadataPath = () => { @@ -117,10 +117,11 @@ export class SuiteTracker { file, tag: suite.suiteTag, title: suite.title, - leafSuite: !!( + hasTests: !!( (suite.tests && suite.tests.length) || + // The below statement is so that `hasTests` will bubble up nested describes in the same file (this.finishedSuitesByConfig[config][file] && - this.finishedSuitesByConfig[config][file].leafSuite) + this.finishedSuitesByConfig[config][file].hasTests) ), } as SuiteWithMetadata; });