diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 3d55c9451e..0165b0cd1c 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -192,10 +192,9 @@ export class Dispatcher { } } - // Only retry expected failures, not passes and only if the test failed. for (const testId of retryCandidates) { const pair = this._testById.get(testId)!; - if (!this._isStopped && pair.test.expectedStatus === 'passed' && pair.test.results.length < pair.test.retries + 1) { + if (!this._isStopped && pair.test.results.length < pair.test.retries + 1) { pair.result = pair.test._appendTestResult(); pair.steps = new Map(); pair.stepStack = new Set(); diff --git a/src/test/reporters/base.ts b/src/test/reporters/base.ts index e6f73107e6..74ec1fe125 100644 --- a/src/test/reporters/base.ts +++ b/src/test/reporters/base.ts @@ -147,8 +147,8 @@ export class BaseReporter implements Reporter { }); } - willRetry(test: TestCase, result: TestResult): boolean { - return test.outcome() === 'unexpected' && result.status !== 'passed' && test.results.length <= test.retries; + willRetry(test: TestCase): boolean { + return test.outcome() === 'unexpected' && test.results.length <= test.retries; } } @@ -159,10 +159,9 @@ export function formatFailure(config: FullConfig, test: TestCase, index?: number const resultTokens = formatResultFailure(test, result, ' '); if (!resultTokens.length) continue; - const statusSuffix = (result.status === 'passed' && test.expectedStatus === 'failed') ? ' -- passed unexpectedly' : ''; if (result.retry) { tokens.push(''); - tokens.push(colors.gray(pad(` Retry #${result.retry}${statusSuffix}`, '-'))); + tokens.push(colors.gray(pad(` Retry #${result.retry}`, '-'))); } tokens.push(...resultTokens); const output = ((result as any)[kOutputSymbol] || []) as TestResultOutput[]; @@ -187,6 +186,10 @@ export function formatResultFailure(test: TestCase, result: TestResult, initialI resultTokens.push(''); resultTokens.push(indent(colors.red(`Timeout of ${test.timeout}ms exceeded.`), initialIndent)); } + if (result.status === 'passed' && test.expectedStatus === 'failed') { + resultTokens.push(''); + resultTokens.push(indent(colors.red(`Expected to fail, but passed.`), initialIndent)); + } if (result.error !== undefined) resultTokens.push(indent(formatError(result.error, test.location.file), initialIndent)); return resultTokens; @@ -211,8 +214,7 @@ export function formatTestTitle(config: FullConfig, test: TestCase, step?: TestS function formatTestHeader(config: FullConfig, test: TestCase, indent: string, index?: number): string { const title = formatTestTitle(config, test); - const passedUnexpectedlySuffix = (test.results[0].status === 'passed' && test.expectedStatus === 'failed') ? ' -- passed unexpectedly' : ''; - const header = `${indent}${index ? index + ') ' : ''}${title}${passedUnexpectedlySuffix}`; + const header = `${indent}${index ? index + ') ' : ''}${title}`; return colors.red(pad(header, '=')); } diff --git a/src/test/reporters/dot.ts b/src/test/reporters/dot.ts index bed29db42b..1105423313 100644 --- a/src/test/reporters/dot.ts +++ b/src/test/reporters/dot.ts @@ -44,7 +44,7 @@ class DotReporter extends BaseReporter { process.stdout.write(colors.yellow('°')); return; } - if (this.willRetry(test, result)) { + if (this.willRetry(test)) { process.stdout.write(colors.gray('×')); return; } diff --git a/src/test/reporters/line.ts b/src/test/reporters/line.ts index 7962231722..547fd653a4 100644 --- a/src/test/reporters/line.ts +++ b/src/test/reporters/line.ts @@ -59,7 +59,7 @@ class LineReporter extends BaseReporter { const width = process.stdout.columns! - 1; const title = `[${++this._current}/${this._total}] ${formatTestTitle(this.config, test)}`.substring(0, width); process.stdout.write(`\u001B[1A\u001B[2K${title}\n`); - if (!this.willRetry(test, result) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { + if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { process.stdout.write(`\u001B[1A\u001B[2K`); console.log(formatFailure(this.config, test, ++this._failures)); console.log(); diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index eaef35bcdd..771b17f934 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -365,7 +365,7 @@ export class WorkerRunner extends EventEmitter { if (reportEvents) this.emit('testEnd', buildTestEndPayload(testId, testInfo)); - const isFailure = testInfo.status === 'timedOut' || (testInfo.status === 'failed' && testInfo.expectedStatus !== 'failed'); + const isFailure = testInfo.status !== 'skipped' && testInfo.status !== testInfo.expectedStatus; const preserveOutput = this._loader.fullConfig().preserveOutput === 'always' || (this._loader.fullConfig().preserveOutput === 'failures-only' && isFailure); if (!preserveOutput) @@ -374,7 +374,7 @@ export class WorkerRunner extends EventEmitter { this._currentTest = null; setCurrentTestInfo(null); - if (testInfo.status !== 'passed' && testInfo.status !== 'skipped') { + if (isFailure) { if (test._type === 'test') this._failedTestId = testId; else if (!this._fatalError) diff --git a/tests/playwright-test/exit-code.spec.ts b/tests/playwright-test/exit-code.spec.ts index 2155736f2d..56b1686972 100644 --- a/tests/playwright-test/exit-code.spec.ts +++ b/tests/playwright-test/exit-code.spec.ts @@ -116,7 +116,7 @@ test('should fail on unexpected pass', async ({ runInlineTest }) => { }); expect(exitCode).toBe(1); expect(failed).toBe(1); - expect(output).toContain('passed unexpectedly'); + expect(output).toContain('Expected to fail, but passed'); }); test('should respect global timeout', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/retry.spec.ts b/tests/playwright-test/retry.spec.ts index 25e916641b..7ff92ae9e8 100644 --- a/tests/playwright-test/retry.spec.ts +++ b/tests/playwright-test/retry.spec.ts @@ -87,10 +87,10 @@ test('should fail on unexpected pass with retries', async ({ runInlineTest }) => }, { retries: 1 }); expect(exitCode).toBe(1); expect(failed).toBe(1); - expect(output).toContain('passed unexpectedly'); + expect(output).toContain('Expected to fail, but passed.'); }); -test('should not retry unexpected pass', async ({ runInlineTest }) => { +test('should retry unexpected pass', async ({ runInlineTest }) => { const { exitCode, passed, failed, output } = await runInlineTest({ 'unexpected-pass.spec.js': ` const { test } = pwt; @@ -103,7 +103,7 @@ test('should not retry unexpected pass', async ({ runInlineTest }) => { expect(exitCode).toBe(1); expect(passed).toBe(0); expect(failed).toBe(1); - expect(stripAscii(output).split('\n')[0]).toBe('F'); + expect(stripAscii(output).split('\n')[0]).toBe('××F'); }); test('should not retry expected failure', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/test-serial.spec.ts b/tests/playwright-test/test-serial.spec.ts index fec4824925..9e4fd63ca3 100644 --- a/tests/playwright-test/test-serial.spec.ts +++ b/tests/playwright-test/test-serial.spec.ts @@ -129,3 +129,84 @@ test('test.describe.serial.only should work', async ({ runInlineTest }) => { '%%test3', ]); }); + +test('test.describe.serial should work with test.fail', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + const { test } = pwt; + test.describe.serial('suite', () => { + test('zero', () => { + console.log('\\n%%zero'); + }); + + test('one', ({}) => { + console.log('\\n%%one'); + test.fail(); + expect(1).toBe(2); + }); + + test('two', ({}, testInfo) => { + console.log('\\n%%two'); + test.fail(); + expect(testInfo.retry).toBe(0); + }); + + test('three', () => { + console.log('\\n%%three'); + }); + }); + `, + }, { retries: 0 }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(2); + expect(result.failed).toBe(1); + expect(result.skipped).toBe(1); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%zero', + '%%one', + '%%two', + ]); +}); + +test('test.describe.serial should work with test.fail and retries', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + const { test } = pwt; + test.describe.serial('suite', () => { + test('zero', () => { + console.log('\\n%%zero'); + }); + + test('one', ({}) => { + console.log('\\n%%one'); + test.fail(); + expect(1).toBe(2); + }); + + test('two', ({}, testInfo) => { + console.log('\\n%%two'); + test.fail(); + expect(testInfo.retry).toBe(0); + }); + + test('three', () => { + console.log('\\n%%three'); + }); + }); + `, + }, { retries: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(3); + expect(result.flaky).toBe(1); + expect(result.failed).toBe(0); + expect(result.skipped).toBe(0); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%zero', + '%%one', + '%%two', + '%%zero', + '%%one', + '%%two', + '%%three', + ]); +}); diff --git a/tests/playwright-test/worker-index.spec.ts b/tests/playwright-test/worker-index.spec.ts index 55df9a10b6..8f56bfca49 100644 --- a/tests/playwright-test/worker-index.spec.ts +++ b/tests/playwright-test/worker-index.spec.ts @@ -115,7 +115,7 @@ test('should reuse worker after test.skip()', async ({ runInlineTest }) => { expect(result.exitCode).toBe(0); }); -test('should use new worker after test.fail()', async ({ runInlineTest }) => { +test('should not use new worker after test.fail()', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; @@ -129,7 +129,7 @@ test('should use new worker after test.fail()', async ({ runInlineTest }) => { }); test('succeeds 2', async ({}, testInfo) => { - expect(testInfo.workerIndex).toBe(1); + expect(testInfo.workerIndex).toBe(0); }); `, });