chore(test-runner): misc changes to reporter api (#7664)

- `Location` with `file`, `line` and `column`. - `fullTitle` does not include project name. - `titlePath` method. - All methods of `Reporter` are optional. - Removed `Test.skipped` property that is superseeded by `Test.status()`. - Replaced `Suite.findTest()` with `Suite.allTests()`. - Removed `Test.suite` property.
microsoft · Jul 16, 2021 · 31572fc · 31572fc
1 parent 09764a4
commit 31572fc
Show file tree

Hide file tree

Showing 17 changed files with 123 additions and 161 deletions.
diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts
@@ -50,7 +50,7 @@ export class Dispatcher {
 
     this._suite = suite;
     for (const suite of this._suite.suites) {
-      for (const test of suite._allTests())
+      for (const test of suite.allTests())
         this._testById.set(test._id, { test, result: test._appendTestResult() });
     }
 
@@ -59,7 +59,7 @@ export class Dispatcher {
     // Shard tests.
     const shard = this._loader.fullConfig().shard;
     if (shard) {
-      let total = this._suite.totalTestCount();
+      let total = this._suite.allTests().length;
       const shardSize = Math.ceil(total / shard.total);
       const from = shardSize * shard.current;
       const to = shardSize * (shard.current + 1);
@@ -81,7 +81,7 @@ export class Dispatcher {
     const entriesByWorkerHashAndFile = new Map<string, Map<string, DispatcherEntry>>();
     for (const fileSuite of this._suite.suites) {
       const file = fileSuite._requireFile;
-      for (const test of fileSuite._allTests()) {
+      for (const test of fileSuite.allTests()) {
         let entriesByFile = entriesByWorkerHashAndFile.get(test._workerHash);
         if (!entriesByFile) {
           entriesByFile = new Map();
@@ -275,27 +275,25 @@ export class Dispatcher {
       test.expectedStatus = params.expectedStatus;
       test.annotations = params.annotations;
       test.timeout = params.timeout;
-      if (params.expectedStatus === 'skipped' && params.status === 'skipped')
-        test.skipped = true;
       this._reportTestEnd(test, result, params.status);
     });
     worker.on('stdOut', (params: TestOutputPayload) => {
       const chunk = chunkFromParams(params);
       const pair = params.testId ? this._testById.get(params.testId) : undefined;
       if (pair)
         pair.result.stdout.push(chunk);
-      this._reporter.onStdOut(chunk, pair ? pair.test : undefined);
+      this._reporter.onStdOut?.(chunk, pair ? pair.test : undefined);
     });
     worker.on('stdErr', (params: TestOutputPayload) => {
       const chunk = chunkFromParams(params);
       const pair = params.testId ? this._testById.get(params.testId) : undefined;
       if (pair)
         pair.result.stderr.push(chunk);
-      this._reporter.onStdErr(chunk, pair ? pair.test : undefined);
+      this._reporter.onStdErr?.(chunk, pair ? pair.test : undefined);
     });
     worker.on('teardownError', ({error}) => {
       this._hasWorkerErrors = true;
-      this._reporter.onError(error);
+      this._reporter.onError?.(error);
     });
     worker.on('exit', () => {
       this._workers.delete(worker);
@@ -322,7 +320,7 @@ export class Dispatcher {
       return;
     const maxFailures = this._loader.fullConfig().maxFailures;
     if (!maxFailures || this._failureCount < maxFailures)
-      this._reporter.onTestBegin(test);
+      this._reporter.onTestBegin?.(test);
   }
 
   private _reportTestEnd(test: Test, result: TestResult, status: TestStatus) {
@@ -333,7 +331,7 @@ export class Dispatcher {
       ++this._failureCount;
     const maxFailures = this._loader.fullConfig().maxFailures;
     if (!maxFailures || this._failureCount <= maxFailures)
-      this._reporter.onTestEnd(test, result);
+      this._reporter.onTestEnd?.(test, result);
     if (maxFailures && this._failureCount === maxFailures)
       this.stop().catch(e => {});
   }

diff --git a/src/test/loader.ts b/src/test/loader.ts
@@ -112,7 +112,7 @@ export class Loader {
     try {
       const suite = new Suite('');
       suite._requireFile = file;
-      suite.file = file;
+      suite.location.file = file;
       setCurrentlyLoadingFileSuite(suite);
       await this._requireOrImport(file);
       this._fileSuites.set(file, suite);

diff --git a/src/test/project.ts b/src/test/project.ts
@@ -60,17 +60,14 @@ export class ProjectImpl {
       if (Object.entries(overrides).length) {
         const overridesWithLocation = {
           fixtures: overrides,
-          location: {
-            file: test.file,
-            line: 1,  // TODO: capture location
-            column: 1,  // TODO: capture location
-          }
+          // TODO: pass location from test.use() callsite.
+          location: test.location,
         };
         pool = new FixturePool([overridesWithLocation], pool);
       }
       this.testPools.set(test, pool);
 
-      pool.validateFunction(test.fn, 'Test', true, test);
+      pool.validateFunction(test.fn, 'Test', true, test.location);
       for (let parent = test.parent; parent; parent = parent.parent) {
         for (const hook of parent._hooks)
           pool.validateFunction(hook.fn, hook.type + ' hook', hook.type === 'beforeEach' || hook.type === 'afterEach', hook.location);
@@ -98,7 +95,7 @@ export class ProjectImpl {
         test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`;
         test._id = `${entry._ordinalInFile}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`;
         test._pool = pool;
-        test._buildFullTitle(suite.fullTitle());
+        test._buildTitlePath(suite._titlePath);
         if (!filter(test))
           continue;
         result._addTest(test);

diff --git a/src/test/reporter.ts b/src/test/reporter.ts
@@ -17,29 +17,30 @@
 import type { FullConfig, TestStatus, TestError } from './types';
 export type { FullConfig, TestStatus, TestError } from './types';
 
-export interface Suite {
-  title: string;
+export interface Location {
   file: string;
   line: number;
   column: number;
+}
+export interface Suite {
+  title: string;
+  location: Location;
   suites: Suite[];
   tests: Test[];
-  findTest(fn: (test: Test) => boolean | void): boolean;
-  totalTestCount(): number;
+  titlePath(): string[];
+  fullTitle(): string;
+  allTests(): Test[];
 }
 export interface Test {
-  suite: Suite;
   title: string;
-  file: string;
-  line: number;
-  column: number;
+  location: Location;
   results: TestResult[];
-  skipped: boolean;
   expectedStatus: TestStatus;
   timeout: number;
   annotations: { type: string, description?: string }[];
   projectName: string;
   retries: number;
+  titlePath(): string[];
   fullTitle(): string;
   status(): 'skipped' | 'expected' | 'unexpected' | 'flaky';
   ok(): boolean;
@@ -58,11 +59,11 @@ export interface FullResult {
   status: 'passed' | 'failed' | 'timedout' | 'interrupted';
 }
 export interface Reporter {
-  onBegin(config: FullConfig, suite: Suite): void;
-  onTestBegin(test: Test): void;
-  onStdOut(chunk: string | Buffer, test?: Test): void;
-  onStdErr(chunk: string | Buffer, test?: Test): void;
-  onTestEnd(test: Test, result: TestResult): void;
-  onError(error: TestError): void;
-  onEnd(result: FullResult): void | Promise<void>;
+  onBegin?(config: FullConfig, suite: Suite): void;
+  onTestBegin?(test: Test): void;
+  onStdOut?(chunk: string | Buffer, test?: Test): void;
+  onStdErr?(chunk: string | Buffer, test?: Test): void;
+  onTestEnd?(test: Test, result: TestResult): void;
+  onError?(error: TestError): void;
+  onEnd?(result: FullResult): void | Promise<void>;
 }
diff --git a/src/test/reporters/base.ts b/src/test/reporters/base.ts
@@ -33,18 +33,12 @@ export class BaseReporter implements Reporter  {
   fileDurations = new Map<string, number>();
   monotonicStartTime: number = 0;
 
-  constructor() {
-  }
-
   onBegin(config: FullConfig, suite: Suite) {
     this.monotonicStartTime = monotonicTime();
     this.config = config;
     this.suite = suite;
   }
 
-  onTestBegin(test: Test) {
-  }
-
   onStdOut(chunk: string | Buffer) {
     if (!this.config.quiet)
       process.stdout.write(chunk);
@@ -91,7 +85,7 @@ export class BaseReporter implements Reporter  {
     const unexpected: Test[] = [];
     const flaky: Test[] = [];
 
-    this.suite.findTest(test => {
+    this.suite.allTests().forEach(test => {
       switch (test.status()) {
         case 'skipped': ++skipped; break;
         case 'expected': ++expected; break;
@@ -158,13 +152,14 @@ export function formatFailure(config: FullConfig, test: Test, index?: number): s
 }
 
 function relativeTestPath(config: FullConfig, test: Test): string {
-  return path.relative(config.rootDir, test.file) || path.basename(test.file);
+  return path.relative(config.rootDir, test.location.file) || path.basename(test.location.file);
 }
 
 export function formatTestTitle(config: FullConfig, test: Test): string {
   let relativePath = relativeTestPath(config, test);
-  relativePath += ':' + test.line + ':' + test.column;
-  return `${relativePath} › ${test.fullTitle()}`;
+  relativePath += ':' + test.location.line + ':' + test.location.column;
+  const title = (test.projectName ? `[${test.projectName}] ` : '') + test.fullTitle();
+  return `${relativePath} › ${title}`;
 }
 
 function formatTestHeader(config: FullConfig, test: Test, indent: string, index?: number): string {
@@ -182,9 +177,9 @@ function formatFailedResult(test: Test, result: TestResult): string {
     tokens.push('');
     tokens.push(indent(colors.red(`Timeout of ${test.timeout}ms exceeded.`), '    '));
     if (result.error !== undefined)
-      tokens.push(indent(formatError(result.error, test.file), '    '));
+      tokens.push(indent(formatError(result.error, test.location.file), '    '));
   } else {
-    tokens.push(indent(formatError(result.error!, test.file), '    '));
+    tokens.push(indent(formatError(result.error!, test.location.file), '    '));
   }
   return tokens.join('\n');
 }

diff --git a/src/test/reporters/empty.ts b/src/test/reporters/empty.ts
@@ -14,16 +14,9 @@
  * limitations under the License.
  */
 
-import { FullConfig, TestResult, Test, Suite, TestError, Reporter, FullResult } from '../reporter';
+import { Reporter } from '../reporter';
 
 class EmptyReporter implements Reporter {
-  onBegin(config: FullConfig, suite: Suite) {}
-  onTestBegin(test: Test) {}
-  onStdOut(chunk: string | Buffer, test?: Test) {}
-  onStdErr(chunk: string | Buffer, test?: Test) {}
-  onTestEnd(test: Test, result: TestResult) {}
-  onError(error: TestError) {}
-  async onEnd(result: FullResult) {}
 }
 
 export default EmptyReporter;
diff --git a/src/test/reporters/json.ts b/src/test/reporters/json.ts
@@ -16,8 +16,7 @@
 
 import fs from 'fs';
 import path from 'path';
-import EmptyReporter from './empty';
-import { FullConfig, Test, Suite, TestResult, TestError, FullResult, TestStatus } from '../reporter';
+import { FullConfig, Test, Suite, TestResult, TestError, FullResult, TestStatus, Location, Reporter } from '../reporter';
 
 export interface JSONReport {
   config: Omit<FullConfig, 'projects'> & {
@@ -76,14 +75,13 @@ function toPosixPath(aPath: string): string {
   return aPath.split(path.sep).join(path.posix.sep);
 }
 
-class JSONReporter extends EmptyReporter {
+class JSONReporter implements Reporter {
   config!: FullConfig;
   suite!: Suite;
   private _errors: TestError[] = [];
   private _outputFile: string | undefined;
 
   constructor(options: { outputFile?: string } = {}) {
-    super();
     this._outputFile = options.outputFile;
   }
 
@@ -129,22 +127,35 @@ class JSONReporter extends EmptyReporter {
     const fileSuites = new Map<string, JSONReportSuite>();
     const result: JSONReportSuite[] = [];
     for (const suite of suites) {
-      if (!fileSuites.has(suite.file)) {
+      if (!fileSuites.has(suite.location.file)) {
         const serialized = this._serializeSuite(suite);
         if (serialized) {
-          fileSuites.set(suite.file, serialized);
+          fileSuites.set(suite.location.file, serialized);
           result.push(serialized);
         }
       } else {
-        this._mergeTestsFromSuite(fileSuites.get(suite.file)!, suite);
+        this._mergeTestsFromSuite(fileSuites.get(suite.location.file)!, suite);
       }
     }
     return result;
   }
 
+  private _relativeLocation(location: Location): Location {
+    return {
+      file: toPosixPath(path.relative(this.config.rootDir, location.file)),
+      line: location.line,
+      column: location.column,
+    };
+  }
+
+  private _locationMatches(s: JSONReportSuite | JSONReportSpec, location: Location) {
+    const relative = this._relativeLocation(location);
+    return s.file === relative.file && s.line === relative.line && s.column === relative.column;
+  }
+
   private _mergeTestsFromSuite(to: JSONReportSuite, from: Suite) {
     for (const fromSuite of from.suites) {
-      const toSuite = (to.suites || []).find(s => s.title === fromSuite.title && s.file === toPosixPath(path.relative(this.config.rootDir, fromSuite.file)) && s.line === fromSuite.line && s.column === fromSuite.column);
+      const toSuite = (to.suites || []).find(s => s.title === fromSuite.title && this._locationMatches(s, from.location));
       if (toSuite) {
         this._mergeTestsFromSuite(toSuite, fromSuite);
       } else {
@@ -157,7 +168,7 @@ class JSONReporter extends EmptyReporter {
       }
     }
     for (const test of from.tests) {
-      const toSpec = to.specs.find(s => s.title === test.title && s.file === toPosixPath(path.relative(this.config.rootDir, test.file)) && s.line === test.line && s.column === test.column);
+      const toSpec = to.specs.find(s => s.title === test.title && s.file === toPosixPath(path.relative(this.config.rootDir, test.location.file)) && s.line === test.location.line && s.column === test.location.column);
       if (toSpec)
         toSpec.tests.push(this._serializeTest(test));
       else
@@ -166,14 +177,12 @@ class JSONReporter extends EmptyReporter {
   }
 
   private _serializeSuite(suite: Suite): null | JSONReportSuite {
-    if (!suite.findTest(test => true))
+    if (!suite.allTests().length)
       return null;
     const suites = suite.suites.map(suite => this._serializeSuite(suite)).filter(s => s) as JSONReportSuite[];
     return {
       title: suite.title,
-      file: toPosixPath(path.relative(this.config.rootDir, suite.file)),
-      line: suite.line,
-      column: suite.column,
+      ...this._relativeLocation(suite.location),
       specs: suite.tests.map(test => this._serializeTestSpec(test)),
       suites: suites.length ? suites : undefined,
     };
@@ -184,9 +193,7 @@ class JSONReporter extends EmptyReporter {
       title: test.title,
       ok: test.ok(),
       tests: [ this._serializeTest(test) ],
-      file: toPosixPath(path.relative(this.config.rootDir, test.file)),
-      line: test.line,
-      column: test.column,
+      ...this._relativeLocation(test.location),
     };
   }