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

[Feature]: An option to NOT shutdown worker after test failure #32803

Open
vitalets opened this issue Sep 25, 2024 · 8 comments · May be fixed by #34891
Open

[Feature]: An option to NOT shutdown worker after test failure #32803

vitalets opened this issue Sep 25, 2024 · 8 comments · May be fixed by #34891

Comments

@vitalets
Copy link
Contributor

vitalets commented Sep 25, 2024

🚀 Feature Request

Currently Playwright always creates a new worker if test fails:

Workers are always shutdown after a test failure to guarantee pristine environment for following tests.

Although such behavior is quite reasonable, it has some downsides:

In most of my tests, I don't store any state in the worker. I just use test scoped fixtures that are re-created for every test anyway. For such cases, an option to re-use worker would significantly improve the performance.

Strictly speaking, current approach does not guarantee 100% pristine environment even for passed tests. In the following example test 2 fails because test 1 changes global state of regexp:

const titleRe = /end-to-end testing/g;

test('test 1', async ({ page }) => {
  await page.goto('https://playwright.dev');
  const title = await page.title();
  expect(titleRe.test(title)).toEqual(true);
});

test('test 2', async ({ page }) => {
  await page.goto('https://playwright.dev');
  const title = await page.title();
  expect(titleRe.test(title)).toEqual(true);
});

Example

I propose to add an option recreateWorker that can hold one of two values:

  • on-failure (current behavior, default)
  • never (not shutdown worker on failure)

For example:

export default defineConfig({
  use: {
    recreateWorker: 'on-failure', // or 'never' 
  },

Motivation

When tests don't store any state in worker, having an option to re-use worker after failures with greatly improve performance.

@matt-kxs
Copy link

matt-kxs commented Dec 23, 2024

Is there any workaround for this? I already put in work to get beforeAll / afterAll to execute once per file on first worker only, but was shocked to discover that the worker is destroyed on failure. This makes porting to playwright near impossible unless we disable parallelism. And even then it would have terrible performance on test failure since each test failure would run beforeAll / afterAll when really it only needs to run once - a failure shouldn't affect the state of beforeAll so it seems redundant to run it again.

@vitalets
Copy link
Contributor Author

vitalets commented Dec 23, 2024

Is there any workaround for this?

This is a workaround I've shared in another issue: #22520 (comment)
It allows to run code once across workers. Suitable for beforeAll hooks, where we typically interested in the first invocation.
The approach does not work for afterAll logic, where we interested in the last invocation. It requires more difficult trick.

@matt-kxs
Copy link

Is there any workaround for this?

This is a workaround I've shared in another issue: #22520 (comment) It allows to run code once across workers. Suitable for beforeAll hooks, where we typically interested in the first invocation. The approach does not work for afterAll logic, where we interested in the last invocation. It requires more difficult trick.

Thanks for this! The .afterAll is the tricky part I can't get working well with the destroying worker on failure case. For now I've settled on the following:

  1. Use my existing structure where I've forced .beforeAll / .afterAll to work like it does in most other frameworks where it only executes once per file
  2. I have a command line runner that I've modified to run one file at a time and will run with multiple workers, retry=0, maxFailures = 1.
  3. If the file has test failures, re-run but with workers=1, retry=2, maxFailures=100

So when things are good we get the full benefits of parallelism but when things fail, fall back to how playwright wants things to work (losing all the speed benefits unfortunately).

@Kartheeswari-git
Copy link

Can this be fixed in version 1.51? We're unable to proceed with end-to-end automation for our products due to this blocker. @dgozman

@ztesterparadise2
Copy link

Please, inform on the decision in respect of the issue. It really is a problem for test automation on our project

@alexsch01
Copy link

I made a PR to fix this issue, please let me know any feedback you may have

@ztesterparadise2
Copy link

I made a PR to fix this issue, please let me know any feedback you may have

Tried your changes, the worker is indeed recreated - browser does not close on failure. Which is awesome news! Thanks!

@alexsch01
Copy link

Tried your changes, the worker is indeed recreated - browser does not close on failure. Which is awesome news! Thanks!

Great to hear! Actually in your case that's the worker not being recreated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants