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

Support multiple Actors. #12

Merged
merged 1 commit into from
May 15, 2024
Merged

Support multiple Actors. #12

merged 1 commit into from
May 15, 2024

Conversation

perrygoy
Copy link
Member

While i was writing a test that has two Actors attempt to get on a call together, i noticed only one of the Actors was successfully tearing down and taking screenshots. The second Actor would get an error from Playwright, saying that the Playwright instance was already closed.

Well of course it was, that first Actor would do self.playwright.close() when their BrowseTheWebSynchronously ability was forgotten!

This PR takes out that call. Now that i'm more familiar with how Playwright works, keeping the Playwright instance active makes a lot more sense, especially in the context of the pytest fixtures Playwright provides in their plugin.

@perrygoy perrygoy requested a review from a team April 30, 2024 19:29
@MarcelWilson
Copy link

MarcelWilson commented May 7, 2024

I'm concerned about parity with screenpy_selenium. As a tester, I would expect BrowseTheWeb.forget to behave similar to BrowseTheWebSynchronously.forget.

I guess my question is; is playwright.close() on par with selenium driver.quit()?

In addition to this, if BrowseTheWebSynchronously.using_chromium() calls playwright.start() I would expect forget to call playwright.close(). At the same time I would also expect that if the caller already did the initial playwright initialization, then forget shouldn't bother shutting it down.

Does this need to be a bit more intelligent about determining what should happen during forget?

  • If BrowseTheWebSynchronously calls start(), forget should call close()
  • If BrowseTheWebSynchronously does not call start(), forget should NOT call close()

@perrygoy
Copy link
Member Author

perrygoy commented May 7, 2024

I guess my question is; is playwright.close() on par with selenium driver.quit()?

Shortly, no. playwright.close() is similar to shutting down the SeleniumServer, while browser.close is the Playwright version of driver.quit. I don't know if that parallel is 100% perfect, but if you shut down playwright that closes all the browsers and browser contexts it spawned, so all other active Actors will no longer be able to perform ScreenPy: Playwright Actions.

@perrygoy perrygoy merged commit a59f20c into trunk May 15, 2024
23 checks passed
@perrygoy perrygoy deleted the support-multiple-actors branch May 15, 2024 18:15
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 this pull request may close these issues.

2 participants