diff --git a/packages/playwright/src/reporters/base.ts b/packages/playwright/src/reporters/base.ts index 7affd1a016..3d905fcfca 100644 --- a/packages/playwright/src/reporters/base.ts +++ b/packages/playwright/src/reporters/base.ts @@ -270,7 +270,8 @@ export class TerminalReporter implements ReporterV2 { if (full && summary.failuresToPrint.length && !this._omitFailures) this._printFailures(summary.failuresToPrint); this._printSlowTests(); - this._printWarnings(); + // TODO: 1.52: Make warning display prettier + // this._printWarnings(); this._printSummary(summaryMessage); } diff --git a/packages/playwright/src/worker/floatingPromiseScope.ts b/packages/playwright/src/worker/floatingPromiseScope.ts index 5c3997f5e6..630183dece 100644 --- a/packages/playwright/src/worker/floatingPromiseScope.ts +++ b/packages/playwright/src/worker/floatingPromiseScope.ts @@ -23,6 +23,9 @@ export class FloatingPromiseScope { * **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(promise: Promise): Promise { + if (process.env.PW_DISABLE_FLOATING_PROMISES_WARNING) + return promise; + const promiseProxy = new Proxy(promise, { get: (target, prop, receiver) => { if (prop === 'then') { diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index 91abde8a2e..126d3c8052 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -324,9 +324,12 @@ export class WorkerMain extends ProcessRunner { // Create warning if any of the async calls were not awaited in various stages. const checkForFloatingPromises = (functionDescription: string) => { + if (process.env.PW_DISABLE_FLOATING_PROMISES_WARNING) + return; if (!testInfo._floatingPromiseScope.hasFloatingPromises()) return; - testInfo.annotations.push({ type: 'warning', description: `Some async calls were not awaited by the end of ${functionDescription}. This can cause flakiness.` }); + // TODO: 1.52: Actually build annotations + // testInfo.annotations.push({ type: 'warning', description: `Some async calls were not awaited by the end of ${functionDescription}. This can cause flakiness.` }); testInfo._floatingPromiseScope.clear(); }; diff --git a/tests/playwright-test/warnings.spec.ts b/tests/playwright-test/warnings.spec.ts index d9ed42eb5d..2734c9593e 100644 --- a/tests/playwright-test/warnings.spec.ts +++ b/tests/playwright-test/warnings.spec.ts @@ -14,249 +14,274 @@ * limitations under the License. */ -import { test, expect } from './playwright-test-fixtures'; +// import { JSONReport } from 'packages/playwright-test/reporter'; +// import { test, expect } from './playwright-test-fixtures'; -const warningSnippet = 'Some async calls were not awaited'; +// const warningSnippet = 'Some async calls were not awaited'; -test.describe.configure({ mode: 'parallel' }); +// 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); - }); +// const getWarnings = (report: JSONReport) => report.suites.flatMap(s => s.specs).flatMap(s => s.tests).flatMap(t => t.annotations).filter(a => a.type === 'warning'); - 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('the test'); - expect(stdout).toContain('custom test name'); - }); +// test.describe('await', () => { +// test('should not care about non-API promises', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test } from '@playwright/test'; +// test('test', () => { +// new Promise(() => {}); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(0); +// }); - 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('
A
'); - expect(page.locator('div')).toHaveText('A'); - }); - ` - }); - expect(exitCode).toBe(0); - expect(stdout).toContain(warningSnippet); - }); +// test('should warn about missing await on expects when failing', async ({ runInlineTest }) => { +// const { exitCode, report } = 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); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).toContain(warningSnippet); +// expect(warnings[0].description).toContain('the test'); +// }); - 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 warn about missing await on expects when passing', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test('test', async ({ page }) => { +// await page.setContent('
A
'); +// expect(page.locator('div')).toHaveText('A'); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).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('
A
'); - await expect(page.locator('div')).toHaveText('A'); - }); - ` - }); - expect(exitCode).toBe(0); - expect(stdout).not.toContain(warningSnippet); - }); +// test('should not warn when not missing await on expects when failing', async ({ runInlineTest }) => { +// const { exitCode, report } = 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); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(0); +// }); - test('should not warn when using then 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('
A
'); - expect(page.locator('div')).toHaveText('A').then(() => {}); - }); - ` - }); - expect(exitCode).toBe(0); - expect(stdout).not.toContain(warningSnippet); - }); +// test('should not warn when not missing await on expects when passing', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test('test', async ({ page }) => { +// await page.setContent('
A
'); +// await expect(page.locator('div')).toHaveText('A'); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(0); +// }); - 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 not warn when using then on expects when passing', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test('test', async ({ page }) => { +// await page.setContent('
A
'); +// expect(page.locator('div')).toHaveText('A').then(() => {}); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(0); +// }); - 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 reject', async ({ runInlineTest }) => { +// const { exitCode, report } = 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); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).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('
A
'); - test.step('step', () => {}); - await expect(page.locator('div')).toHaveText('A'); - }); - ` - }); - expect(exitCode).toBe(0); - expect(stdout).toContain(warningSnippet); - }); +// test('should warn about missing await on reject.not', async ({ runInlineTest }) => { +// const { exitCode, report } = 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); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).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('
A
'); - 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', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test('test', async ({ page }) => { +// await page.setContent('
A
'); +// test.step('step', () => {}); +// await expect(page.locator('div')).toHaveText('A'); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).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('
A
'); - test.step.skip('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, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test('test', async ({ page }) => { +// await page.setContent('
A
'); +// await test.step('step', () => {}); +// await expect(page.locator('div')).toHaveText('A'); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(0); +// }); - 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('
A
'); - const expectPromise = expect(page.locator('div')).toHaveText('A'); - expect(expectPromise instanceof Promise).toBeTruthy(); - }); - ` - }); - expect(exitCode).toBe(0); - }); +// test('should warn about missing await on test.step.skip', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test('test', async ({ page }) => { +// await page.setContent('
A
'); +// test.step.skip('step', () => {}); +// await expect(page.locator('div')).toHaveText('A'); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).toContain(warningSnippet); +// }); - 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, stdout } = await runInlineTest({ - 'a.test.ts': ` - import { test, expect } from '@playwright/test'; - let page; - test.${hook}(async ({ browser }) => { - page = await browser.newPage(); - await page.setContent('
A
'); - expect(page.locator('div')).toHaveText('A'); - }); - test('test ${hook}', async () => { - await expect(page.locator('div')).toBeVisible(); - }); - ` - }); +// 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('
A
'); +// const expectPromise = expect(page.locator('div')).toHaveText('A'); +// expect(expectPromise instanceof Promise).toBeTruthy(); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// }); - expect(exitCode).toBe(0); - expect(stdout).toContain(warningSnippet); - expect(stdout).toContain(`${group[0]}/${group[1]} hooks`); - }); - } - }); +// 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, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// let page; +// test.${hook}(async ({ browser }) => { +// page = await browser.newPage(); +// await page.setContent('
A
'); +// expect(page.locator('div')).toHaveText('A'); +// }); +// test('test ${hook}', async () => { +// await expect(page.locator('div')).toBeVisible(); +// }); +// ` +// }); - 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, stdout } = await runInlineTest({ - 'a.test.ts': ` - import { test, expect } from '@playwright/test'; - let page; - test('test ${hook}', async ({ browser }) => { - await expect(Promise.resolve()).resolves.toBe(undefined); - }); - test.${hook}(async () => { - expect(Promise.resolve()).resolves.toBe(undefined); - }); - ` - }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).toContain(warningSnippet); +// expect(warnings[0].description).toContain(`${group[0]}/${group[1]} hooks`); +// }); +// } +// }); - expect(exitCode).toBe(0); - expect(stdout).toContain(warningSnippet); - expect(stdout).toContain(`${group[0]}/${group[1]} hooks`); - }); - } - }); +// 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, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// let page; +// test('test ${hook}', async ({ browser }) => { +// await expect(Promise.resolve()).resolves.toBe(undefined); +// }); +// test.${hook}(async () => { +// expect(Promise.resolve()).resolves.toBe(undefined); +// }); +// ` +// }); - test('should warn about missing await across hooks and test', async ({ runInlineTest }) => { - const { exitCode, stdout } = await runInlineTest({ - 'a.test.ts': ` - import { test, expect } from '@playwright/test'; - test.beforeAll(async () => { - expect(Promise.resolve()).resolves.toBe(undefined); - }); - test('test', async () => { - expect(Promise.resolve()).resolves.toBe(undefined); - }); - test.afterEach(async () => { - expect(Promise.resolve()).resolves.toBe(undefined); - }); - ` - }); - expect(exitCode).toBe(0); - expect(stdout).toContain(`${warningSnippet} by the end of beforeAll/beforeEach hooks.`); - expect(stdout).toContain(`${warningSnippet} by the end of the test.`); - expect(stdout).toContain(`${warningSnippet} by the end of afterAll/afterEach hooks.`); - }); -}); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(1); +// expect(warnings[0].description).toContain(warningSnippet); +// expect(warnings[0].description).toContain(`${group[0]}/${group[1]} hooks`); +// }); +// } +// }); + +// test('should warn about missing await across hooks and test', async ({ runInlineTest }) => { +// const { exitCode, report } = await runInlineTest({ +// 'a.test.ts': ` +// import { test, expect } from '@playwright/test'; +// test.beforeAll(async () => { +// expect(Promise.resolve()).resolves.toBe(undefined); +// }); +// test('test', async () => { +// expect(Promise.resolve()).resolves.toBe(undefined); +// }); +// test.afterEach(async () => { +// expect(Promise.resolve()).resolves.toBe(undefined); +// }); +// ` +// }); +// expect(exitCode).toBe(0); +// const warnings = getWarnings(report); +// expect(warnings.length).toEqual(3); +// expect(warnings[0].description).toContain(`${warningSnippet} by the end of beforeAll/beforeEach hooks.`); +// expect(warnings[1].description).toContain(`${warningSnippet} by the end of the test.`); +// expect(warnings[2].description).toContain(`${warningSnippet} by the end of afterAll/afterEach hooks.`); +// }); +// });