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] Browser context is closing too long #5051

Closed
viraxslot opened this issue Jan 19, 2021 · 4 comments · Fixed by #5333
Closed

[BUG] Browser context is closing too long #5051

viraxslot opened this issue Jan 19, 2021 · 4 comments · Fixed by #5333
Assignees

Comments

@viraxslot
Copy link

viraxslot commented Jan 19, 2021

Context:

  • Playwright Version: ^1.8.0-next-1611032356000
  • Operating System: MacOS 10.15.7
  • Node.js version: v12.18.3
  • Browser: Chromium

Describe the bug
Hi, guys! I’ve added video recording to my tests as described in the documentation. Between tests I want to close all contexts except one, because I need sometimes 2, sometimes 1 context (I use approach 1 context = 1 page, because it’s easier to maintain for my application).

So, the problem is: when I’m closing context after 2-contexts test I see:

  pw:api => browserContext.close started +1ms
  pw:api <= browserContext.close succeeded +51s

It takes 50-60 seconds, so it's too long. I have a lot of such "switches".
I can provide additional logs if you tell me what to do. Could you please help? Maybe I'm doing something wrong.

Thanks in advance!

Code Snippet

// how I create context and page
let parameters = {};
if (config.get('recordFailedTestVideo')) {
    parameters = {
        recordVideo: {
            dir: config.get('videosDir'),
        },
    };
}

const ctx = await this._instance.newContext(parameters);
this._contexts.push({
    ctx: ctx,
    occupied: true,
});

const page = await ctx.newPage();
page.setDefaultTimeout(Timeouts.FifteenSecondsTimeout);
this._pages.push({
    page: page,
    occupied: true,
});

// how I close contexts
for (let i = 1; i < this._contexts.length; i++) {
      await this._contexts[i].ctx.close(); // here is the problem
      this._contexts.splice(i, 1);
      this._pages.splice(i, 1);
}

Update: here is repo to reproduce: https://github.com/viraxslot/close-context-problem.git
Hope it'll help.

@LanderBeeuwsaert
Copy link

In essence, it's what this item is about as well :):
#4821

@pavelfeldman
Copy link
Member

This one is a regression from 1.7.

Note that you should never recycle contexts, discard all of them after each test.

@pavelfeldman
Copy link
Member

pavelfeldman commented Jan 27, 2021

Not seeing this as a regression:

v1.7.1

    ✓ 1 create and close context without video recording (2355ms)
    ✓ 2 create and close context WITH video recording (4393ms)
    ✓ 3 create and close context WITH video recording (6687ms)
    ✓ 4 create and close context without video recording (860ms) -- sometimes fails as a timeout due to a flaky test

v1.8.0

    ✓ 1 create and close context without video recording (2159ms)
    ✓ 2 create and close context WITH video recording (3887ms)
    ✓ 3 create and close context WITH video recording (6037ms)
    ✓ 4 create and close context without video recording (705ms) -- sometimes fails as a timeout due to a flaky test

@pavelfeldman
Copy link
Member

I think the issue is in your test - you don't close the contexts after each test and as a result, closing all the contexts in the end of the suite. This results in a minute of the video that needs to be processed. You should close context after each test. We do that in http://github.com/microsoft/playwright-test, maybe it works for you.

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

Successfully merging a pull request may close this issue.

4 participants