Skip to content

Commit 0259acf

Browse files
agg23mxschmitt
authored andcommitted
cherry-pick(#35637): chore: completely remove annotation warnings code
1 parent a0fdb3f commit 0259acf

File tree

8 files changed

+4
-457
lines changed

8 files changed

+4
-457
lines changed

packages/playwright/src/common/testType.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ import { wrapFunctionWithLocation } from '../transform/transform';
2525
import type { FixturesWithLocation } from './config';
2626
import type { Fixtures, TestDetails, TestStepInfo, TestType } from '../../types/test';
2727
import type { Location } from '../../types/testReporter';
28-
import type { TestInfoImpl, TestStepInternal } from '../worker/testInfo';
29-
3028

3129
const testTypeSymbol = Symbol('testType');
3230

@@ -262,15 +260,11 @@ export class TestTypeImpl {
262260
suite._use.push({ fixtures, location });
263261
}
264262

265-
_step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
263+
async _step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
266264
const testInfo = currentTestInfo();
267265
if (!testInfo)
268266
throw new Error(`test.step() can only be called from a test`);
269267
const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
270-
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, step, body, options), step.location);
271-
}
272-
273-
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> {
274268
return await currentZone().with('stepZone', step).run(async () => {
275269
try {
276270
let result: Awaited<ReturnType<typeof raceAgainstDeadline<T>>> | undefined = undefined;

packages/playwright/src/matchers/expect.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -385,10 +385,8 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
385385
setMatcherCallContext({ expectInfo: this._info, testInfo, step: step.info });
386386
const callback = () => matcher.call(target, ...args);
387387
const result = currentZone().with('stepZone', step).run(callback);
388-
if (result instanceof Promise) {
389-
const promise = result.then(finalizer).catch(reportStepError);
390-
return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise, stackFrames[0]);
391-
}
388+
if (result instanceof Promise)
389+
return result.then(finalizer).catch(reportStepError);
392390
finalizer();
393391
return result;
394392
} catch (e) {

packages/playwright/src/reporters/base.ts

-6
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,6 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
319319
const header = formatTestHeader(screen, config, test, { indent: ' ', index, mode: 'error' });
320320
lines.push(screen.colors.red(header));
321321
for (const result of test.results) {
322-
// const warnings = result.annotations.filter(a => a.type === 'warning');
323322
const resultLines: string[] = [];
324323
const errors = formatResultFailure(screen, test, result, ' ');
325324
if (!errors.length)
@@ -329,11 +328,6 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
329328
resultLines.push(screen.colors.gray(separator(screen, ` Retry #${result.retry}`)));
330329
}
331330
resultLines.push(...errors.map(error => '\n' + error.message));
332-
// TODO: 1.53: Actually build annotations
333-
// if (warnings.length) {
334-
// resultLines.push('');
335-
// resultLines.push(...formatTestWarning(screen, config, warnings));
336-
// }
337331
for (let i = 0; i < result.attachments.length; ++i) {
338332
const attachment = result.attachments[i];
339333
if (attachment.name.startsWith('_error-context') && attachment.path) {

packages/playwright/src/worker/floatingPromiseScope.ts

-62
This file was deleted.

packages/playwright/src/worker/testInfo.ts

-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import { TimeoutManager, TimeoutManagerError, kMaxDeadline } from './timeoutMana
2323
import { filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, trimLongString, windowsFilesystemFriendlyLength } from '../util';
2424
import { TestTracing } from './testTracing';
2525
import { testInfoError } from './util';
26-
import { FloatingPromiseScope } from './floatingPromiseScope';
2726

2827
import type { RunnableDescription } from './timeoutManager';
2928
import type { FullProject, TestInfo, TestStatus, TestStepInfo } from '../../types/test';
@@ -60,7 +59,6 @@ export class TestInfoImpl implements TestInfo {
6059
readonly _startTime: number;
6160
readonly _startWallTime: number;
6261
readonly _tracing: TestTracing;
63-
readonly _floatingPromiseScope: FloatingPromiseScope = new FloatingPromiseScope();
6462
readonly _uniqueSymbol;
6563

6664
_wasInterrupted = false;

packages/playwright/src/worker/workerMain.ts

+1-18
Original file line numberDiff line numberDiff line change
@@ -427,26 +427,9 @@ export class WorkerMain extends ProcessRunner {
427427
throw firstAfterHooksError;
428428
}).catch(() => {}); // Ignore the top-level error, it is already inside TestInfo.errors.
429429

430-
if (testInfo._isFailure()) {
430+
if (testInfo._isFailure())
431431
this._isStopped = true;
432432

433-
// Only if failed, create warning if any of the async calls were not awaited in various stages.
434-
if (!process.env.PW_DISABLE_FLOATING_PROMISES_WARNING && testInfo._floatingPromiseScope.hasFloatingPromises()) {
435-
// TODO: 1.53: Actually build annotations
436-
// Dedupe by location
437-
// const annotationLocations = new Map<string | undefined, Location | undefined>(testInfo._floatingPromiseScope.floatingPromises().map(
438-
// ({ location }) => {
439-
// const locationKey = location ? `${location.file}:${location.line}:${location.column}` : undefined;
440-
// return [locationKey, location];
441-
// }));
442-
443-
// testInfo.annotations.push(...[...annotationLocations.values()].map(location => ({
444-
// 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
445-
// })));
446-
testInfo._floatingPromiseScope.clear();
447-
}
448-
}
449-
450433
if (this._isStopped) {
451434
// Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down.
452435
// Mark as "cleaned up" early to avoid running cleanup twice.

tests/playwright-test/reporter-base.spec.ts

-67
Original file line numberDiff line numberDiff line change
@@ -486,72 +486,5 @@ for (const useIntermediateMergeReport of [false, true] as const) {
486486
expect(text).toContain('› passes @bar1 @bar2 (');
487487
expect(text).toContain('› passes @baz1 @baz2 (');
488488
});
489-
490-
test.skip('should show warnings on failing tests', async ({ runInlineTest }) => {
491-
const result = await runInlineTest({
492-
'a.spec.ts': `
493-
import { test, expect } from '@playwright/test';
494-
test('fail', async ({ page }, testInfo) => {
495-
testInfo.annotations.push({ type: 'warning', description: 'foo' });
496-
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
497-
throw new Error();
498-
});
499-
`,
500-
});
501-
expect(result.exitCode).toBe(1);
502-
expect(result.passed).toBe(0);
503-
expect(result.output).toContain('Warning: a.spec.ts:5:41: This async call was not awaited by the end of the test.');
504-
expect(result.output).toContain('Warning: foo');
505-
});
506-
507-
test.skip('should not show warnings on passing tests', async ({ runInlineTest }) => {
508-
const result = await runInlineTest({
509-
'a.spec.ts': `
510-
import { test, expect } from '@playwright/test';
511-
test('success', async ({ page }, testInfo) => {
512-
testInfo.annotations.push({ type: 'warning', description: 'foo' });
513-
});
514-
`,
515-
});
516-
expect(result.exitCode).toBe(0);
517-
expect(result.passed).toBe(1);
518-
expect(result.output).not.toContain('Warning: foo');
519-
});
520-
521-
test.skip('should properly sort warnings', async ({ runInlineTest }) => {
522-
const result = await runInlineTest({
523-
'external.js': `
524-
import { expect } from '@playwright/test';
525-
export const externalAsyncCall = (page) => {
526-
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
527-
};
528-
`,
529-
'a.spec.ts': `
530-
import { test, expect } from '@playwright/test';
531-
import { externalAsyncCall } from './external.js';
532-
test('fail a', async ({ page }, testInfo) => {
533-
testInfo.annotations.push({ type: 'warning', description: 'foo' });
534-
externalAsyncCall(page);
535-
expect(page.locator('div')).toHaveText('A', { timeout: 100 });
536-
testInfo.annotations.push({ type: 'warning', description: 'bar' });
537-
throw new Error();
538-
});
539-
`,
540-
});
541-
expect(result.exitCode).toBe(1);
542-
expect(result.passed).toBe(0);
543-
expect(result.output).toContain('Warning: a.spec.ts:7:41: This async call was not awaited by the end of the test.');
544-
expect(result.output).toContain('Warning: external.js:4:41: This async call was not awaited by the end of the test.');
545-
expect(result.output).toContain('Warning: foo');
546-
expect(result.output).toContain('Warning: bar');
547-
548-
const manualIndexFoo = result.output.indexOf('Warning: foo');
549-
const manualIndexBar = result.output.indexOf('Warning: bar');
550-
const externalIndex = result.output.indexOf('Warning: external.js:4:41');
551-
const specIndex = result.output.indexOf('Warning: a.spec.ts:7:41');
552-
expect(specIndex).toBeLessThan(externalIndex);
553-
expect(externalIndex).toBeLessThan(manualIndexFoo);
554-
expect(manualIndexFoo).toBeLessThan(manualIndexBar);
555-
});
556489
});
557490
}

0 commit comments

Comments
 (0)