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

Fix broken Playwright tests #1819

Merged
merged 6 commits into from
Sep 28, 2024
Merged

Fix broken Playwright tests #1819

merged 6 commits into from
Sep 28, 2024

Conversation

brandonpayton
Copy link
Member

@brandonpayton brandonpayton commented Sep 28, 2024

Motivation for the change, related issues

Some Playwright tests were broken by last-minute changes to #1731.

Fixes #1818

Implementation details

  • Fix selection of "edit settings" and "save settings" buttons to select "Create a similar Playground" and "Create" buttons instead.
  • Change playground viewport selection to account for there being multiple possible viewports now. We were using an ID, but now we're using a CSS class.
  • Some expect() calls were failing, at least locally, and that was fixed by awaiting them. They need to be async.

Testing Instructions (or ideally a Blueprint)

  • CI

@brandonpayton brandonpayton added [Type] Bug An existing feature does not function as intended Tests labels Sep 28, 2024
@brandonpayton brandonpayton self-assigned this Sep 28, 2024
@brandonpayton
Copy link
Member Author

The Blueprint tests are still failing, and I'm looking into it.

@brandonpayton brandonpayton requested a review from a team September 28, 2024 04:58
@brandonpayton
Copy link
Member Author

Actually, the Blueprint tests were fine but anything related to WP multisite fails when the local server has a non-standard port (5400 in our case).

@@ -13,7 +13,7 @@ export const playwrightConfig: PlaywrightTestConfig = {
/* Retry on CI only */
retries: process.env.CI ? 2 : 0,
/* Opt out of parallel tests on CI. */
workers: 1,
workers: process.env.CI ? 1 : 3,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgrgicak, why do we opt out of parallel tests on CI, and should we continue opting out of parallel tests when running locally?

I enabled more workers to speed local testing, but we can change it back if needed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were occasionally failing locally and this made them more robust.
I just went with disabling parallel tests to get it out quicker. We can test it now with multiple workers and see how well it works.

@brandonpayton brandonpayton marked this pull request as ready for review September 28, 2024 05:16
@brandonpayton
Copy link
Member Author

Let's merge this so CI tests complete again and make any adjustments in follow-up PRs.

@brandonpayton brandonpayton merged commit c133c6c into trunk Sep 28, 2024
6 checks passed
@brandonpayton brandonpayton deleted the fix-playwright-tests branch September 28, 2024 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tests [Type] Bug An existing feature does not function as intended
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

E2E: Playwright tests are currently timing out
2 participants