Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Reusing page between tests when running serially adds extra teardown time per test #30635

Closed
david-olink opened this issue May 2, 2024 · 4 comments
Assignees
Labels

Comments

@david-olink
Copy link

Version

1.43.1

Steps to reproduce

Reusing page between tests when running serially, as described here https://playwright.dev/docs/test-retries#reuse-single-page-between-tests , adds extra time in in teardown for each test (even if the tests are empty).

Run this Test Suite:

import type { Page } from '@playwright/test';
import { test } from '@playwright/test';

// Based on the reuse-single-page-between-tests example:
// https://playwright.dev/docs/test-retries#reuse-single-page-between-tests

test.describe.configure({ mode: 'serial' });

let page: Page;

test.beforeAll(async ({ browser }) => {
  page = await browser.newPage();
});

test.afterAll(async () => {
  await page.close();
});

for (let i = 0; i < 5; i++) {
  test('Before: Empty test ' + i, async () => {});
}

test('First time we use the page object', async () => {
  await page.goto('/');
});

//
// Bug: These empty tests takes much more time than they should.
//

for (let i = 0; i < 5; i++) {
  test('After: Empty test (takes much more time) ' + i, async () => {});
}

Expected behavior

No test except the last should have a "After Hooks" that takes more than ~20ms instead of the ~700ms for the last tests as seen here:

image

Actual behavior

Running the above code adds extra "After Hooks" time as seen here:

image

Additional context

No response

Environment

System:
    OS: Windows 11 10.0.22631
    CPU: (12) x64 12th Gen Intel(R) Core(TM) i7-1255U
    Memory: 8.83 GB / 31.69 GB
  Binaries:
    Node: 20.11.1 - C:\Program Files\nodejs\node.EXE
    npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD
  IDEs:
    VSCode: 1.88.1 - C:\Users\user\AppData\Local\Programs\Microsoft VS Code\bin\code.CMD
  Languages:
    Bash: 5.2.15 - C:\Users\user\AppData\Local\Programs\Git\usr\bin\bash.EXE
@dgozman
Copy link
Contributor

dgozman commented May 2, 2024

@david-olink This is because you run tests in UI mode, which forces recording the trace, and tests after the first goto() have non-empty traces containing page resources to show you the snapshots. Why do you consider this to be a problem?

@david-olink
Copy link
Author

Thanks for the clarification of what is happening. I think it would be clearer if the "After Hooks" sections show that it is the trace that takes that long time.

  • Could it be possible to clarify that this is what the after hook does?
  • Could it maybe be possible to speed up the performance of the trace after hook?

I want speed up our testing as much as possible for our developers when running in UI mode, we also need the tests to be run serially. So that is why I want to remove any unnecessary overhead if possible :)

@dgozman
Copy link
Contributor

dgozman commented May 3, 2024

  • Could it be possible to clarify that this is what the after hook does?

We can look into this.

  • Could it maybe be possible to speed up the performance of the trace after hook?

We are always keen to improve Playwright's performance, but we do not have any specific ideas about tracing right now.

I want speed up our testing as much as possible for our developers when running in UI mode, we also need the tests to be run serially. So that is why I want to remove any unnecessary overhead if possible :)

Note that we strongly advise against the serial mode. Perhaps you just need to disable the parallelization with workers: 1? Making tests independent will improve everything, from reliability to performance.

dgozman added a commit that referenced this issue May 22, 2024
This includes two major changes:
- reuse `SerializedFS` for live test runner tracing;
- merge scheduled `appendFile` operations into a single `fs` call.

In some cases, this improves performance of UI mode by 61% and
performance of `trace: on` mode by 38%. Note that performance
improvement on the average test will not be as noticeable.

References #30875, #30635.
@dgozman
Copy link
Contributor

dgozman commented May 28, 2024

I'll tentatively close this given the improvements in #30946 that will be released in v1.45. If you encounter issues after upgrading to 1.45 once it's out, please file a new issue.

@dgozman dgozman closed this as completed May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants