From a56d80135262117837708ce6d068251a6a7959be Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 15 Jul 2022 13:05:48 -0700 Subject: [PATCH] fix(test runner): show fixture name when worker times out (#15724) --- packages/playwright-test/src/fixtures.ts | 22 +++++++++++++++----- tests/playwright-test/fixture-errors.spec.ts | 2 +- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/playwright-test/src/fixtures.ts b/packages/playwright-test/src/fixtures.ts index ad34752633..9d19ed547e 100644 --- a/packages/playwright-test/src/fixtures.ts +++ b/packages/playwright-test/src/fixtures.ts @@ -128,11 +128,25 @@ class Fixture { } async teardown(timeoutManager: TimeoutManager) { - if (!this._teardownWithDepsComplete) - this._teardownWithDepsComplete = this._teardownInternal(timeoutManager); + if (this._teardownWithDepsComplete) { + // When we are waiting for the teardown for the second time, + // most likely after the first time did timeout, annotate current fixture + // for better error messages. + this._setTeardownDescription(timeoutManager); + await this._teardownWithDepsComplete; + timeoutManager.setCurrentFixture(undefined); + return; + } + this._teardownWithDepsComplete = this._teardownInternal(timeoutManager); await this._teardownWithDepsComplete; } + private _setTeardownDescription(timeoutManager: TimeoutManager) { + const title = this.registration.customTitle || this.registration.name; + this._runnableDescription.title = this.registration.timeout !== undefined ? `Fixture "${title}"` : `tearing down "${title}"`; + timeoutManager.setCurrentFixture(this._runnableDescription); + } + private async _teardownInternal(timeoutManager: TimeoutManager) { if (typeof this.registration.fn !== 'function') return; @@ -146,9 +160,7 @@ class Fixture { } if (this._useFuncFinished) { debugTest(`teardown ${this.registration.name}`); - const title = this.registration.customTitle || this.registration.name; - this._runnableDescription.title = this.registration.timeout !== undefined ? `Fixture "${title}"` : `tearing down "${title}"`; - timeoutManager.setCurrentFixture(this._runnableDescription); + this._setTeardownDescription(timeoutManager); this._useFuncFinished.resolve(); await this._selfTeardownComplete; timeoutManager.setCurrentFixture(undefined); diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index 11e23fb92a..f6dee67709 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -474,7 +474,7 @@ test('should not report fixture teardown timeout twice', async ({ runInlineTest expect(result.output).toContain('Test timeout of 1000ms exceeded while tearing down "fixture".'); expect(stripAnsi(result.output)).not.toContain('pwt.test.extend'); // Should not point to the location. // TODO: this should be "not.toContain" actually. - expect(result.output).toContain('Worker teardown timeout of 1000ms exceeded.'); + expect(result.output).toContain('Worker teardown timeout of 1000ms exceeded while tearing down "fixture".'); }); test('should handle fixture teardown error after test timeout and continue', async ({ runInlineTest }) => {