Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/playwright/src/common/testType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { wrapFunctionWithLocation } from '../transform/transform';
import type { FixturesWithLocation } from './config';
import type { Fixtures, TestDetails, TestStepInfo, TestType } from '../../types/test';
import type { Location } from '../../types/testReporter';
import type { TestInfoImpl } from '../worker/testInfo';


const testTypeSymbol = Symbol('testType');
Expand Down Expand Up @@ -261,10 +262,14 @@ export class TestTypeImpl {
suite._use.push({ fixtures, location });
}

async _step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
_step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
const testInfo = currentTestInfo();
if (!testInfo)
throw new Error(`test.step() can only be called from a test`);
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, title, body, options));
}

private async _stepInternal<T>(expectation: 'pass'|'skip', testInfo: TestInfoImpl, title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
return await currentZone().with('stepZone', step).run(async () => {
try {
Expand Down
6 changes: 4 additions & 2 deletions packages/playwright/src/matchers/expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,10 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
setMatcherCallContext({ expectInfo: this._info, testInfo, step: step.info });
const callback = () => matcher.call(target, ...args);
const result = currentZone().with('stepZone', step).run(callback);
if (result instanceof Promise)
return result.then(finalizer).catch(reportStepError);
if (result instanceof Promise) {
const promise = result.then(finalizer).catch(reportStepError);
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise);
}
finalizer();
return result;
} catch (e) {
Expand Down
23 changes: 23 additions & 0 deletions packages/playwright/src/reporters/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ export class TerminalReporter implements ReporterV2 {
if (full && summary.failuresToPrint.length && !this._omitFailures)
this._printFailures(summary.failuresToPrint);
this._printSlowTests();
this._printWarnings();
this._printSummary(summaryMessage);
}

Expand All @@ -289,6 +290,28 @@ export class TerminalReporter implements ReporterV2 {
console.log(this.screen.colors.yellow(' Consider running tests from slow files in parallel, see https://playwright.dev/docs/test-parallel.'));
}

private _printWarnings() {
const warningTests = this.suite.allTests().filter(test => test.annotations.some(a => a.type === 'warning'));
const encounteredWarnings = new Map<string, Array<TestCase>>();
for (const test of warningTests) {
for (const annotation of test.annotations) {
if (annotation.type !== 'warning' || annotation.description === undefined)
continue;
let tests = encounteredWarnings.get(annotation.description);
if (!tests) {
tests = [];
encounteredWarnings.set(annotation.description, tests);
}
tests.push(test);
}
}
for (const [description, tests] of encounteredWarnings) {
console.log(this.screen.colors.yellow(' Warning: ') + description);
for (const test of tests)
console.log(this.formatTestHeader(test, { indent: ' ', mode: 'default' }));
}
}

private _printSummary(summary: string) {
if (summary.trim())
console.log(summary);
Expand Down
49 changes: 49 additions & 0 deletions packages/playwright/src/worker/floatingPromiseScope.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export class FloatingPromiseScope {
readonly _floatingCalls: Set<Promise<any>> = new Set();

/**
* Enables a promise API call to be tracked by the test, alerting if unawaited.
*
* **NOTE:** Returning from an async function wraps the result in a promise, regardless of whether the return value is a promise. This will automatically mark the promise as awaited. Avoid this.
*/
wrapPromiseAPIResult<T>(promise: Promise<T>): Promise<T> {
const promiseProxy = new Proxy(promise, {
get: (target, prop, receiver) => {
if (prop === 'then') {
return (...args: any[]) => {
this._floatingCalls.delete(promise);

const originalThen = Reflect.get(target, prop, receiver) as Promise<T>['then'];
return originalThen.call(target, ...args);
};
} else {
return Reflect.get(target, prop, receiver);
}
}
});

this._floatingCalls.add(promise);

return promiseProxy;
}

hasFloatingPromises(): boolean {
return this._floatingCalls.size > 0;
}
}
2 changes: 2 additions & 0 deletions packages/playwright/src/worker/testInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { TimeoutManager, TimeoutManagerError, kMaxDeadline } from './timeoutMana
import { debugTest, filteredStackTrace, formatLocation, getContainedPath, normalizeAndSaveAttachment, trimLongString, windowsFilesystemFriendlyLength } from '../util';
import { TestTracing } from './testTracing';
import { testInfoError } from './util';
import { FloatingPromiseScope } from './floatingPromiseScope';

import type { RunnableDescription } from './timeoutManager';
import type { FullProject, TestInfo, TestStatus, TestStepInfo } from '../../types/test';
Expand Down Expand Up @@ -67,6 +68,7 @@ export class TestInfoImpl implements TestInfo {
readonly _startTime: number;
readonly _startWallTime: number;
readonly _tracing: TestTracing;
readonly _floatingPromiseScope: FloatingPromiseScope = new FloatingPromiseScope();

_wasInterrupted = false;
_lastStepId = 0;
Expand Down
3 changes: 3 additions & 0 deletions packages/playwright/src/worker/workerMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ export class WorkerMain extends ProcessRunner {
// Now run the test itself.
const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]").
await fn(testFunctionParams, testInfo);
// Create warning if any of the async calls were not awaited.
if (testInfo._floatingPromiseScope.hasFloatingPromises())
testInfo.annotations.push({ type: 'warning', description: 'Some async calls were not awaited by the end of the test. This can cause flakiness.' });
});
}).catch(() => {}); // Ignore the top-level error, it is already inside TestInfo.errors.

Expand Down
176 changes: 176 additions & 0 deletions tests/playwright-test/warnings.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { test, expect } from './playwright-test-fixtures';

const warningSnippet = 'Some async calls were not awaited';

test.describe.configure({ mode: 'parallel' });

test.describe('await', () => {
test('should not care about non-API promises', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test } from '@playwright/test';
test('test', () => {
new Promise(() => {});
});
`
});
expect(exitCode).toBe(0);
expect(stdout).not.toContain(warningSnippet);
});

test('should warn about missing await on expects when failing', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('custom test name', async ({ page }) => {
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
});
`
});
expect(exitCode).toBe(1);
expect(stdout).toContain(warningSnippet);
expect(stdout).toContain('custom test name');
});

