From 66ea613c4d64aff3a01ea249a13498deee2fb04a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Sun, 18 Jul 2021 17:40:59 -0700 Subject: [PATCH] feat(test-runner): small changes to Reporter api (#7709) - `TestResult.startTime` - `Suite.location` is optional now - `Test.status()` renamed to `Test.outcome()` to differentiate against a `Test.expectedStatus` and `TestResult.status` of the different type. --- src/test/dispatcher.ts | 1 + src/test/ipc.ts | 3 ++- src/test/loader.ts | 2 +- src/test/reporters/base.ts | 2 +- src/test/reporters/dot.ts | 2 +- src/test/reporters/json.ts | 15 +++++++++------ src/test/reporters/junit.ts | 6 +++--- src/test/runner.ts | 4 +++- src/test/test.ts | 14 ++++++++------ src/test/testType.ts | 3 +-- src/test/workerRunner.ts | 8 +++++--- tests/playwright-test/reporter.spec.ts | 4 +++- types/testReporter.d.ts | 15 +++++++++++---- 13 files changed, 49 insertions(+), 30 deletions(-) diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 038f3195ab..8c6b6de6b6 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -265,6 +265,7 @@ export class Dispatcher { worker.on('testBegin', (params: TestBeginPayload) => { const { test, result: testRun } = this._testById.get(params.testId)!; testRun.workerIndex = params.workerIndex; + testRun.startTime = new Date(params.startWallTime); this._reportTestBegin(test); }); worker.on('testEnd', (params: TestEndPayload) => { diff --git a/src/test/ipc.ts b/src/test/ipc.ts index 14304ade2d..7f7c5768fc 100644 --- a/src/test/ipc.ts +++ b/src/test/ipc.ts @@ -31,7 +31,8 @@ export type WorkerInitParams = { export type TestBeginPayload = { testId: string; - workerIndex: number, + startWallTime: number; // milliseconds since unix epoch + workerIndex: number; }; export type TestEndPayload = { diff --git a/src/test/loader.ts b/src/test/loader.ts index 6f0ca9f6e6..c35ffb610e 100644 --- a/src/test/loader.ts +++ b/src/test/loader.ts @@ -112,7 +112,7 @@ export class Loader { try { const suite = new Suite(path.relative(this._fullConfig.rootDir, file) || path.basename(file)); suite._requireFile = file; - suite.location.file = file; + suite.location = { file, line: 0, column: 0 }; setCurrentlyLoadingFileSuite(suite); await this._requireOrImport(file); this._fileSuites.set(file, suite); diff --git a/src/test/reporters/base.ts b/src/test/reporters/base.ts index 27fc004d6e..6045c70b21 100644 --- a/src/test/reporters/base.ts +++ b/src/test/reporters/base.ts @@ -87,7 +87,7 @@ export class BaseReporter implements Reporter { const flaky: Test[] = []; this.suite.allTests().forEach(test => { - switch (test.status()) { + switch (test.outcome()) { case 'skipped': ++skipped; break; case 'expected': ++expected; break; case 'unexpected': unexpected.push(test); break; diff --git a/src/test/reporters/dot.ts b/src/test/reporters/dot.ts index d98b3baf85..dcc7725fa3 100644 --- a/src/test/reporters/dot.ts +++ b/src/test/reporters/dot.ts @@ -35,7 +35,7 @@ class DotReporter extends BaseReporter { process.stdout.write(colors.gray('×')); return; } - switch (test.status()) { + switch (test.outcome()) { case 'expected': process.stdout.write(colors.green('·')); break; case 'unexpected': process.stdout.write(colors.red(test.results[test.results.length - 1].status === 'timedOut' ? 'T' : 'F')); break; case 'flaky': process.stdout.write(colors.yellow('±')); break; diff --git a/src/test/reporters/json.ts b/src/test/reporters/json.ts index 1f67f9d8d7..542c9edf45 100644 --- a/src/test/reporters/json.ts +++ b/src/test/reporters/json.ts @@ -127,21 +127,24 @@ class JSONReporter implements Reporter { const result: JSONReportSuite[] = []; for (const projectSuite of suites) { for (const fileSuite of projectSuite.suites) { - if (!fileSuites.has(fileSuite.location.file)) { + const file = fileSuite.location!.file; + if (!fileSuites.has(file)) { const serialized = this._serializeSuite(fileSuite); if (serialized) { - fileSuites.set(fileSuite.location.file, serialized); + fileSuites.set(file, serialized); result.push(serialized); } } else { - this._mergeTestsFromSuite(fileSuites.get(fileSuite.location.file)!, fileSuite); + this._mergeTestsFromSuite(fileSuites.get(file)!, fileSuite); } } } return result; } - private _relativeLocation(location: Location): Location { + private _relativeLocation(location: Location | undefined): Location { + if (!location) + return { file: '', line: 0, column: 0 }; return { file: toPosixPath(path.relative(this.config.rootDir, location.file)), line: location.line, @@ -149,7 +152,7 @@ class JSONReporter implements Reporter { }; } - private _locationMatches(s: JSONReportSuite | JSONReportSpec, location: Location) { + private _locationMatches(s: JSONReportSuite | JSONReportSpec, location: Location | undefined) { const relative = this._relativeLocation(location); return s.file === relative.file && s.line === relative.line && s.column === relative.column; } @@ -205,7 +208,7 @@ class JSONReporter implements Reporter { expectedStatus: test.expectedStatus, projectName: test.titlePath()[1], results: test.results.map(r => this._serializeTestResult(r)), - status: test.status(), + status: test.outcome(), }; } diff --git a/src/test/reporters/junit.ts b/src/test/reporters/junit.ts index 0e2eea7e95..9cbb31c538 100644 --- a/src/test/reporters/junit.ts +++ b/src/test/reporters/junit.ts @@ -87,7 +87,7 @@ class JUnitReporter implements Reporter { suite.allTests().forEach(test => { ++tests; - if (test.status() === 'skipped') + if (test.outcome() === 'skipped') ++skipped; if (!test.ok()) ++failures; @@ -102,7 +102,7 @@ class JUnitReporter implements Reporter { const entry: XMLEntry = { name: 'testsuite', attributes: { - name: path.relative(this.config.rootDir, suite.location.file), + name: suite.location ? path.relative(this.config.rootDir, suite.location.file) : '', timestamp: this.timestamp, hostname: '', tests, @@ -130,7 +130,7 @@ class JUnitReporter implements Reporter { }; entries.push(entry); - if (test.status() === 'skipped') { + if (test.outcome() === 'skipped') { entry.children.push({ name: 'skipped'}); return; } diff --git a/src/test/runner.ts b/src/test/runner.ts index 83725964af..10991dac30 100644 --- a/src/test/runner.ts +++ b/src/test/runner.ts @@ -295,7 +295,7 @@ function filterByFocusedLine(suite: Suite, focusedTestFileLines: FilePatternFilt re.lastIndex = 0; return re.test(testFileName) && (line === testLine || line === null); }); - const suiteFilter = (suite: Suite) => testFileLineMatches(suite.location.file, suite.location.line); + const suiteFilter = (suite: Suite) => !!suite.location && testFileLineMatches(suite.location.file, suite.location.line); const testFilter = (test: Test) => testFileLineMatches(test.location.file, test.location.line); return filterSuite(suite, suiteFilter, testFilter); } @@ -406,6 +406,8 @@ function getClashingTestsPerSuite(rootSuite: Suite): Map { } function buildItemLocation(rootDir: string, testOrSuite: Suite | Test) { + if (!testOrSuite.location) + return ''; return `${path.relative(rootDir, testOrSuite.location.file)}:${testOrSuite.location.line}`; } diff --git a/src/test/test.ts b/src/test/test.ts index 11f79af7d4..c496a73495 100644 --- a/src/test/test.ts +++ b/src/test/test.ts @@ -21,7 +21,6 @@ import { Annotations, Location } from './types'; class Base { title: string; - location: Location = { file: '', line: 0, column: 0 }; parent?: Suite; _only = false; @@ -48,6 +47,7 @@ export type Modifier = { export class Suite extends Base implements reporterTypes.Suite { suites: Suite[] = []; tests: Test[] = []; + location?: Location; _fixtureOverrides: any = {}; _entries: (Suite | Test)[] = []; _hooks: { @@ -116,6 +116,7 @@ export class Suite extends Base implements reporterTypes.Suite { export class Test extends Base implements reporterTypes.Test { fn: Function; results: reporterTypes.TestResult[] = []; + location: Location; expectedStatus: reporterTypes.TestStatus = 'passed'; timeout = 0; @@ -131,14 +132,15 @@ export class Test extends Base implements reporterTypes.Test { _repeatEachIndex = 0; _projectIndex = 0; - constructor(title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl) { + constructor(title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl, location: Location) { super(title); this.fn = fn; this._ordinalInFile = ordinalInFile; this._testType = testType; + this.location = location; } - status(): 'skipped' | 'expected' | 'unexpected' | 'flaky' { + outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' { if (!this.results.length) return 'skipped'; if (this.results.length === 1 && this.expectedStatus === this.results[0].status) @@ -158,14 +160,13 @@ export class Test extends Base implements reporterTypes.Test { } ok(): boolean { - const status = this.status(); + const status = this.outcome(); return status === 'expected' || status === 'flaky' || status === 'skipped'; } _clone(): Test { - const test = new Test(this.title, this.fn, this._ordinalInFile, this._testType); + const test = new Test(this.title, this.fn, this._ordinalInFile, this._testType, this.location); test._only = this._only; - test.location = this.location; test._requireFile = this._requireFile; return test; } @@ -175,6 +176,7 @@ export class Test extends Base implements reporterTypes.Test { retry: this.results.length, workerIndex: 0, duration: 0, + startTime: new Date(), stdout: [], stderr: [], attachments: [], diff --git a/src/test/testType.ts b/src/test/testType.ts index c3af29a4c9..b45e3c8d14 100644 --- a/src/test/testType.ts +++ b/src/test/testType.ts @@ -62,9 +62,8 @@ export class TestTypeImpl { const ordinalInFile = countByFile.get(suite._requireFile) || 0; countByFile.set(suite._requireFile, ordinalInFile + 1); - const test = new Test(title, fn, ordinalInFile, this); + const test = new Test(title, fn, ordinalInFile, this, location); test._requireFile = suite._requireFile; - test.location = location; suite._addTest(test); if (type === 'only') diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 54f6f3515c..a48bffc011 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -199,6 +199,7 @@ export class WorkerRunner extends EventEmitter { return; const startTime = monotonicTime(); + const startWallTime = Date.now(); let deadlineRunner: DeadlineRunner | undefined; const testId = test._id; @@ -293,7 +294,7 @@ export class WorkerRunner extends EventEmitter { return testInfo.timeout ? startTime + testInfo.timeout : undefined; }; - this.emit('testBegin', buildTestBeginPayload(testId, testInfo)); + this.emit('testBegin', buildTestBeginPayload(testId, testInfo, startWallTime)); if (testInfo.expectedStatus === 'skipped') { testInfo.status = 'skipped'; @@ -461,10 +462,11 @@ export class WorkerRunner extends EventEmitter { } } -function buildTestBeginPayload(testId: string, testInfo: TestInfo): TestBeginPayload { +function buildTestBeginPayload(testId: string, testInfo: TestInfo, startWallTime: number): TestBeginPayload { return { testId, - workerIndex: testInfo.workerIndex + workerIndex: testInfo.workerIndex, + startWallTime, }; } diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index 3545c8b787..0bb36fb423 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -35,8 +35,10 @@ test('should work with custom reporter', async ({ runInlineTest }) => { onStdErr() { console.log('\\n%%reporter-stderr%%'); } - onTestEnd(test) { + onTestEnd(test, result) { console.log('\\n%%reporter-testend-' + test.title + '-' + test.titlePath()[1] + '%%'); + if (!result.startTime) + console.log('\\n%%error-no-start-time'); } onTimeout() { console.log('\\n%%reporter-timeout%%'); diff --git a/types/testReporter.d.ts b/types/testReporter.d.ts index 64f2e3b0b8..dd1eb08792 100644 --- a/types/testReporter.d.ts +++ b/types/testReporter.d.ts @@ -64,7 +64,7 @@ export interface Suite { /** * Location where the suite is defined. */ - location: Location; + location?: Location; /** * Child suites. @@ -142,9 +142,11 @@ export interface Test { results: TestResult[]; /** - * Overall test status. + * Testing outcome for this test. Note that outcome does not directly match to the status: + * - Test that is expected to fail and actually fails is 'expected'. + * - Test that passes on a second retry is 'flaky'. */ - status(): 'skipped' | 'expected' | 'unexpected' | 'flaky'; + outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky'; /** * Whether the test is considered running fine. @@ -165,7 +167,12 @@ export interface TestResult { /** * Index of the worker where the test was run. */ - workerIndex: number, + workerIndex: number; + + /** + * Test run start time. + */ + startTime: Date; /** * Running time in milliseconds.