diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index 7855659e69..2fc74e2800 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -371,19 +371,6 @@ export function monotonicTime(): number { return seconds * 1000 + (nanoseconds / 1000 | 0) / 1000; } -class HashStream extends stream.Writable { - private _hash = crypto.createHash('sha1'); - - override _write(chunk: Buffer, encoding: string, done: () => void) { - this._hash.update(chunk); - done(); - } - - digest(): string { - return this._hash.digest('hex'); - } -} - export function objectToArray(map?: { [key: string]: any }): NameValue[] | undefined { if (!map) return undefined; @@ -402,17 +389,6 @@ export function arrayToObject(array?: NameValue[]): { [key: string]: string } | return result; } -export async function calculateFileSha1(filename: string): Promise { - const hashStream = new HashStream(); - const stream = fs.createReadStream(filename); - stream.on('open', () => stream.pipe(hashStream)); - await new Promise((f, r) => { - hashStream.on('finish', f); - hashStream.on('error', r); - }); - return hashStream.digest(); -} - export function calculateSha1(buffer: Buffer | string): string { const hash = crypto.createHash('sha1'); hash.update(buffer); diff --git a/packages/playwright-test/src/fixtures.ts b/packages/playwright-test/src/fixtures.ts index d819065daa..bc3ada3fa1 100644 --- a/packages/playwright-test/src/fixtures.ts +++ b/packages/playwright-test/src/fixtures.ts @@ -96,15 +96,18 @@ class Fixture { private async _teardownInternal() { if (typeof this.registration.fn !== 'function') return; - for (const fixture of this.usages) - await fixture.teardown(); - this.usages.clear(); - if (this._useFuncFinished) { - debugTest(`teardown ${this.registration.name}`); - this._useFuncFinished.resolve(); - await this._selfTeardownComplete; + try { + for (const fixture of this.usages) + await fixture.teardown(); + this.usages.clear(); + if (this._useFuncFinished) { + debugTest(`teardown ${this.registration.name}`); + this._useFuncFinished.resolve(); + await this._selfTeardownComplete; + } + } finally { + this.runner.instanceForId.delete(this.registration.id); } - this.runner.instanceForId.delete(this.registration.id); } } diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts index 85c6566fc3..19b5f70f33 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -30,7 +30,7 @@ import { Annotations, TestCaseType, TestError, TestInfo, TestInfoImpl, TestStepI import { ProjectImpl } from './project'; import { FixtureRunner } from './fixtures'; import { DeadlineRunner, raceAgainstDeadline } from 'playwright-core/lib/utils/async'; -import { calculateFileSha1 } from 'playwright-core/lib/utils/utils'; +import { calculateSha1 } from 'playwright-core/lib/utils/utils'; const removeFolderAsync = util.promisify(rimraf); @@ -268,7 +268,7 @@ export class WorkerRunner extends EventEmitter { if ((options.path !== undefined ? 1 : 0) + (options.body !== undefined ? 1 : 0) !== 1) throw new Error(`Exactly one of "path" and "body" must be specified`); if (options.path) { - const hash = await calculateFileSha1(options.path); + const hash = calculateSha1(options.path); const dest = testInfo.outputPath('attachments', hash + path.extname(options.path)); await fs.promises.mkdir(path.dirname(dest), { recursive: true }); await fs.promises.copyFile(options.path, dest); diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index 5dd167b15a..22800c0999 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { test, expect } from './playwright-test-fixtures'; +import { test, expect, countTimes, stripAscii } from './playwright-test-fixtures'; test('should handle fixture timeout', async ({ runInlineTest }) => { const result = await runInlineTest({ @@ -435,3 +435,23 @@ test('should not teardown when setup times out', async ({ runInlineTest }) => { expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ ]); }); + +test('should not report fixture teardown error twice', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const test = pwt.test.extend({ + fixture: async ({ }, use) => { + await use(); + throw new Error('Oh my error'); + }, + }); + test('good', async ({ fixture }) => { + }); + `, + }, { reporter: 'list' }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output).toContain('Error: Oh my error'); + expect(stripAscii(result.output)).toContain(`throw new Error('Oh my error')`); + expect(countTimes(result.output, 'Oh my error')).toBe(2); +}); diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index 850218b735..0923092042 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -247,7 +247,7 @@ export function stripAscii(str: string): string { return str.replace(asciiRegex, ''); } -function countTimes(s: string, sub: string): number { +export function countTimes(s: string, sub: string): number { let result = 0; for (let index = 0; index !== -1;) { index = s.indexOf(sub, index); diff --git a/tests/playwright-test/reporter-attachment.spec.ts b/tests/playwright-test/reporter-attachment.spec.ts index f975eb716f..94d6dc0ae5 100644 --- a/tests/playwright-test/reporter-attachment.spec.ts +++ b/tests/playwright-test/reporter-attachment.spec.ts @@ -80,7 +80,6 @@ test('render trace attachment', async ({ runInlineTest }) => { expect(result.exitCode).toBe(1); }); - test(`testInfo.attach errors`, async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` @@ -97,9 +96,50 @@ test(`testInfo.attach errors`, async ({ runInlineTest }) => { `, }, { reporter: 'line', workers: 1 }); const text = stripAscii(result.output).replace(/\\/g, '/'); - expect(text).toMatch(/Error: ENOENT: no such file or directory, open '.*foo.txt.*'/); + expect(text).toMatch(/Error: ENOENT: no such file or directory, copyfile '.*foo.txt.*'/); expect(text).toContain(`Exactly one of "path" and "body" must be specified`); expect(result.passed).toBe(0); expect(result.failed).toBe(3); expect(result.exitCode).toBe(1); }); + +test(`testInfo.attach error in fixture`, async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.js': ` + const test = pwt.test.extend({ + fixture: async ({}, use, testInfo) => { + await use(); + await testInfo.attach('name', { path: 'foo.txt' }); + }, + }); + test('fail1', async ({ fixture }) => { + }); + `, + }, { reporter: 'line', workers: 1 }); + const text = stripAscii(result.output).replace(/\\/g, '/'); + expect(text).toMatch(/Error: ENOENT: no such file or directory, copyfile '.*foo.txt.*'/); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(0); + expect(result.failed).toBe(1); +}); + +test(`testInfo.attach success in fixture`, async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.js': ` + const test = pwt.test.extend({ + fixture: async ({}, use, testInfo) => { + const filePath = testInfo.outputPath('foo.txt'); + require('fs').writeFileSync(filePath, 'hello'); + await use(); + await testInfo.attach('name', { path: filePath }); + }, + }); + test('success', async ({ fixture }) => { + expect(true).toBe(false); + }); + `, + }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(stripAscii(result.output)).toContain('attachment #1: name (text/plain)'); +}); diff --git a/tests/playwright-test/reporter-raw.spec.ts b/tests/playwright-test/reporter-raw.spec.ts index 866ba31ee3..427e59f497 100644 --- a/tests/playwright-test/reporter-raw.spec.ts +++ b/tests/playwright-test/reporter-raw.spec.ts @@ -155,7 +155,7 @@ test(`testInfo.attach should save attachments via path`, async ({ runInlineTest expect(result.attachments[0].name).toBe('foo'); expect(result.attachments[0].contentType).toBe('application/json'); const p = result.attachments[0].path; - expect(p).toMatch(/[/\\]attachments[/\\]01a5667d100fac2200bf40cf43083fae0580c58e\.json$/); + expect(p).toMatch(/[/\\]attachments[/\\][0-9a-f]+\.json$/); const contents = fs.readFileSync(p); expect(contents.toString()).toBe('We <3 Playwright!'); } @@ -164,7 +164,7 @@ test(`testInfo.attach should save attachments via path`, async ({ runInlineTest expect(result.attachments[0].name).toBe('foo'); expect(result.attachments[0].contentType).toBe('image/png'); const p = result.attachments[0].path; - expect(p).toMatch(/[/\\]attachments[/\\]01a5667d100fac2200bf40cf43083fae0580c58e\.json$/); + expect(p).toMatch(/[/\\]attachments[/\\][0-9a-f]+\.json$/); const contents = fs.readFileSync(p); expect(contents.toString()).toBe('We <3 Playwright!'); } @@ -173,7 +173,7 @@ test(`testInfo.attach should save attachments via path`, async ({ runInlineTest expect(result.attachments[0].name).toBe('example.png'); expect(result.attachments[0].contentType).toBe('x-playwright/custom'); const p = result.attachments[0].path; - expect(p).toMatch(/[/\\]attachments[/\\]01a5667d100fac2200bf40cf43083fae0580c58e\.json$/); + expect(p).toMatch(/[/\\]attachments[/\\][0-9a-f]+\.json$/); const contents = fs.readFileSync(p); expect(contents.toString()).toBe('We <3 Playwright!'); } @@ -182,7 +182,7 @@ test(`testInfo.attach should save attachments via path`, async ({ runInlineTest expect(result.attachments[0].name).toBe('foo'); expect(result.attachments[0].contentType).toBe('application/octet-stream'); const p = result.attachments[0].path; - expect(p).toMatch(/[/\\]attachments[/\\]01a5667d100fac2200bf40cf43083fae0580c58e\.this-extension-better-not-map-to-an-actual-mimetype$/); + expect(p).toMatch(/[/\\]attachments[/\\][0-9a-f]+\.this-extension-better-not-map-to-an-actual-mimetype$/); const contents = fs.readFileSync(p); expect(contents.toString()).toBe('We <3 Playwright!'); }