From 34c6197f9ee7b708e1d845918d84b751b216bbde Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Tue, 29 Aug 2023 10:56:21 -0700
Subject: [PATCH] chore: include start/endTime and duration in onEnd report
 callback (#26760)

Fixes https://github.com/microsoft/playwright/issues/23637
---
 docs/src/test-reporter-api/class-reporter.md  |  2 ++
 packages/html-reporter/src/chip.css           |  3 +-
 packages/html-reporter/src/testFilesView.tsx  |  3 +-
 packages/html-reporter/src/types.ts           |  5 ++--
 .../src/isomorphic/teleReceiver.ts            | 14 +++++++--
 .../playwright-test/src/reporters/base.ts     |  8 ++---
 .../playwright-test/src/reporters/html.ts     | 16 +++++-----
 .../src/reporters/internalReporter.ts         | 15 ++++++++--
 .../playwright-test/src/reporters/merge.ts    | 29 ++++++++++++-------
 .../src/reporters/teleEmitter.ts              | 14 +++++++--
 packages/playwright-test/src/runner/tasks.ts  |  9 ++----
 .../playwright-test/types/testReporter.d.ts   | 10 +++++++
 tests/playwright-test/reporter-blob.spec.ts   |  1 -
 .../overrides-testReporter.d.ts               | 10 +++++++
 14 files changed, 94 insertions(+), 45 deletions(-)

diff --git a/docs/src/test-reporter-api/class-reporter.md b/docs/src/test-reporter-api/class-reporter.md
index 7e3d5cfd43..d2c8b111b3 100644
--- a/docs/src/test-reporter-api/class-reporter.md
+++ b/docs/src/test-reporter-api/class-reporter.md
@@ -113,6 +113,8 @@ Called after all tests have been run, or testing has been interrupted. Note that
 * since: v1.10
 - `result` <[Object]>
   - `status` <[FullStatus]<"passed"|"failed"|"timedout"|"interrupted">>
+  - `startTime` <[Date]>
+  - `duration` <[int]>
 
 Result of the full test run.
 * `'passed'` - Everything went as expected.
diff --git a/packages/html-reporter/src/chip.css b/packages/html-reporter/src/chip.css
index f8d7938f90..1b40275166 100644
--- a/packages/html-reporter/src/chip.css
+++ b/packages/html-reporter/src/chip.css
@@ -21,7 +21,7 @@
   background-color: var(--color-canvas-subtle);
   padding: 0 8px;
   border-bottom: none;
-  margin-top: 24px;
+  margin-top: 12px;
   font-weight: 600;
   line-height: 38px;
   white-space: nowrap;
@@ -44,6 +44,7 @@
   border-bottom-left-radius: 6px;
   border-bottom-right-radius: 6px;
   padding: 16px;
+  margin-bottom: 12px;
 }
 
 .chip-body-no-insets {
diff --git a/packages/html-reporter/src/testFilesView.tsx b/packages/html-reporter/src/testFilesView.tsx
index b6a57fdd5e..c0154af4d6 100644
--- a/packages/html-reporter/src/testFilesView.tsx
+++ b/packages/html-reporter/src/testFilesView.tsx
@@ -41,10 +41,11 @@ export const TestFilesView: React.FC<{
     return result;
   }, [report, filter]);
   return <>
-    <div className='p-2' style={{ display: 'flex' }}>
+    <div className='mt-2 mx-1' style={{ display: 'flex' }}>
       {projectNames.length === 1 && !!projectNames[0] && <div data-testid="project-name" style={{ color: 'var(--color-fg-subtle)' }}>Project: {projectNames[0]}</div>}
       {!filter.empty() && <div data-testid="filtered-tests-count" style={{ color: 'var(--color-fg-subtle)', padding: '0 10px' }}>Filtered: {filteredStats.total}</div>}
       <div style={{ flex: 'auto' }}></div>
