Infrastructure/11912 playwright setup#12229
Conversation
…e wordpress helper.
…rd with proxy and GCP authentication.
…12-playwright-setup.
…12-playwright-setup.
…ts with new helpers.
…trigger conditions, and add `actions: read` permission.
… version in Playwright tests.
…12-playwright-setup.
… uploading it to artifacts.
…spect` for checking image existence.
…12-playwright-setup.
…setup' into infrastructure/11740-playwright-email-reporting.
…40-playwright-email-reporting.
aaemnnosttv
left a comment
There was a problem hiding this comment.
Thanks @eugene-manuilov , this is nearly there I think, just a few things left to work through.
| @@ -0,0 +1,62 @@ | |||
| services: | |||
There was a problem hiding this comment.
It looks like we're missing the image for monitoring the debug log? This is important to keep.
There was a problem hiding this comment.
Added using a slightly different approach.
…12-playwright-setup.
…p and update related configurations.
…o ensure it's not empty and has at least three segments.
…setup' into infrastructure/11740-playwright-email-reporting.
aaemnnosttv
left a comment
There was a problem hiding this comment.
Thanks @eugene-manuilov , just a few small things left to address then this should be good to go.
| 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 }} |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Should we use just GITHUB_TOKEN?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ok, updated to be GITHUB_TOKEN and github-actions[bot].
There was a problem hiding this comment.
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 ] |
There was a problem hiding this comment.
Does this need to be escaped?
There was a problem hiding this comment.
Nope, this is just playwright tests; there is no way for SQL injection.
There was a problem hiding this comment.
I don't mean regarding a malicious injection, but serialized PHP could have characters that break the statement? That's all. Not an issue?
There was a problem hiding this comment.
Yes, not an issue, at least for now. If we encounter any problem there, then we will fix it.
Co-authored-by: Evan Mattson <emattson@google.com>
…12-playwright-setup.
…ogle/site-kit-wp into infrastructure/11912-playwright-setup.
…s[bot]` for Docker registry username.
aaemnnosttv
left a comment
There was a problem hiding this comment.
LGTM, thanks @eugene-manuilov 🚀
Summary
Addresses issue:
Relevant technical choices
upload-to-gcs— uploads files to GCS and generates public URLsremove-gcs-path— cleans up GCS artifacts on PR closeupdate-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 cachingpublish-codegen-docker-image.yml,publish-vrt-docker-image.yml) with a singlepublish-docker-images.ymlusing the new reusablebuild-push-docker-imageaction.PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist