From ab85b23e6770b1db456f15ede9727ea0b163166b Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 3 Apr 2023 15:06:13 -0700 Subject: [PATCH] fix(expect): report expect "Timed out" when it actually does (#22174) Previously, it would say "Timed out" when page was closed at test timeout, or not say "Timed out" when at least one element matched. Fixes #21664. --- packages/playwright-core/src/server/frames.ts | 3 +- tests/page/expect-timeout.spec.ts | 52 +++++++++++++++++++ tests/playwright-test/expect.spec.ts | 10 ++-- .../ui-mode-test-progress.spec.ts | 6 +-- tests/playwright-test/watch.spec.ts | 2 +- 5 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 tests/page/expect-timeout.spec.ts diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 85239919fe..c9c0afc904 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -41,6 +41,7 @@ import type { ScreenshotOptions } from './screenshotter'; import type { InputFilesItems } from './dom'; import { asLocator } from '../utils/isomorphic/locatorGenerators'; import { FrameSelectors } from './frameSelectors'; +import { TimeoutError } from '../common/errors'; type ContextData = { contextPromise: ManualPromise; @@ -1435,7 +1436,7 @@ export class Frame extends SdkObject { const result: { matches: boolean, received?: any, log?: string[], timedOut?: boolean } = { matches: options.isNot, log: metadata.log }; if (lastIntermediateResult.isSet) result.received = lastIntermediateResult.received; - else + if (e instanceof TimeoutError) result.timedOut = true; return result; }); diff --git a/tests/page/expect-timeout.spec.ts b/tests/page/expect-timeout.spec.ts new file mode 100644 index 0000000000..887a6583b6 --- /dev/null +++ b/tests/page/expect-timeout.spec.ts @@ -0,0 +1,52 @@ +/** + * 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 { stripAnsi } from '../config/utils'; +import { test, expect } from './pageTest'; + +test('should print timed out error message', async ({ page }) => { + await page.setContent('
Text content
'); + const error = await expect(page.locator('no-such-thing')).toHaveText('hey', { timeout: 1000 }).catch(e => e); + expect(stripAnsi(error.message)).toContain('Timed out 1000ms waiting for expect(received).toHaveText(expected)'); +}); + +test('should print timed out error message when value does not match', async ({ page }) => { + await page.setContent('
Text content
'); + const error = await expect(page.locator('div')).toHaveText('hey', { timeout: 1000 }).catch(e => e); + expect(stripAnsi(error.message)).toContain('Timed out 1000ms waiting for expect(received).toHaveText(expected)'); +}); + +test('should print timed out error message with impossible timeout', async ({ page }) => { + await page.setContent('
Text content
'); + const error = await expect(page.locator('no-such-thing')).toHaveText('hey', { timeout: 1 }).catch(e => e); + expect(stripAnsi(error.message)).toContain('Timed out 1ms waiting for expect(received).toHaveText(expected)'); +}); + +test('should print timed out error message when value does not match with impossible timeout', async ({ page }) => { + await page.setContent('
Text content
'); + const error = await expect(page.locator('div')).toHaveText('hey', { timeout: 1 }).catch(e => e); + expect(stripAnsi(error.message)).toContain('Timed out 1ms waiting for expect(received).toHaveText(expected)'); +}); + +test('should not print timed out error message when page closes', async ({ page }) => { + await page.setContent('
Text content
'); + const [error] = await Promise.all([ + expect(page.locator('div')).toHaveText('hey', { timeout: 100000 }).catch(e => e), + page.close(), + ]); + expect(stripAnsi(error.message)).toContain('expect.toHaveText with timeout 100000ms'); + expect(stripAnsi(error.message)).not.toContain('Timed out'); +}); diff --git a/tests/playwright-test/expect.spec.ts b/tests/playwright-test/expect.spec.ts index 61fcdf50d2..ad070efd4c 100644 --- a/tests/playwright-test/expect.spec.ts +++ b/tests/playwright-test/expect.spec.ts @@ -611,21 +611,23 @@ test('should print expected/received on Ctrl+C', async ({ runInlineTest }) => { expect(result.output).toContain('Received string: "Text content"'); }); -test('should print timed out error message', async ({ runInlineTest }) => { +test('should not print timed out error message when test times out', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.ts': ` import { test, expect } from '@playwright/test'; test('fail', async ({ page }) => { await page.setContent('
Text content
'); - await expect(page.locator('no-such-thing')).toHaveText('hey', { timeout: 1000 }); + await expect(page.locator('no-such-thing')).toHaveText('hey', { timeout: 5000 }); }); `, - }, { workers: 1 }); + }, { workers: 1, timeout: 3000 }); expect(result.failed).toBe(1); expect(result.exitCode).toBe(1); const output = result.output; - expect(output).toContain('Timed out 1000ms waiting for expect(received).toHaveText(expected)'); + expect(output).toContain('Test timeout of 3000ms exceeded'); + expect(output).not.toContain('Timed out 5000ms waiting for expect'); + expect(output).toContain('Error: expect(received).toHaveText(expected)'); }); test('should not leak long expect message strings', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/ui-mode-test-progress.spec.ts b/tests/playwright-test/ui-mode-test-progress.spec.ts index 03f9672486..2feb79af00 100644 --- a/tests/playwright-test/ui-mode-test-progress.spec.ts +++ b/tests/playwright-test/ui-mode-test-progress.spec.ts @@ -53,7 +53,7 @@ test('should update trace live', async ({ runUITest, server }) => { ).toHaveText([ /browserContext.newPage[\d.]+m?s/, /page.gotohttp:\/\/localhost:\d+\/one.html/ - ]); + ], { timeout: 15000 }); await expect( listItem.locator(':scope.selected'), @@ -138,7 +138,7 @@ test('should preserve action list selection upon live trace update', async ({ ru /browserContext.newPage[\d.]+m?s/, /page.gotoabout:blank[\d.]+m?s/, /page.setContent[\d.]+m?s/, - ]); + ], { timeout: 15000 }); // Manually select page.goto. await page.getByTestId('action-list').getByText('page.goto').click(); @@ -199,7 +199,7 @@ test('should update tracing network live', async ({ runUITest, server }) => { /browserContext.newPage[\d.]+m?s/, /page.gotohttp:\/\/localhost:\d+\/one.html[\d.]+m?s/, /page.setContent[\d.]+m?s/, - ]); + ], { timeout: 15000 }); // Once page.setContent is visible, we can be sure that page.goto has all required // resources in the trace. Switch to it and check that everything renders. diff --git a/tests/playwright-test/watch.spec.ts b/tests/playwright-test/watch.spec.ts index 985774fc70..6cf7115d71 100644 --- a/tests/playwright-test/watch.spec.ts +++ b/tests/playwright-test/watch.spec.ts @@ -602,7 +602,7 @@ test('should run CT on changed deps', async ({ runWatchTest, writeFiles }) => { await testProcess.waitForOutput(`src${path.sep}button.spec.tsx:4:11 › pass`); expect(testProcess.output).not.toContain(`src${path.sep}link.spec.tsx`); - await testProcess.waitForOutput('Error: expect(received).toHaveText(expected)'); + await testProcess.waitForOutput('Error: Timed out 1000ms waiting for expect(received).toHaveText(expected)'); await testProcess.waitForOutput('Waiting for file changes.'); });