+      <div data-testid="overall-time" style={{ color: 'var(--color-fg-subtle)', marginRight: '10px' }}>{report ? new Date(report.startTime).toLocaleString() : ''}</div>
       <div data-testid="overall-duration" style={{ color: 'var(--color-fg-subtle)' }}>Total time: {msToString(filteredStats.duration)}</div>
     </div>
     {report && filteredFiles.map(({ file, defaultExpanded }) => {
diff --git a/packages/html-reporter/src/types.ts b/packages/html-reporter/src/types.ts
index 65db7d7716..021218abc5 100644
--- a/packages/html-reporter/src/types.ts
+++ b/packages/html-reporter/src/types.ts
@@ -23,11 +23,10 @@ export type Stats = {
   flaky: number;
   skipped: number;
   ok: boolean;
-  duration: number;
 };
 
 export type FilteredStats = {
-  total: number
+  total: number,
   duration: number,
 };
 
@@ -42,6 +41,8 @@ export type HTMLReport = {
   files: TestFileSummary[];
   stats: Stats;
   projectNames: string[];
+  startTime: number;
+  duration: number;
 };
 
 export type TestFile = {
diff --git a/packages/playwright-test/src/isomorphic/teleReceiver.ts b/packages/playwright-test/src/isomorphic/teleReceiver.ts
index 2bda3d1b01..a7e818e6ea 100644
--- a/packages/playwright-test/src/isomorphic/teleReceiver.ts
+++ b/packages/playwright-test/src/isomorphic/teleReceiver.ts
@@ -115,6 +115,12 @@ export type JsonTestStepEnd = {
   error?: TestError;
 };
 
+export type JsonFullResult = {
+  status: FullResult['status'];
+  startTime: number;
+  duration: number;
+};
+
 export type JsonEvent = {
   method: string;
   params: any
@@ -300,8 +306,12 @@ export class TeleReporterReceiver {
     }
   }
 
-  private _onEnd(result: FullResult): Promise<void> | void {
-    return this._reporter.onEnd?.(result);
+  private _onEnd(result: JsonFullResult): Promise<void> | void {
+    return this._reporter.onEnd?.({
+      status: result.status,
+      startTime: new Date(result.startTime),
+      duration: result.duration,
+    });
   }
 
   private _onExit(): Promise<void> | void {
diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts
index a8c8688962..205e58da09 100644
--- a/packages/playwright-test/src/reporters/base.ts
+++ b/packages/playwright-test/src/reporters/base.ts
@@ -18,7 +18,7 @@ import { colors, ms as milliseconds, parseStackTraceLine } from 'playwright-core
 import path from 'path';
 import type { FullConfig, TestCase, Suite, TestResult, TestError, FullResult, TestStep, Location } from '../../types/testReporter';
 import type { SuitePrivate } from '../../types/reporterPrivate';
-import { getPackageManagerExecCommand, monotonicTime } from 'playwright-core/lib/utils';
+import { getPackageManagerExecCommand } from 'playwright-core/lib/utils';
 import type { ReporterV2 } from './reporterV2';
 export type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' };
 export const kOutputSymbol = Symbol('output');
@@ -45,13 +45,11 @@ type TestSummary = {
 };
 
 export class BaseReporter implements ReporterV2 {
-  duration = 0;
   config!: FullConfig;
   suite!: Suite;
   totalTestCount = 0;
   result!: FullResult;
   private fileDurations = new Map<string, number>();
-  private monotonicStartTime: number = 0;
   private _omitFailures: boolean;
   private readonly _ttyWidthForTest: number;
   private _fatalErrors: TestError[] = [];
@@ -71,7 +69,6 @@ export class BaseReporter implements ReporterV2 {
   }
 
   onBegin(suite: Suite) {
-    this.monotonicStartTime = monotonicTime();
     this.suite = suite;
     this.totalTestCount = suite.allTests().length;
   }
@@ -114,7 +111,6 @@ export class BaseReporter implements ReporterV2 {
   }
 
   async onEnd(result: FullResult) {
-    this.duration = monotonicTime() - this.monotonicStartTime;
     this.result = result;
   }
 
@@ -182,7 +178,7 @@ export class BaseReporter implements ReporterV2 {
     if (skipped)
       tokens.push(colors.yellow(`  ${skipped} skipped`));
     if (expected)
-      tokens.push(colors.green(`  ${expected} passed`) + colors.dim(` (${milliseconds(this.duration)})`));
+      tokens.push(colors.green(`  ${expected} passed`) + colors.dim(` (${milliseconds(this.result.duration)})`));
     if (this.result.status === 'timedout')
       tokens.push(colors.red(`  Timed out waiting ${this.config.globalTimeout / 1000}s for the entire test run`));
     if (fatalErrors.length && expected + unexpected.length + interrupted.length + flaky.length > 0)
diff --git a/packages/playwright-test/src/reporters/html.ts b/packages/playwright-test/src/reporters/html.ts
index c3130c6817..396518da83 100644
--- a/packages/playwright-test/src/reporters/html.ts
+++ b/packages/playwright-test/src/reporters/html.ts
@@ -22,7 +22,7 @@ import type { TransformCallback } from 'stream';
 import { Transform } from 'stream';
 import { toPosixPath } from './json';
 import { codeFrameColumns } from '../transform/babelBundle';
-import type { FullConfig, Location, Suite, TestCase as TestCasePublic, TestResult as TestResultPublic, TestStep as TestStepPublic } from '../../types/testReporter';
+import type { FullResult, FullConfig, Location, Suite, TestCase as TestCasePublic, TestResult as TestResultPublic, TestStep as TestStepPublic } from '../../types/testReporter';
 import type { SuitePrivate } from '../../types/reporterPrivate';
 import { HttpServer, assert, calculateSha1, copyFileAndMakeWritable, gracefullyProcessExitDoNotHang, removeFolders, sanitizeForFilePath } from 'playwright-core/lib/utils';
 import { formatResultFailure, stripAnsiEscapes } from './base';
@@ -40,7 +40,6 @@ type TestEntry = {
   testCaseSummary: TestCaseSummary
 };
 
-
 const htmlReportOptions = ['always', 'never', 'on-failure'];
 type HtmlReportOpenOption = (typeof htmlReportOptions)[number];
 
@@ -112,11 +111,11 @@ class HtmlReporter extends EmptyReporter {
     };
   }
 
-  override async onEnd() {
+  override async onEnd(result: FullResult) {
     const projectSuites = this.suite.suites;
     await removeFolders([this._outputFolder]);
     const builder = new HtmlBuilder(this.config, this._outputFolder, this._attachmentsBaseURL);
-    this._buildResult = await builder.build(this.config.metadata, projectSuites);
+    this._buildResult = await builder.build(this.config.metadata, projectSuites, result);
   }
 
   override async onExit() {
@@ -218,7 +217,7 @@ class HtmlBuilder {
     this._attachmentsBaseURL = attachmentsBaseURL;
   }
 
-  async build(metadata: Metadata, projectSuites: Suite[]): Promise<{ ok: boolean, singleTestId: string | undefined }> {
+  async build(metadata: Metadata, projectSuites: Suite[], result: FullResult): Promise<{ ok: boolean, singleTestId: string | undefined }> {
 
     const data = new Map<string, { testFile: TestFile, testFileSummary: TestFileSummary }>();
     for (const projectSuite of projectSuites) {
@@ -257,7 +256,6 @@ class HtmlBuilder {
         if (test.outcome === 'flaky')
           ++stats.flaky;
         ++stats.total;
-        stats.duration += test.duration;
       }
       stats.ok = stats.unexpected + stats.flaky === 0;
       if (!stats.ok)
@@ -274,9 +272,11 @@ class HtmlBuilder {
     }
     const htmlReport: HTMLReport = {
       metadata,
+      startTime: result.startTime.getTime(),
+      duration: result.duration,
       files: [...data.values()].map(e => e.testFileSummary),
       projectNames: projectSuites.map(r => r.project()!.name),
-      stats: { ...[...data.values()].reduce((a, e) => addStats(a, e.testFileSummary.stats), emptyStats()), duration: metadata.totalTime }
+      stats: { ...[...data.values()].reduce((a, e) => addStats(a, e.testFileSummary.stats), emptyStats()) }
     };
     htmlReport.files.sort((f1, f2) => {
       const w1 = f1.stats.unexpected * 1000 + f1.stats.flaky;
@@ -501,7 +501,6 @@ const emptyStats = (): Stats => {
     flaky: 0,
     skipped: 0,
     ok: true,
-    duration: 0,
   };
 };
 
@@ -512,7 +511,6 @@ const addStats = (stats: Stats, delta: Stats): Stats => {
   stats.unexpected += delta.unexpected;
   stats.flaky += delta.flaky;
   stats.ok = stats.ok && delta.ok;
-  stats.duration += delta.duration;
   return stats;
 };
 
diff --git a/packages/playwright-test/src/reporters/internalReporter.ts b/packages/playwright-test/src/reporters/internalReporter.ts
index a796803a47..4075e92c48 100644
--- a/packages/playwright-test/src/reporters/internalReporter.ts
+++ b/packages/playwright-test/src/reporters/internalReporter.ts
@@ -21,11 +21,14 @@ import type { FullConfig, TestCase, TestError, TestResult, FullResult, TestStep
 import { Suite } from '../common/test';
 import { prepareErrorStack, relativeFilePath } from './base';
 import type { ReporterV2 } from './reporterV2';
+import { monotonicTime } from 'playwright-core/lib/utils';
 
-export class InternalReporter implements ReporterV2 {
+export class InternalReporter {
   private _reporter: ReporterV2;
   private _didBegin = false;
   private _config!: FullConfig;
+  private _startTime: Date | undefined;
+  private _monotonicStartTime: number | undefined;
 
   constructor(reporter: ReporterV2) {
     this._reporter = reporter;
@@ -37,6 +40,8 @@ export class InternalReporter implements ReporterV2 {
 
   onConfigure(config: FullConfig) {
     this._config = config;
+    this._startTime = new Date();
+    this._monotonicStartTime = monotonicTime();
     this._reporter.onConfigure(config);
   }
 
@@ -62,12 +67,16 @@ export class InternalReporter implements ReporterV2 {
     this._reporter.onTestEnd(test, result);
   }
 
-  async onEnd(result: FullResult) {
+  async onEnd(result: { status: FullResult['status'] }) {
     if (!this._didBegin) {
       // onBegin was not reported, emit it.
       this.onBegin(new Suite('', 'root'));
     }
-    await this._reporter.onEnd(result);
+    await this._reporter.onEnd({
+      ...result,
+      startTime: this._startTime!,
+      duration: monotonicTime() - this._monotonicStartTime!,
+    });
   }
 
   async onExit() {
diff --git a/packages/playwright-test/src/reporters/merge.ts b/packages/playwright-test/src/reporters/merge.ts
index 448c050033..e114455eab 100644
--- a/packages/playwright-test/src/reporters/merge.ts
+++ b/packages/playwright-test/src/reporters/merge.ts
@@ -17,9 +17,8 @@
 import fs from 'fs';
 import path from 'path';
 import type { ReporterDescription } from '../../types/test';
-import type { FullResult } from '../../types/testReporter';
 import type { FullConfigInternal } from '../common/config';
-import type { JsonConfig, JsonEvent, JsonProject, JsonSuite, JsonTestResultEnd } from '../isomorphic/teleReceiver';
+import type { JsonConfig, JsonEvent, JsonFullResult, JsonProject, JsonSuite, JsonTestResultEnd } from '../isomorphic/teleReceiver';
 import { TeleReporterReceiver } from '../isomorphic/teleReceiver';
 import { JsonStringInternalizer, StringInternPool } from '../isomorphic/stringInternPool';
 import { createReporters } from '../runner/reporters';
@@ -228,7 +227,6 @@ function mergeConfigureEvents(configureEvents: JsonEvent[]): JsonEvent {
     globalTimeout: 0,
     maxFailures: 0,
     metadata: {
-      totalTime: 0,
     },
     rootDir: '',
     version: '',
@@ -252,7 +250,6 @@ function mergeConfigs(to: JsonConfig, from: JsonConfig): JsonConfig {
     metadata: {
       ...to.metadata,
       ...from.metadata,
-      totalTime: to.metadata.totalTime + from.metadata.totalTime,
       actualWorkers: (to.metadata.actualWorkers || 0) + (from.metadata.actualWorkers || 0),
     },
     workers: to.workers + from.workers,
@@ -260,16 +257,26 @@ function mergeConfigs(to: JsonConfig, from: JsonConfig): JsonConfig {
 }
 
 function mergeEndEvents(endEvents: JsonEvent[]): JsonEvent {
-  const result: FullResult = { status: 'passed' };
+  let startTime = endEvents.length ? 10000000000000 : Date.now();
+  let status: JsonFullResult['status'] = 'passed';
+  let duration: number = 0;
+
   for (const event of endEvents) {
-    const shardResult: FullResult = event.params.result;
+    const shardResult: JsonFullResult = event.params.result;
     if (shardResult.status === 'failed')
-      result.status = 'failed';
-    else if (shardResult.status === 'timedout' && result.status !== 'failed')
-      result.status = 'timedout';
-    else if (shardResult.status === 'interrupted' && result.status !== 'failed' && result.status !== 'timedout')
-      result.status = 'interrupted';
+      status = 'failed';
+    else if (shardResult.status === 'timedout' && status !== 'failed')
+      status = 'timedout';
+    else if (shardResult.status === 'interrupted' && status !== 'failed' && status !== 'timedout')
+      status = 'interrupted';
+    startTime = Math.min(startTime, shardResult.startTime);
+    duration = Math.max(duration, shardResult.duration);
   }
+  const result: JsonFullResult = {
+    status,
+    startTime,
+    duration,
+  };
   return {
     method: 'onEnd',
     params: {
diff --git a/packages/playwright-test/src/reporters/teleEmitter.ts b/packages/playwright-test/src/reporters/teleEmitter.ts
index 04e41102c6..113856ce62 100644
--- a/packages/playwright-test/src/reporters/teleEmitter.ts
+++ b/packages/playwright-test/src/reporters/teleEmitter.ts
@@ -20,7 +20,7 @@ import type { SuitePrivate } from '../../types/reporterPrivate';
 import type { FullConfig, FullResult, Location, TestCase, TestError, TestResult, TestStep } from '../../types/testReporter';
 import { FullConfigInternal, getProjectId } from '../common/config';
 import type { Suite } from '../common/test';
-import type { JsonAttachment, JsonConfig, JsonEvent, JsonProject, JsonStdIOType, JsonSuite, JsonTestCase, JsonTestEnd, JsonTestResultEnd, JsonTestResultStart, JsonTestStepEnd, JsonTestStepStart } from '../isomorphic/teleReceiver';
+import type { JsonAttachment, JsonConfig, JsonEvent, JsonFullResult, JsonProject, JsonStdIOType, JsonSuite, JsonTestCase, JsonTestEnd, JsonTestResultEnd, JsonTestResultStart, JsonTestStepEnd, JsonTestStepStart } from '../isomorphic/teleReceiver';
 import { serializeRegexPatterns } from '../isomorphic/teleReceiver';
 import type { ReporterV2 } from './reporterV2';
 
@@ -125,7 +125,17 @@ export class TeleReporterEmitter implements ReporterV2 {
   }
 
   async onEnd(result: FullResult) {
-    this._messageSink({ method: 'onEnd', params: { result } });
+    const resultPayload: JsonFullResult = {
+      status: result.status,
+      startTime: result.startTime.getTime(),
+      duration: result.duration,
+    };
+    this._messageSink({
+      method: 'onEnd',
+      params: {
+        result: resultPayload
+      }
+    });
   }
 
   async onExit() {
diff --git a/packages/playwright-test/src/runner/tasks.ts b/packages/playwright-test/src/runner/tasks.ts
index 8a5399680e..96982eebc0 100644
--- a/packages/playwright-test/src/runner/tasks.ts
+++ b/packages/playwright-test/src/runner/tasks.ts
@@ -29,7 +29,6 @@ import { collectProjectsAndTestFiles, createRootSuite, loadFileSuites, loadGloba
 import type { Matcher } from '../util';
 import type { Suite } from '../common/test';
 import { buildDependentProjects, buildTeardownToSetupsMap } from './projectUtils';
-import { monotonicTime } from 'playwright-core/lib/utils';
 
 const readDirAsync = promisify(fs.readdir);
 
@@ -105,15 +104,11 @@ export function createTaskRunnerForList(config: FullConfigInternal, reporter: Re
 }
 
 function createReportBeginTask(): Task<TestRun> {
-  let montonicStartTime = 0;
   return {
-    setup: async ({ config, reporter, rootSuite }) => {
-      montonicStartTime = monotonicTime();
+    setup: async ({ reporter, rootSuite }) => {
       reporter.onBegin(rootSuite!);
     },
-    teardown: async ({ config }) => {
-      config.config.metadata.totalTime = monotonicTime() - montonicStartTime;
-    },
+    teardown: async ({}) => {},
   };
 }
 
diff --git a/packages/playwright-test/types/testReporter.d.ts b/packages/playwright-test/types/testReporter.d.ts
index 45f2d6b312..9f18d3114e 100644
--- a/packages/playwright-test/types/testReporter.d.ts
+++ b/packages/playwright-test/types/testReporter.d.ts
@@ -305,6 +305,16 @@ export interface FullResult {
    *   - 'interrupted' - interrupted by the user.
    */
   status: 'passed' | 'failed' | 'timedout' | 'interrupted';
+
+  /**
+   * Test start wall time.
+   */
+  startTime: Date;
+
+  /**
+   * Test duration in milliseconds.
+   */
+  duration: number;
 }
 
 /**
diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts
index af138a6902..2924625e54 100644
--- a/tests/playwright-test/reporter-blob.spec.ts
+++ b/tests/playwright-test/reporter-blob.spec.ts
@@ -996,7 +996,6 @@ test('preserve config fields', async ({ runInlineTest, mergeReports }) => {
   expect(json.globalTimeout).toBe(config.globalTimeout);
   expect(json.maxFailures).toBe(config.maxFailures);
   expect(json.metadata).toEqual(expect.objectContaining(config.metadata));
-  expect(json.metadata.totalTime).toBeTruthy();
   expect(json.workers).toBe(2);
   expect(json.version).toBeTruthy();
   expect(json.version).not.toEqual(test.info().config.version);
diff --git a/utils/generate_types/overrides-testReporter.d.ts b/utils/generate_types/overrides-testReporter.d.ts
index 484c63a7f2..ead17e201c 100644
--- a/utils/generate_types/overrides-testReporter.d.ts
+++ b/utils/generate_types/overrides-testReporter.d.ts
@@ -41,6 +41,16 @@ export interface FullResult {
    *   - 'interrupted' - interrupted by the user.
    */
   status: 'passed' | 'failed' | 'timedout' | 'interrupted';
+
+  /**
+   * Test start wall time.
+   */
+  startTime: Date;
+
+  /**
+   * Test duration in milliseconds.
+   */
+  duration: number;
 }
 
 export interface Reporter {