Skip to content

Commit

Permalink
Merge pull request #905 from carstingaxion/fix/screenshots-tests-race…
Browse files Browse the repository at this point in the history 
…-condition

Fix race-condition during Playground setup and Playwright start
  • Loading branch information
@carstingaxion
carstingaxion authored Sep 24, 2024
2 parents cb5622c + 01dffe0 commit 9b52ca4
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 19 deletions.
23 changes: 21 additions & 2 deletions .github/scripts/wordpress-org-screenshots/playwright.config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,33 @@ export default defineConfig({
reportSlowTests: null,
use: {
...baseConfig.use,
// Gutenberg uses wp-env by default for Playwright tests,
// which runs on a different port.
// Playground runs on port: 9400.
baseURL: process.env.WP_BASE_URL || 'http://127.0.0.1:9400',
},
retries: 0,
webServer: {
...baseConfig.webServer,
command: 'npm run playground -- --blueprint=./localized_blueprint.json',
// This is kind of a hack PART 2/2,
// to make sure Playwright DOES NOT start the webserver on its own.
//
// Part 1/2 is the "run" of the "Running (& Updating) the screenshot tests" steps
// in .github/workflows/wordpress-org-screenshots.yml
//
// While auto-loading the webserver when needed sounded nice, it introduced a race-condition
// between the setup of Playground and Playwrights own start event.
// Playwright listens for the availability of the webserver relatively simple,
// as soon as there is a status code 200, Playwright starts all engines.
//
// Unfortunately Playground is not ready at this point, it hast started WordPress
// and is going to start stepping through the blueprint, but hasn't loaded GatherPress nor imported any data;
// Resulting in failing tests.
//
// Sending just >undefined< is an idea, taken from @swisspidy at:
// https://github.com/swissspidy/wp-performance-action/pull/173/files#diff-980717ce45eb5ef0a66e87dd5b664656800d31ca809fe902f069b5e8f3cdcd31
command: undefined,
port: 9400,
// reuseExistingServer: !process.env.CI,
reuseExistingServer: true,
},
});
55 changes: 38 additions & 17 deletions .github/workflows/wordpress-org-screenshots.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ jobs:
matrix:
# Use all locales with more than 90% complete translations on https://translate.wordpress.org/projects/wp-plugins/gatherpress/
locale: [
'en_US',
'fr_FR',
'de_DE',
'es_ES',
'mr',
'nl_BE',
'it_IT'
'it_IT',
'en_US'
]

steps:
Expand Down Expand Up @@ -111,16 +111,6 @@ jobs:
jq --argjson languageStep "$language_step" '.steps += $languageStep' .github/scripts/wordpress-org-screenshots/blueprint.json > localized_blueprint.json
fi
- name: Starting Playground
# Having this as a separate workflow step, should help making sure Playwright runs only, when this is DONE & READY.
#
# Because it seems to be a problem to "wait on webServer.command" https://github.com/microsoft/playwright/issues/11811
# & "it seems that globalSetup runs before webServer is started." https://github.com/microsoft/playwright/issues/11811#issuecomment-1040732201
#
# The "&" is important to allow the next step to start!
run: |
npm run playground -- --blueprint=./localized_blueprint.json &
- name: Running the screenshot tests
id: screenshot-tests
# Having set "continue-on-error:true" should help the workflow to pass (and get a green checkmark)
Expand All @@ -129,9 +119,35 @@ jobs:
if: ${{ ! github.event.inputs.updateAllSnapshots }}
env:
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1
WP_BASE_URL: 'http://127.0.0.1:9400/'
# This is kind of a hack PART 1/2,
# to make sure Playwright DOES NOT start the webserver on its own.
#
# Part 2/2 is the "command: undefined," declaration
# in .github/scripts/wordpress-org-screenshots/playwright.config.ts
#
# While auto-loading the webserver when needed sounded nice, it introduced a race-condition
# between the setup of Playground and Playwrights own start event.
# Playwright listens for the availability of the webserver relatively simple,
# as soon as there is a status code 200, Playwright starts all engines.
#
# Unfortunately Playground is not ready at this point, it hast started WordPress
# and is going to start stepping through the blueprint, but hasn't loaded GatherPress nor imported any data;
# Resulting in failing tests.
#
# It was not possible (for me) to keep the setup of Playground in a separate step,
# why this "run > sleep > run" became necessary.
# The setup process usually takes about 20sec, so 60 is just a good extra, to not run into errors.
#
# The sleep step, should help making sure Playwright runs only, when this is DONE & READY.
#
# Because it seems to be a problem to "wait on webServer.command" https://github.com/microsoft/playwright/issues/11811
# & "it seems that globalSetup runs before webServer is started." https://github.com/microsoft/playwright/issues/11811#issuecomment-1040732201
run: |
DEBUG=pw:webserver \
npm run playground -- --blueprint=./localized_blueprint.json & \
sleep 60 && \
echo 'Playground is ready now, lets take some pictures.' && \
WP_BASE_URL='http://127.0.0.1:9400/' \
DEBUG=pw:api,pw:webserver \
npm run screenshots:wporg
- name: Updating the Screenshots
Expand All @@ -143,10 +159,15 @@ jobs:
if: ${{ github.event.inputs.updateAllSnapshots || steps.screenshot-tests.outcome == 'failure' }}
env:
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1
WP_BASE_URL: 'http://127.0.0.1:9400/'
UPDATE_ALL_SNAPSHOTS: ${{ github.event.inputs.updateAllSnapshots }}
UPDATE_ALL_SNAPSHOTS: ${{ github.event.inputs.updateAllSnapshots }}
# Important Documentation about this "run"
# can be found on the "screenshot-tests" step!
run: |
DEBUG=pw:webserver \
npm run playground -- --blueprint=./localized_blueprint.json & \
sleep 60 && \
echo 'Playground is ready now, lets take some pictures.' && \
WP_BASE_URL='http://127.0.0.1:9400/' \
DEBUG=pw:api,pw:webserver \
npm run screenshots:wporg -- --update-snapshots
- name: Commit updated screenshots
Expand Down

0 comments on commit 9b52ca4

Please sign in to comment.