yujunjun/playwright
1
0
Fork 0
mirror of https://github.com/microsoft/playwright.git synced 2025-06-26 21:40:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore: completely remove annotation warnings code (#35637)

Browse Source
This commit is contained in:
Adam Gastineau 2025-04-17 05:19:14 -07:00 committed by GitHub
parent 5e3e29f7d2
commit 53abf161b0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 4 additions and 457 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
packages/playwright/src/common/testType.ts
View File

@ -25,8 +25,6 @@ import { wrapFunctionWithLocation } from '../transform/transform';
import type { FixturesWithLocation } from './config'; import type { FixturesWithLocation } from './config';
import type { Fixtures, TestDetails, TestStepInfo, TestType } from '../../types/test'; import type { Fixtures, TestDetails, TestStepInfo, TestType } from '../../types/test';
import type { Location } from '../../types/testReporter'; import type { Location } from '../../types/testReporter';
import type { TestInfoImpl, TestStepInternal } from '../worker/testInfo';
const testTypeSymbol = Symbol('testType'); const testTypeSymbol = Symbol('testType');
@ -262,15 +260,11 @@ export class TestTypeImpl {
suite._use.push({ fixtures, location }); suite._use.push({ fixtures, location });
} }
_step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> { async _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(); const testInfo = currentTestInfo();
if (!testInfo) if (!testInfo)
throw new Error(`test.step() can only be called from a test`); throw new Error(`test.step() can only be called from a test`);
const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box }); const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, step, body, options), step.location);
}
private async _stepInternal<T>(expectation: 'pass'|'skip', testInfo: TestInfoImpl, step: TestStepInternal, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
return await currentZone().with('stepZone', step).run(async () => { return await currentZone().with('stepZone', step).run(async () => {
try { try {
let result: Awaited<ReturnType<typeof raceAgainstDeadline<T>>> | undefined = undefined; let result: Awaited<ReturnType<typeof raceAgainstDeadline<T>>> | undefined = undefined;

6
packages/playwright/src/matchers/expect.ts
View File

@ -385,10 +385,8 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
setMatcherCallContext({ expectInfo: this._info, testInfo, step: step.info }); setMatcherCallContext({ expectInfo: this._info, testInfo, step: step.info });
const callback = () => matcher.call(target, ...args); const callback = () => matcher.call(target, ...args);
const result = currentZone().with('stepZone', step).run(callback); const result = currentZone().with('stepZone', step).run(callback);
if (result instanceof Promise) { if (result instanceof Promise)
const promise = result.then(finalizer).catch(reportStepError); return result.then(finalizer).catch(reportStepError);
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise, stackFrames[0]);
}
finalizer(); finalizer();
return result; return result;
} catch (e) { } catch (e) {

6
packages/playwright/src/reporters/base.ts
View File

@ -319,7 +319,6 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
const header = formatTestHeader(screen, config, test, { indent: ' ', index, mode: 'error' }); const header = formatTestHeader(screen, config, test, { indent: ' ', index, mode: 'error' });
lines.push(screen.colors.red(header)); lines.push(screen.colors.red(header));
for (const result of test.results) { for (const result of test.results) {
// const warnings = result.annotations.filter(a => a.type === 'warning');
const resultLines: string[] = []; const resultLines: string[] = [];
const errors = formatResultFailure(screen, test, result, ' '); const errors = formatResultFailure(screen, test, result, ' ');
if (!errors.length) if (!errors.length)
@ -329,11 +328,6 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
resultLines.push(screen.colors.gray(separator(screen, ` Retry #${result.retry}`))); resultLines.push(screen.colors.gray(separator(screen, ` Retry #${result.retry}`)));
} }
resultLines.push(...errors.map(error => '\n' + error.message)); resultLines.push(...errors.map(error => '\n' + error.message));
// TODO: 1.53: Actually build annotations
// if (warnings.length) {
// resultLines.push('');
// resultLines.push(...formatTestWarning(screen, config, warnings));
// }
for (let i = 0; i < result.attachments.length; ++i) { for (let i = 0; i < result.attachments.length; ++i) {
const attachment = result.attachments[i]; const attachment = result.attachments[i];
if (attachment.name.startsWith('_error-context') && attachment.path) { if (attachment.name.startsWith('_error-context') && attachment.path) {

62
packages/playwright/src/worker/floatingPromiseScope.ts
View File

@ -1,62 +0,0 @@
/**
* 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 type { Location } from '../../types/test';
export class FloatingPromiseScope {
readonly _floatingCalls: Map<Promise<any>, Location | undefined> = new Map();
/**
* 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>, location: Location | undefined): Promise<T> {
if (process.env.PW_DISABLE_FLOATING_PROMISES_WARNING)
return promise;
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.set(promise, location);
return promiseProxy;
}
clear() {
this._floatingCalls.clear();
}
hasFloatingPromises(): boolean {
return this._floatingCalls.size > 0;
}
floatingPromises(): Array<{ location: Location | undefined, promise: Promise<any> }> {
return Array.from(this._floatingCalls.entries()).map(([promise, location]) => ({ location, promise }));
}
}

2
packages/playwright/src/worker/testInfo.ts
View File

@ -23,7 +23,6 @@ import { TimeoutManager, TimeoutManagerError, kMaxDeadline } from './timeoutMana
import { filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, trimLongString, windowsFilesystemFriendlyLength } from '../util'; import { filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, trimLongString, windowsFilesystemFriendlyLength } from '../util';
import { TestTracing } from './testTracing'; import { TestTracing } from './testTracing';
import { testInfoError } from './util'; import { testInfoError } from './util';
import { FloatingPromiseScope } from './floatingPromiseScope';
import type { RunnableDescription } from './timeoutManager'; import type { RunnableDescription } from './timeoutManager';
import type { FullProject, TestInfo, TestStatus, TestStepInfo } from '../../types/test'; import type { FullProject, TestInfo, TestStatus, TestStepInfo } from '../../types/test';
@ -60,7 +59,6 @@ export class TestInfoImpl implements TestInfo {
readonly _startTime: number; readonly _startTime: number;
readonly _startWallTime: number; readonly _startWallTime: number;
readonly _tracing: TestTracing; readonly _tracing: TestTracing;
readonly _floatingPromiseScope: FloatingPromiseScope = new FloatingPromiseScope();
readonly _uniqueSymbol; readonly _uniqueSymbol;
_wasInterrupted = false; _wasInterrupted = false;

19
packages/playwright/src/worker/workerMain.ts
View File

@ -427,26 +427,9 @@ export class WorkerMain extends ProcessRunner {
throw firstAfterHooksError; throw firstAfterHooksError;
}).catch(() => {}); // Ignore the top-level error, it is already inside TestInfo.errors. }).catch(() => {}); // Ignore the top-level error, it is already inside TestInfo.errors.
if (testInfo._isFailure()) { if (testInfo._isFailure())
this._isStopped = true; this._isStopped = true;
// Only if failed, create warning if any of the async calls were not awaited in various stages.
if (!process.env.PW_DISABLE_FLOATING_PROMISES_WARNING && testInfo._floatingPromiseScope.hasFloatingPromises()) {
// TODO: 1.53: Actually build annotations
// Dedupe by location
// const annotationLocations = new Map<string | undefined, Location | undefined>(testInfo._floatingPromiseScope.floatingPromises().map(
// ({ location }) => {
// const locationKey = location ? `${location.file}:${location.line}:${location.column}` : undefined;
// return [locationKey, location];
// }));
// testInfo.annotations.push(...[...annotationLocations.values()].map(location => ({
// type: 'warning', description: `This async call was not awaited by the end of the test. This can cause flakiness. It is recommended to run ESLint with "@typescript-eslint/no-floating-promises" to verify.`, location
// })));
testInfo._floatingPromiseScope.clear();
}
}
if (this._isStopped) { if (this._isStopped) {
// Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down. // Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down.
// Mark as "cleaned up" early to avoid running cleanup twice. // Mark as "cleaned up" early to avoid running cleanup twice.

67
tests/playwright-test/reporter-base.spec.ts
View File

@ -486,72 +486,5 @@ for (const useIntermediateMergeReport of [false, true] as const) {
expect(text).toContain('› passes @bar1 @bar2 ('); expect(text).toContain('› passes @bar1 @bar2 (');
expect(text).toContain('› passes @baz1 @baz2 ('); expect(text).toContain('› passes @baz1 @baz2 (');
}); });
test.skip('should show warnings on failing tests', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test('fail', async ({ page }, testInfo) => {
testInfo.annotations.push({ type: 'warning', description: 'foo' });
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
throw new Error();
});
`,
});
expect(result.exitCode).toBe(1);
expect(result.passed).toBe(0);
expect(result.output).toContain('Warning: a.spec.ts:5:41: This async call was not awaited by the end of the test.');
expect(result.output).toContain('Warning: foo');
});
test.skip('should not show warnings on passing tests', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test('success', async ({ page }, testInfo) => {
testInfo.annotations.push({ type: 'warning', description: 'foo' });
});
`,
});
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
expect(result.output).not.toContain('Warning: foo');
});
test.skip('should properly sort warnings', async ({ runInlineTest }) => {
const result = await runInlineTest({
'external.js': `
import { expect } from '@playwright/test';
export const externalAsyncCall = (page) => {
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
};
`,
'a.spec.ts': `
import { test, expect } from '@playwright/test';
import { externalAsyncCall } from './external.js';
test('fail a', async ({ page }, testInfo) => {
testInfo.annotations.push({ type: 'warning', description: 'foo' });
externalAsyncCall(page);
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
testInfo.annotations.push({ type: 'warning', description: 'bar' });
throw new Error();
});
`,
});
expect(result.exitCode).toBe(1);
expect(result.passed).toBe(0);
expect(result.output).toContain('Warning: a.spec.ts:7:41: This async call was not awaited by the end of the test.');
expect(result.output).toContain('Warning: external.js:4:41: This async call was not awaited by the end of the test.');
expect(result.output).toContain('Warning: foo');
expect(result.output).toContain('Warning: bar');
const manualIndexFoo = result.output.indexOf('Warning: foo');
const manualIndexBar = result.output.indexOf('Warning: bar');
const externalIndex = result.output.indexOf('Warning: external.js:4:41');
const specIndex = result.output.indexOf('Warning: a.spec.ts:7:41');
expect(specIndex).toBeLessThan(externalIndex);
expect(externalIndex).toBeLessThan(manualIndexFoo);
expect(manualIndexFoo).toBeLessThan(manualIndexBar);
});
}); });
} }

291
tests/playwright-test/warnings.spec.ts
View File

@ -1,291 +0,0 @@
/**
* 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 description = 'This async call was not awaited by the end of the test. This can cause flakiness. It is recommended to run ESLint with "@typescript-eslint/no-floating-promises" to verify.';
test.describe.configure({ mode: 'parallel' });
test.describe.skip('await', () => {
test('should not care about non-API promises', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test } from '@playwright/test';
test('test', async () => {
new Promise(() => {});
await expect(page.locator('div')).toHaveText('A', { timeout: 100 });
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([]);
});
test('should warn on failure', async ({ runInlineTest }) => {
const { exitCode, results } = 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 });
// Timeout to make sure the expect actually gets processed
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 39 }) }]);
});
test('should not warn on success', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('custom test name', async ({ page }) => {
await page.setContent('<div>A</div>');
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(0);
expect(results[0].annotations).toEqual([]);
});
test('should warn about missing await on expects', async ({ runInlineTest }) => {
const { exitCode, results } = 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 });
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 39 }) }]);
});
test('should not warn when not missing await on expects', async ({ runInlineTest }) => {
const { exitCode, results } = 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(results[0].annotations).toEqual([]);
});
test('should not warn when using then() on expects', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
expect(page.locator('div')).toHaveText('A').then(() => {});
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([]);
});
test('should warn about missing await on resolve', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
expect(Promise.reject(new Error('foo'))).resolves.toBe('foo');
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 61 }) }]);
});
test('should warn about missing await on reject.not', async ({ runInlineTest }) => {
const { exitCode, results } = 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');
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 64 }) }]);
});
test('should warn about missing await on test.step', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
test.step('step', () => {});
await expect(page.locator('div')).toHaveText('A', { timeout: 100 });
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 16 }) }]);
});
test('should not warn when not missing await on test.step', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
await test.step('step', () => {});
await expect(page.locator('div')).toHaveText('A', { timeout: 100 });
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([]);
});
test('should warn about missing await on test.step.skip', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
test.step.skip('step', () => {});
await expect(page.locator('div')).toHaveText('A');
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 21 }) }]);
});
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('<div>A</div>');
const expectPromise = expect(page.locator('div')).toHaveText('A');
expect(expectPromise instanceof Promise).toBeTruthy();
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(0);
});
test('should warn about missing await in before hooks', async ({ runInlineTest }) => {
const group = ['beforeAll', 'beforeEach'];
for (const hook of group) {
await test.step(hook, async () => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
let page;
test.${hook}(async ({ browser }) => {
page = await browser.newPage();
await page.setContent('<div>A</div>');
expect(page.locator('div')).toHaveText('A');
await new Promise(f => setTimeout(f, 1000));
});
test('test ${hook}', async () => {
await expect(page.locator('button')).toBeVisible({ timeout: 100 });
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 7, column: 43 }) }]);
});
}
});
test.describe('should warn about missing await in after hooks', () => {
const group = ['afterAll', 'afterEach'];
for (const hook of group) {
test(hook, async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
let page;
test('test ${hook}', async ({ browser }) => {
await expect(Promise.resolve()).resolves.toBe('A');
});
test.${hook}(async () => {
expect(Promise.resolve()).resolves.toBe(undefined);
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 8, column: 50 }) }]);
});
}
});
test('should warn about missing await across hooks and test', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test.beforeAll(async () => {
expect(Promise.resolve()).resolves.toBe(undefined);
await new Promise(f => setTimeout(f, 1000));
});
test('test', async () => {
expect(Promise.resolve()).resolves.toBe('A');
await new Promise(f => setTimeout(f, 1000));
});
test.afterEach(async () => {
expect(Promise.resolve()).resolves.toBe(undefined);
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([
{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 4, column: 46 }) },
{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 8, column: 46 }) },
{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 12, column: 46 }) },
]);
});
test('should dedupe warnings that occur at the same location', async ({ runInlineTest }) => {
const { exitCode, results } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
for (let i = 0; i < 3; i++) {
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
}
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
await new Promise(f => setTimeout(f, 1000));
});
`
});
expect(exitCode).toBe(1);
expect(results[0].annotations).toEqual([
{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 5, column: 41 }) },
{ type: 'warning', description, location: expect.objectContaining({ file: expect.stringMatching(/a\.test\.ts$/), line: 7, column: 39 }) },
]);
});
});