-
Notifications
You must be signed in to change notification settings - Fork 447
[bugfix] merge playwright deploy and comment workflows #5298
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
Conversation
Combines pr-playwright-deploy.yaml and pr-playwright-comment.yaml into a single workflow to ensure proper dependency ordering. The comment job now waits for deployments to complete before generating comments with deployment URLs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
🎭 Playwright Test Results✅ All tests passed across all browsers! ⏰ Completed at: 09/02/2025, 04:05:03 AM UTC 📊 Test Reports by Browser🎉 Your tests are passing across all browsers! |
|
@christian-byrne |
seems still not working yet.... unlucky |
…links The issue in PR #5298 was caused by missing deployment-info artifact creation. The deploy-reports job was deploying to Cloudflare but wasn't creating the deployment-info-* artifacts that comment-tests-completed job expected to download. This change consolidates the deployment and commenting into a single job, eliminating the artifact dependency and ensuring links are always available. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…links The issue in PR #5298 was caused by missing deployment-info artifact creation. The deploy-reports job was deploying to Cloudflare but wasn't creating the deployment-info-* artifacts that comment-tests-completed job expected to download. This change consolidates the deployment and commenting into a single job, eliminating the artifact dependency and ensuring links are always available. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
* [fix] Consolidate Playwright workflow jobs to fix missing deployment links The issue in PR #5298 was caused by missing deployment-info artifact creation. The deploy-reports job was deploying to Cloudflare but wasn't creating the deployment-info-* artifacts that comment-tests-completed job expected to download. This change consolidates the deployment and commenting into a single job, eliminating the artifact dependency and ensuring links are always available. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Split PR deployment workflow for forked vs non-forked repos - Extract deployment logic to reusable script (scripts/cicd/pr-playwright-deploy-and-comment.sh) - Non-forked PRs: Use direct pull_request event in test-ui.yaml (faster) - Forked PRs: Use workflow_run in pr-playwright-deploy.yaml (handles permissions) - Add starting comment for both forked and non-forked PRs - Make Cloudflare tokens optional for starting status comments * refactor: Simplify PR deployment workflow and script - Consolidate workflow into single job with clearer structure - Reduce script from 200+ to ~140 lines - Simplify deployment retry logic and comment generation - Remove redundant checks and unnecessary complexity * fix: Add debugging and wrangler installation to deployment script - Add debug output to identify missing reports - Install wrangler if not available - Show deployment attempts and failures - Log available reports before deployment * chore: Trigger CI to test deployment workflow * fix: Fix browser artifact name mismatch in deployment script - Use dot notation (0.5x) for artifact names as Playwright creates them - Convert to dash notation (0-5x) for Cloudflare project names - Properly handle browser name display in comments * refactor: Convert deployment script to POSIX sh for better compatibility - Replace bash arrays with space-separated strings - Use while loops instead of bash-specific for syntax - Remove bash-specific string manipulation features - Replace local variables (not required in functions) - Ensure compatibility with standard /bin/sh * fix: Fix deployment script output to properly capture URLs - Redirect debug messages to stderr - Only output URL to stdout for proper capture - This fixes the missing deployment links in PR comments * fix: Add input validation to prevent command injection - Validate PR number is numeric only - Sanitize branch name at script start - Validate status parameter values - Use pre-sanitized branch throughout script - Addresses high-severity security issue from PR review * fix: Add null checks and logging to workflow condition - Add explicit null checks for head_repository and repository - Add debug logging to help diagnose workflow trigger issues - Prevents potential failures from undefined repository objects - Addresses medium-severity issue from PR review * fix: Pin wrangler to major version 4 with error handling - Pin wrangler to major version 4 (^4.0.0) for stability - Add error handling if wrangler installation fails - Return 'failed' status if installation fails - Addresses dependency management issue from PR review * perf: Implement parallel deployments to reduce CI time - Deploy all browser reports in parallel using background processes - Use temporary directory to collect deployment results - Wait for all deployments to complete before generating comment - Maintains result order for consistent output - Significantly reduces deployment time from sequential to parallel execution * fix: Use specific comment ID for updates instead of edit-last - Use GitHub API to find exact comment ID - Update specific comment by ID to avoid editing wrong comment - Prevents race conditions if user posts between finding and editing - More reliable comment updates * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * Update scripts/cicd/pr-playwright-deploy-and-comment.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(pr-playwright-deploy-and-comment.sh): improve regex for URL extraction to include valid characters and ensure correct URL format * chore(pr-playwright-deploy-and-comment.sh): move wrangler installation to the beginning of the script to avoid redundancy and improve efficiency --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* [fix] Consolidate Playwright workflow jobs to fix missing deployment links The issue in PR #5298 was caused by missing deployment-info artifact creation. The deploy-reports job was deploying to Cloudflare but wasn't creating the deployment-info-* artifacts that comment-tests-completed job expected to download. This change consolidates the deployment and commenting into a single job, eliminating the artifact dependency and ensuring links are always available. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Split PR deployment workflow for forked vs non-forked repos - Extract deployment logic to reusable script (scripts/cicd/pr-playwright-deploy-and-comment.sh) - Non-forked PRs: Use direct pull_request event in test-ui.yaml (faster) - Forked PRs: Use workflow_run in pr-playwright-deploy.yaml (handles permissions) - Add starting comment for both forked and non-forked PRs - Make Cloudflare tokens optional for starting status comments * refactor: Simplify PR deployment workflow and script - Consolidate workflow into single job with clearer structure - Reduce script from 200+ to ~140 lines - Simplify deployment retry logic and comment generation - Remove redundant checks and unnecessary complexity * fix: Add debugging and wrangler installation to deployment script - Add debug output to identify missing reports - Install wrangler if not available - Show deployment attempts and failures - Log available reports before deployment * chore: Trigger CI to test deployment workflow * fix: Fix browser artifact name mismatch in deployment script - Use dot notation (0.5x) for artifact names as Playwright creates them - Convert to dash notation (0-5x) for Cloudflare project names - Properly handle browser name display in comments * refactor: Convert deployment script to POSIX sh for better compatibility - Replace bash arrays with space-separated strings - Use while loops instead of bash-specific for syntax - Remove bash-specific string manipulation features - Replace local variables (not required in functions) - Ensure compatibility with standard /bin/sh * fix: Fix deployment script output to properly capture URLs - Redirect debug messages to stderr - Only output URL to stdout for proper capture - This fixes the missing deployment links in PR comments * fix: Add input validation to prevent command injection - Validate PR number is numeric only - Sanitize branch name at script start - Validate status parameter values - Use pre-sanitized branch throughout script - Addresses high-severity security issue from PR review * fix: Add null checks and logging to workflow condition - Add explicit null checks for head_repository and repository - Add debug logging to help diagnose workflow trigger issues - Prevents potential failures from undefined repository objects - Addresses medium-severity issue from PR review * fix: Pin wrangler to major version 4 with error handling - Pin wrangler to major version 4 (^4.0.0) for stability - Add error handling if wrangler installation fails - Return 'failed' status if installation fails - Addresses dependency management issue from PR review * perf: Implement parallel deployments to reduce CI time - Deploy all browser reports in parallel using background processes - Use temporary directory to collect deployment results - Wait for all deployments to complete before generating comment - Maintains result order for consistent output - Significantly reduces deployment time from sequential to parallel execution * fix: Use specific comment ID for updates instead of edit-last - Use GitHub API to find exact comment ID - Update specific comment by ID to avoid editing wrong comment - Prevents race conditions if user posts between finding and editing - More reliable comment updates * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * Update scripts/cicd/pr-playwright-deploy-and-comment.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(pr-playwright-deploy-and-comment.sh): improve regex for URL extraction to include valid characters and ensure correct URL format * chore(pr-playwright-deploy-and-comment.sh): move wrangler installation to the beginning of the script to avoid redundancy and improve efficiency --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* [fix] Consolidate Playwright workflow jobs to fix missing deployment links The issue in PR #5298 was caused by missing deployment-info artifact creation. The deploy-reports job was deploying to Cloudflare but wasn't creating the deployment-info-* artifacts that comment-tests-completed job expected to download. This change consolidates the deployment and commenting into a single job, eliminating the artifact dependency and ensuring links are always available. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Split PR deployment workflow for forked vs non-forked repos - Extract deployment logic to reusable script (scripts/cicd/pr-playwright-deploy-and-comment.sh) - Non-forked PRs: Use direct pull_request event in test-ui.yaml (faster) - Forked PRs: Use workflow_run in pr-playwright-deploy.yaml (handles permissions) - Add starting comment for both forked and non-forked PRs - Make Cloudflare tokens optional for starting status comments * refactor: Simplify PR deployment workflow and script - Consolidate workflow into single job with clearer structure - Reduce script from 200+ to ~140 lines - Simplify deployment retry logic and comment generation - Remove redundant checks and unnecessary complexity * fix: Add debugging and wrangler installation to deployment script - Add debug output to identify missing reports - Install wrangler if not available - Show deployment attempts and failures - Log available reports before deployment * chore: Trigger CI to test deployment workflow * fix: Fix browser artifact name mismatch in deployment script - Use dot notation (0.5x) for artifact names as Playwright creates them - Convert to dash notation (0-5x) for Cloudflare project names - Properly handle browser name display in comments * refactor: Convert deployment script to POSIX sh for better compatibility - Replace bash arrays with space-separated strings - Use while loops instead of bash-specific for syntax - Remove bash-specific string manipulation features - Replace local variables (not required in functions) - Ensure compatibility with standard /bin/sh * fix: Fix deployment script output to properly capture URLs - Redirect debug messages to stderr - Only output URL to stdout for proper capture - This fixes the missing deployment links in PR comments * fix: Add input validation to prevent command injection - Validate PR number is numeric only - Sanitize branch name at script start - Validate status parameter values - Use pre-sanitized branch throughout script - Addresses high-severity security issue from PR review * fix: Add null checks and logging to workflow condition - Add explicit null checks for head_repository and repository - Add debug logging to help diagnose workflow trigger issues - Prevents potential failures from undefined repository objects - Addresses medium-severity issue from PR review * fix: Pin wrangler to major version 4 with error handling - Pin wrangler to major version 4 (^4.0.0) for stability - Add error handling if wrangler installation fails - Return 'failed' status if installation fails - Addresses dependency management issue from PR review * perf: Implement parallel deployments to reduce CI time - Deploy all browser reports in parallel using background processes - Use temporary directory to collect deployment results - Wait for all deployments to complete before generating comment - Maintains result order for consistent output - Significantly reduces deployment time from sequential to parallel execution * fix: Use specific comment ID for updates instead of edit-last - Use GitHub API to find exact comment ID - Update specific comment by ID to avoid editing wrong comment - Prevents race conditions if user posts between finding and editing - More reliable comment updates * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * Update scripts/cicd/pr-playwright-deploy-and-comment.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(pr-playwright-deploy-and-comment.sh): improve regex for URL extraction to include valid characters and ensure correct URL format * chore(pr-playwright-deploy-and-comment.sh): move wrangler installation to the beginning of the script to avoid redundancy and improve efficiency --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Summary
Test plan
🤖 Generated with Claude Code
┆Issue is synchronized with this Notion page by Unito