fix(test runner): expect.poll error reporting should handle non-expect errors (#32257)

Closes https://github.com/microsoft/playwright/issues/32256

We were expecting all errors to be of type `ExpectError`, but apparently
`expect` propagates rejections in the polling functions right through.
So we also need to handle that case.

I wonder if we have more cases of this. Would it make sense to enable
`useUnknownInCatchVariables` in TypeScript?
This commit is contained in:
Simon Knott 2024-08-26 09:39:25 +02:00 committed by GitHub
parent 9c81eab329
commit 1511d8643e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 38 additions and 8 deletions

View file

@ -60,7 +60,7 @@ import {
} from '../common/expectBundle'; } from '../common/expectBundle';
import { zones } from 'playwright-core/lib/utils'; import { zones } from 'playwright-core/lib/utils';
import { TestInfoImpl } from '../worker/testInfo'; import { TestInfoImpl } from '../worker/testInfo';
import { ExpectError } from './matcherHint'; import { ExpectError, isExpectError } from './matcherHint';
// #region // #region
// Mirrored from https://github.com/facebook/jest/blob/f13abff8df9a0e1148baf3584bcde6d1b479edc7/packages/expect/src/print.ts // Mirrored from https://github.com/facebook/jest/blob/f13abff8df9a0e1148baf3584bcde6d1b479edc7/packages/expect/src/print.ts
@ -289,8 +289,8 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
const step = testInfo._addStep(stepInfo); const step = testInfo._addStep(stepInfo);
const reportStepError = (jestError: ExpectError) => { const reportStepError = (jestError: Error | unknown) => {
const error = new ExpectError(jestError, customMessage, stackFrames); const error = isExpectError(jestError) ? new ExpectError(jestError, customMessage, stackFrames) : jestError;
step.complete({ error }); step.complete({ error });
if (this._info.isSoft) if (this._info.isSoft)
testInfo._failWithError(error); testInfo._failWithError(error);

View file

@ -64,3 +64,7 @@ export class ExpectError extends Error {
this.stack = this.name + ': ' + this.message + '\n' + stringifyStackFrames(stackFrames).join('\n'); this.stack = this.name + ': ' + this.message + '\n' + stringifyStackFrames(stackFrames).join('\n');
} }
} }
export function isExpectError(e: unknown): e is ExpectError {
return e instanceof Error && 'matcherResult' in e;
}

View file

@ -30,7 +30,7 @@ import type { Attachment } from './testTracing';
import type { StackFrame } from '@protocol/channels'; import type { StackFrame } from '@protocol/channels';
export interface TestStepInternal { export interface TestStepInternal {
complete(result: { error?: Error, attachments?: Attachment[] }): void; complete(result: { error?: Error | unknown, attachments?: Attachment[] }): void;
stepId: string; stepId: string;
title: string; title: string;
category: 'hook' | 'fixture' | 'test.step' | 'expect' | 'attach' | string; category: 'hook' | 'fixture' | 'test.step' | 'expect' | 'attach' | string;
@ -270,7 +270,7 @@ export class TestInfoImpl implements TestInfo {
step.endWallTime = Date.now(); step.endWallTime = Date.now();
if (result.error) { if (result.error) {
if (!(result.error as any)[stepSymbol]) if (typeof result.error === 'object' && !(result.error as any)?.[stepSymbol])
(result.error as any)[stepSymbol] = step; (result.error as any)[stepSymbol] = step;
const error = serializeError(result.error); const error = serializeError(result.error);
if (data.boxedStack) if (data.boxedStack)
@ -327,13 +327,13 @@ export class TestInfoImpl implements TestInfo {
this.status = 'interrupted'; this.status = 'interrupted';
} }
_failWithError(error: Error) { _failWithError(error: Error | unknown) {
if (this.status === 'passed' || this.status === 'skipped') if (this.status === 'passed' || this.status === 'skipped')
this.status = error instanceof TimeoutManagerError ? 'timedOut' : 'failed'; this.status = error instanceof TimeoutManagerError ? 'timedOut' : 'failed';
const serialized = serializeError(error); const serialized = serializeError(error);
const step = (error as any)[stepSymbol] as TestStepInternal | undefined; const step: TestStepInternal | undefined = typeof error === 'object' ? (error as any)?.[stepSymbol] : undefined;
if (step && step.boxedStack) if (step && step.boxedStack)
serialized.stack = `${error.name}: ${error.message}\n${stringifyStackFrames(step.boxedStack).join('\n')}`; serialized.stack = `${(error as Error).name}: ${(error as Error).message}\n${stringifyStackFrames(step.boxedStack).join('\n')}`;
this.errors.push(serialized); this.errors.push(serialized);
this._tracing.appendForError(serialized); this._tracing.appendForError(serialized);
} }

View file

@ -232,3 +232,29 @@ test('should show intermediate result for poll that spills over test time', asyn
expect(result.output).toContain('Expected: 2'); expect(result.output).toContain('Expected: 2');
expect(result.output).toContain('Received: 3'); expect(result.output).toContain('Received: 3');
}); });
test('should propagate promise rejections', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/32256' } }, async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test('should fail', async () => {
await expect.poll(() => Promise.reject('some error')).toBe({ foo: 'baz' });
});
`
});
expect(result.output).toContain('some error');
});
test('should propagate string exception from async arrow function', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/32256' } }, async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test('should fail', async () => {
await expect.poll(async () => { throw 'some error' }).toBe({ foo: 'baz' });
});
`
});
expect(result.output).toContain('some error');
});