Moved scope logic to separate class

This commit is contained in:
Adam Gastineau 2025-02-19 06:10:41 -08:00
parent 3a31128827
commit d84588b565
6 changed files with 57 additions and 32 deletions

View file

@ -266,10 +266,10 @@ export class TestTypeImpl {
const testInfo = currentTestInfo();
if (!testInfo)
throw new Error(`test.step() can only be called from a test`);
return testInfo._wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, title, body, options));
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, title, body, options));
}
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> {
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 {

View file

@ -383,7 +383,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
const result = currentZone().with('stepZone', step).run(callback);
if (result instanceof Promise) {
const promise = result.then(finalizer).catch(reportStepError);
return testInfo._wrapPromiseAPIResult(promise);
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise);
}
finalizer();
return result;

View file

@ -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;
}
}

View file

@ -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';
@ -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;
@ -99,7 +101,6 @@ export class TestInfoImpl implements TestInfo {
duration: number = 0;
readonly annotations: Annotation[] = [];
readonly attachments: TestInfo['attachments'] = [];
readonly unusedAsyncApiCalls: Set<Promise<any>> = new Set();
status: TestStatus = 'passed';
snapshotSuffix: string = '';
readonly outputDir: string;
@ -411,32 +412,6 @@ export class TestInfoImpl implements TestInfo {
this._timeoutManager.setIgnoreTimeouts();
}
/**
* 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, reciever) => {
if (prop === 'then') {
return (...args: any[]) => {
this.unusedAsyncApiCalls.delete(promise);
const originalThen = Reflect.get(target, prop, reciever) as Promise<T>['then'];
return originalThen.call(target, ...args);
};
} else {
return Reflect.get(target, prop, reciever);
}
}
});
this.unusedAsyncApiCalls.add(promise);
return promiseProxy;
}
// ------------ TestInfo methods ------------
async attach(name: string, options: { path?: string, body?: string | Buffer, contentType?: string } = {}) {

View file

@ -370,7 +370,7 @@ export class WorkerMain extends ProcessRunner {
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.unusedAsyncApiCalls.size > 0)
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.

View file

@ -38,13 +38,14 @@ test.describe('await', () => {
const { exitCode, stdout } = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({ page }) => {
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 }) => {