Refactors based on PR feedback

elastic · Apr 16, 2020 · 953ff5e · 953ff5e
1 parent fff4a5f
commit 953ff5e
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 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
@@ -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
@@ -35,7 +35,6 @@ interface Options {
 
 export class Config {
   private [$values]: Record<string, any>;
-  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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -46,30 +46,33 @@ const getTestMetadataPath = () => {
 };
 
 export class SuiteTracker {
-  lifecycle: Lifecycle;
   finishedSuitesByConfig: Record<string, Record<string, SuiteWithMetadata>> = {};
   inProgressSuites: Map<object, SuiteInProgress> = new Map<object, SuiteInProgress>();
 
+  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] || {};