Skip to content

Infrastructure/11912 playwright setup#12229

Merged
aaemnnosttv merged 82 commits intodevelopfrom
infrastructure/11912-playwright-setup
Mar 16, 2026
Merged

Infrastructure/11912 playwright setup#12229
aaemnnosttv merged 82 commits intodevelopfrom
infrastructure/11912-playwright-setup

Conversation

@eugene-manuilov
Copy link
Copy Markdown
Collaborator

@eugene-manuilov eugene-manuilov commented Feb 25, 2026

Summary

Addresses issue:

Relevant technical choices

  • Added four new reusable GitHub Actions:
    • upload-to-gcs — uploads files to GCS and generates public URLs
    • remove-gcs-path — cleans up GCS artifacts on PR close
    • update-pr-comment — manages a single consolidated PR comment with named sections (used by Playwright, Storybook, and Zips workflows)
    • build-push-docker-image — builds and pushes Docker images with registry caching
  • Refactored github workflows to use new composite actions to avoid having duplicate steps.
  • Replaced two separate workflows (publish-codegen-docker-image.yml, publish-vrt-docker-image.yml) with a single publish-docker-images.yml using the new reusable build-push-docker-image action.

PR Author Checklist

  • My code is tested and passes existing unit tests.
  • My code has an appropriate set of unit tests which all pass.
  • My code is backward-compatible with WordPress 5.2 and PHP 7.4.
  • My code follows the WordPress coding standards.
  • My code has proper inline documentation.
  • I have added a QA Brief on the issue linked above.
  • I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

  • Run the code.
  • Ensure the acceptance criteria are satisfied.
  • Reassess the implementation with the IB.
  • Ensure no unrelated changes are included.
  • Ensure CI checks pass.
  • Check Storybook where applicable.
  • Ensure there is a QA Brief.
  • Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

  • Ensure the PR has the correct target branch.
  • Double-check that the PR is okay to be merged.
  • Ensure the corresponding issue has a ZenHub release assigned.
  • Add a changelog message to the issue.

Comment thread .github/workflows/playwright.yml Fixed
@eugene-manuilov eugene-manuilov marked this pull request as ready for review February 26, 2026 23:08
Comment thread .github/workflows/playwright.yml Fixed
Copy link
Copy Markdown
Collaborator

@aaemnnosttv aaemnnosttv left a comment

Choose a reason for hiding this comment

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

Thanks @eugene-manuilov , this is nearly there I think, just a few things left to work through.

Comment thread .github/actions/remove-gcs-path/action.yml
@@ -0,0 +1,62 @@
services:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like we're missing the image for monitoring the debug log? This is important to keep.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added using a slightly different approach.

Copy link
Copy Markdown
Collaborator

@aaemnnosttv aaemnnosttv left a comment

Choose a reason for hiding this comment

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

Thanks @eugene-manuilov , just a few small things left to address then this should be good to go.

Comment thread .github/workflows/playwright.yml Outdated
tags: ghcr.io/google/site-kit-wp/playwright-wp:5.2.21
build-args: WP_VERSION=5.2.21
labels: org.opencontainers.image.description=WordPress 5.2.21 Docker image for Site Kit Playwright tests.
registry-password: ${{ secrets.GH_BOT_TOKEN }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I realize this is what we do now, but this will fail to create/publish a new package in GHCR. More context on this here #11069 (comment)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should we use just GITHUB_TOKEN?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sure – we may need to use gh actor for the username then too, I'm not sure, but we need to use the default token secret initially. We can change it later if it's a problem.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok, updated to be GITHUB_TOKEN and github-actions[bot].

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it may require using github.actor as in the example but we'll find out pretty quick.

const value = phpSerializeStringArray( plugins );
await this.db.execute(
'UPDATE wp_options SET option_value = ? WHERE option_name = "active_plugins"',
[ value ]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this need to be escaped?

Copy link
Copy Markdown
Collaborator Author

@eugene-manuilov eugene-manuilov Mar 13, 2026

Choose a reason for hiding this comment

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

Nope, this is just playwright tests; there is no way for SQL injection.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't mean regarding a malicious injection, but serialized PHP could have characters that break the statement? That's all. Not an issue?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, not an issue, at least for now. If we encounter any problem there, then we will fix it.

Comment thread tests/playwright/playwright.config.ts
Comment thread tests/playwright/README.md
Comment thread tests/playwright/README.md
Copy link
Copy Markdown
Collaborator

@aaemnnosttv aaemnnosttv left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @eugene-manuilov 🚀

@aaemnnosttv aaemnnosttv merged commit ad15d80 into develop Mar 16, 2026
37 of 39 checks passed
@aaemnnosttv aaemnnosttv deleted the infrastructure/11912-playwright-setup branch March 16, 2026 20:22
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.

3 participants