test('should warn about missing await on expects when passing', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.setContent('data:text/html,<div>A</div>');
expect(page.locator('div')).toHaveText('A');
});
`
});
expect(exitCode).toBe(0);
expect(stdout).toContain(warningSnippet);
});

test('should not warn when not missing await on expects when failing', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await expect(page.locator('div')).toHaveText('A', { timeout: 100 });
});
`
});
expect(exitCode).toBe(1);
expect(stdout).not.toContain(warningSnippet);
});

test('should not warn when not missing await on expects when passing', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.setContent('data:text/html,<div>A</div>');
await expect(page.locator('div')).toHaveText('A');
});
`
});
expect(exitCode).toBe(0);
expect(stdout).not.toContain(warningSnippet);
});

test('should warn about missing await on reject', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
expect(Promise.reject(new Error('foo'))).rejects.toThrow('foo');
});
`
});
expect(exitCode).toBe(0);
expect(stdout).toContain(warningSnippet);
});

test('should warn about missing await on reject.not', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
expect(Promise.reject(new Error('foo'))).rejects.not.toThrow('foo');
});
`
});
expect(exitCode).toBe(1);
expect(stdout).toContain(warningSnippet);
});

test('should warn about missing await on test.step', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.setContent('data:text/html,<div>A</div>');
test.step('step', () => {});
await expect(page.locator('div')).toHaveText('A');
});
`
});
expect(exitCode).toBe(0);
expect(stdout).toContain(warningSnippet);
});

test('should not warn when not missing await on test.step', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.setContent('data:text/html,<div>A</div>');
await test.step('step', () => {});
await expect(page.locator('div')).toHaveText('A');
});
`
});
expect(exitCode).toBe(0);
expect(stdout).not.toContain(warningSnippet);
});

test('should warn about missing await on test.step.skip', async ({ runInlineTest }) => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.setContent('data:text/html,<div>A</div>');
test.step.skip('step', () => {});
await expect(page.locator('div')).toHaveText('A');
});
`
});
expect(exitCode).toBe(0);
expect(stdout).toContain(warningSnippet);
});

test('traced promise should be instanceof Promise', async ({ runInlineTest }) => {
const { exitCode } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await page.setContent('data:text/html,<div>A</div>');
const expectPromise = expect(page.locator('div')).toHaveText('A');
expect(expectPromise instanceof Promise).toBeTruthy();
});
`
});
expect(exitCode).toBe(0);
});
});
Loading