Fix e2e tests: add proper health check for WordPress Playground#20
Open
Fix e2e tests: add proper health check for WordPress Playground#20
Conversation
Replace the unreliable 2-second fixed wait with a polling mechanism that verifies WordPress is actually ready to accept requests: - Poll /wp-admin/ endpoint up to 30 times (60 seconds max) - Check for HTTP 200 or 302 response - Log progress for debugging - Throw error if server doesn't start in time - Keep additional 1-second wait for plugin initialization
|
🤖 Hi @fellyph, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
📋 Review Summary
This pull request significantly improves the reliability of the e2e tests by replacing a fixed wait with a proper polling-based health check for the WordPress Playground server. The implementation is solid and includes good practices like logging and a clear timeout mechanism.
🔍 General Feedback
- The polling logic is well-implemented, correctly checking for both successful responses and redirects.
- The use of constants for
maxAttemptsanddelayMsis appreciated for readability and maintainability. - One minor suggestion was made to extract the hardcoded base URL into a constant to further improve maintainability.
Comment on lines
+60
to
+66
| const delayMs = 2000; | ||
| let attempts = 0; | ||
|
|
||
| while (attempts < maxAttempts) { | ||
| try { | ||
| const response = await fetch('http://127.0.0.1:9400/wp-admin/'); | ||
| if (response.ok || response.status === 302) { |
There was a problem hiding this comment.
🟡 It's a good practice to avoid hardcoding URLs and ports in test files. Defining the base URL as a constant at the top of the polling logic will improve maintainability and make it easier to update if the port or host changes.
Suggested change
| const delayMs = 2000; | |
| let attempts = 0; | |
| while (attempts < maxAttempts) { | |
| try { | |
| const response = await fetch('http://127.0.0.1:9400/wp-admin/'); | |
| if (response.ok || response.status === 302) { | |
| const BASE_URL = 'http://127.0.0.1:9400'; | |
| const maxAttempts = 30; | |
| const delayMs = 2000; | |
| let attempts = 0; | |
| while (attempts < maxAttempts) { | |
| try { | |
| const response = await fetch(`${BASE_URL}/wp-admin/`); |
- Pin Node.js to version 20 in playwright.yml to avoid compatibility issues with newer Node versions (v24 caused ErrnoError errno: 44) - Add try-catch around WordPress Playground initialization - Add fetch timeout with AbortController to prevent hanging requests - Log last error when health check times out for better debugging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the unreliable 2-second fixed wait with a polling mechanism that verifies WordPress is actually ready to accept requests: