From 97f7c2149a0a459c500b363d636a1c01b4921012 Mon Sep 17 00:00:00 2001 From: Johnpaul Chiwetelu <49923152+Myestery@users.noreply.github.com> Date: Thu, 23 Oct 2025 06:22:44 +0100 Subject: [PATCH] Shard Update Test Expectations PR (#6100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pull request significantly refactors the Playwright expectations update workflow to improve reliability, efficiency, and maintainability. The workflow is now split into three coordinated jobs—setup, sharded snapshot updates, and merge/commit—enabling parallel test execution and artifact management. Key improvements include sharding Playwright snapshot updates, robust caching and artifact handling, and more reliable PR context handling. **Workflow Restructuring and Sharding:** * The workflow is split into three jobs: `setup` (prepares environment and caches it), `update-snapshots-sharded` (runs Playwright snapshot updates in four parallel shards), and `merge-and-commit` (merges results and commits updates). This enables faster, more reliable snapshot updates. [[1]](diffhunk://#diff-0289f4b5962314fa2d58937651c3d2a0f2c6f76e26c95d6a04d43c18b3449917L15-R15) [[2]](diffhunk://#diff-0289f4b5962314fa2d58937651c3d2a0f2c6f76e26c95d6a04d43c18b3449917R27-R175) **Caching and Artifact Management:** * The setup job builds and caches the entire workspace, which is then restored by each shard for consistent environments. Each shard uploads its updated snapshots and test reports as artifacts, which are later downloaded and merged in the final job. **Improved PR Context Handling:** * PR number, branch, and comment IDs are now reliably extracted and passed between jobs using outputs, ensuring correct association with the PR throughout the workflow (e.g., for commenting, reactions, and pushing updates). [[1]](diffhunk://#diff-0289f4b5962314fa2d58937651c3d2a0f2c6f76e26c95d6a04d43c18b3449917R27-R175) [[2]](diffhunk://#diff-0289f4b5962314fa2d58937651c3d2a0f2c6f76e26c95d6a04d43c18b3449917L92-R199) **Job and Step Renaming/Cleanup:** * The main job is renamed from `test` to `setup`, and redundant or unnecessary steps (such as the old branch SHA extraction) are removed for clarity and maintainability. [[1]](diffhunk://#diff-0289f4b5962314fa2d58937651c3d2a0f2c6f76e26c95d6a04d43c18b3449917L15-R15) [[2]](diffhunk://#diff-0289f4b5962314fa2d58937651c3d2a0f2c6f76e26c95d6a04d43c18b3449917R27-R175) **Comment and Label Automation Improvements:** * Automated GitHub comment reactions and label removals now use the correct PR context, ensuring that feedback and status updates are reliably posted to the right place. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6100-Shard-Update-Test-Expectations-PR-28f6d73d36508109bcd8d382c942d44d) by [Unito](https://www.unito.io) --------- Co-authored-by: sno --- .../pr-update-playwright-expectations.yaml | 195 +++++++++++++++--- 1 file changed, 168 insertions(+), 27 deletions(-) diff --git a/.github/workflows/pr-update-playwright-expectations.yaml b/.github/workflows/pr-update-playwright-expectations.yaml index f688c3250..8d82bd675 100644 --- a/.github/workflows/pr-update-playwright-expectations.yaml +++ b/.github/workflows/pr-update-playwright-expectations.yaml @@ -12,11 +12,11 @@ concurrency: cancel-in-progress: true jobs: - test: + setup: runs-on: ubuntu-latest if: > ( github.event_name == 'pull_request' && github.event.label.name == 'New Browser Test Expectations' ) || - ( github.event.issue.pull_request && + ( github.event.issue.pull_request && github.event_name == 'issue_comment' && ( github.event.comment.author_association == 'OWNER' || @@ -24,12 +24,25 @@ jobs: github.event.comment.author_association == 'COLLABORATOR' ) && startsWith(github.event.comment.body, '/update-playwright') ) + outputs: + cache-key: ${{ steps.cache-key.outputs.key }} + pr-number: ${{ steps.pr-info.outputs.pr-number }} + branch: ${{ steps.pr-info.outputs.branch }} + comment-id: ${{ steps.find-update-comment.outputs.comment-id }} steps: + - name: Get PR info + id: pr-info + run: | + echo "pr-number=${{ github.event.number || github.event.issue.number }}" >> $GITHUB_OUTPUT + echo "branch=$(gh pr view ${{ github.event.number || github.event.issue.number }} --repo ${{ github.repository }} --json headRefName --jq '.headRefName')" >> $GITHUB_OUTPUT + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Find Update Comment uses: peter-evans/find-comment@b30e6a3c0ed37e7c023ccd3f1db5c6c0b0c23aad id: "find-update-comment" with: - issue-number: ${{ github.event.number || github.event.issue.number }} + issue-number: ${{ steps.pr-info.outputs.pr-number }} comment-author: "github-actions[bot]" body-includes: "Updating Playwright Expectations" @@ -37,49 +50,177 @@ jobs: uses: peter-evans/create-or-update-comment@e8674b075228eee787fea43ef493e45ece1004c9 with: comment-id: ${{ steps.find-update-comment.outputs.comment-id }} - issue-number: ${{ github.event.number || github.event.issue.number }} + issue-number: ${{ steps.pr-info.outputs.pr-number }} body: | Updating Playwright Expectations edit-mode: replace reactions: eyes - - name: Get Branch SHA - id: "get-branch" - run: echo ::set-output name=branch::$(gh pr view $PR_NO --repo $REPO --json headRefName --jq '.headRefName') - env: - REPO: ${{ github.repository }} - PR_NO: ${{ github.event.number || github.event.issue.number }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Initial Checkout + - name: Checkout repository uses: actions/checkout@v5 with: - ref: ${{ steps.get-branch.outputs.branch }} - - name: Setup Frontend + ref: ${{ steps.pr-info.outputs.branch }} + - name: Setup frontend uses: ./.github/actions/setup-frontend with: include_build_step: true - - name: Setup ComfyUI Server + # Save expensive build artifacts (Python env, built frontend, node_modules) + # Source code will be checked out fresh in sharded jobs + - name: Generate cache key + id: cache-key + run: echo "key=$(date +%s)" >> $GITHUB_OUTPUT + - name: Save cache + uses: actions/cache/save@5a3ec84eff668545956fd18022155c47e93e2684 + with: + path: | + ComfyUI + dist + key: comfyui-setup-${{ steps.cache-key.outputs.key }} + + # Sharded snapshot updates + update-snapshots-sharded: + needs: setup + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + shardIndex: [1, 2, 3, 4] + shardTotal: [4] + steps: + # Checkout source code fresh (not cached) + - name: Checkout repository + uses: actions/checkout@v5 + with: + ref: ${{ needs.setup.outputs.branch }} + + # Restore expensive build artifacts from setup job + - name: Restore cached artifacts + uses: actions/cache/restore@5a3ec84eff668545956fd18022155c47e93e2684 + with: + fail-on-cache-miss: true + path: | + ComfyUI + dist + key: comfyui-setup-${{ needs.setup.outputs.cache-key }} + + - name: Setup ComfyUI server (from cache) uses: ./.github/actions/setup-comfyui-server with: launch_server: true + + - name: Setup nodejs, pnpm, reuse built frontend + uses: ./.github/actions/setup-frontend + - name: Setup Playwright uses: ./.github/actions/setup-playwright - - name: Run Playwright tests and update snapshots + + # Run sharded tests with snapshot updates + - name: Update snapshots (Shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }}) id: playwright-tests - run: pnpm exec playwright test --update-snapshots + run: pnpm exec playwright test --update-snapshots --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} continue-on-error: true - - uses: actions/upload-artifact@v4 + + # Upload updated snapshots from this shard + - name: Upload updated snapshots + uses: actions/upload-artifact@v4 if: always() with: - name: playwright-report + name: snapshots-shard-${{ matrix.shardIndex }} + path: browser_tests/**/*-snapshots/ + retention-days: 1 + + - name: Upload test report + uses: actions/upload-artifact@v4 + if: always() + with: + name: playwright-report-shard-${{ matrix.shardIndex }} path: ./playwright-report/ retention-days: 30 + + # Merge snapshots and commit + merge-and-commit: + needs: [setup, update-snapshots-sharded] + runs-on: ubuntu-latest + if: always() + steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + ref: ${{ needs.setup.outputs.branch }} + + # Download all snapshot artifacts from shards + - name: Download all snapshots + uses: actions/download-artifact@v4 + with: + pattern: snapshots-shard-* + path: ./downloaded-snapshots + merge-multiple: false + + # Merge snapshots from all shards into browser_tests directory + - name: Merge snapshots + run: | + set -euo pipefail + + echo "Merging snapshots from all shards..." + + # Verify target directory exists + if [ ! -d "browser_tests" ]; then + echo "::error::Target directory 'browser_tests' does not exist" + exit 1 + fi + + # List all downloaded artifacts for debugging + echo "Available shard artifacts:" + ls -la ./downloaded-snapshots/ || echo "No downloaded-snapshots directory found" + + merged_count=0 + skipped_count=0 + + for shard_dir in ./downloaded-snapshots/snapshots-shard-*; do + echo "Processing: $shard_dir" + + # Check if glob matched any directories + if [ ! -e "$shard_dir" ]; then + echo "::warning::No shard directories found matching pattern" + break + fi + + if [ ! -d "$shard_dir" ]; then + echo "::warning::Expected directory but found file: $shard_dir" + skipped_count=$((skipped_count + 1)) + continue + fi + + # Check if directory has content (empty is OK - shard might have no changes) + if [ -z "$(ls -A "$shard_dir" 2>/dev/null)" ]; then + echo "Shard directory $shard_dir is empty (no snapshots updated in this shard)" + skipped_count=$((skipped_count + 1)) + continue + fi + + echo "Merging snapshots from $shard_dir..." + + # The artifact path 'browser_tests/**/*-snapshots/' uploads relative to browser_tests/ + # So downloaded artifacts contain 'tests/' directly, not 'browser_tests/tests/' + # We need to merge the artifact contents into browser_tests/ + if ! rsync -a "$shard_dir/" browser_tests/; then + echo "::error::Failed to merge snapshots from $shard_dir/ to browser_tests/" + echo "Contents of $shard_dir:" + find "$shard_dir" -type f | head -20 + exit 1 + fi + merged_count=$((merged_count + 1)) + echo "Successfully merged shard $merged_count" + done + + echo "Snapshot merge complete. Merged: $merged_count shard(s), Skipped: $skipped_count shard(s)" + - name: Debugging info run: | - echo "PR: ${{ github.event.issue.number }}" - echo "Branch: ${{ steps.get-branch.outputs.branch }}" + echo "PR: ${{ needs.setup.outputs.pr-number }}" + echo "Branch: ${{ needs.setup.outputs.branch }}" git status + - name: Commit updated expectations run: | git config --global user.name 'github-actions' @@ -89,20 +230,20 @@ jobs: echo "No changes to commit" else git commit -m "[automated] Update test expectations" - git push origin ${{ steps.get-branch.outputs.branch }} + git push origin ${{ needs.setup.outputs.branch }} fi - name: Add Done Reaction uses: peter-evans/create-or-update-comment@e8674b075228eee787fea43ef493e45ece1004c9 if: github.event_name == 'issue_comment' with: - comment-id: ${{ steps.find-update-comment.outputs.comment-id }} - issue-number: ${{ github.event.number || github.event.issue.number }} + comment-id: ${{ needs.setup.outputs.comment-id }} + issue-number: ${{ needs.setup.outputs.pr-number }} reactions: +1 reactions-edit-mode: replace - name: Remove New Browser Test Expectations label if: always() && github.event_name == 'pull_request' - run: gh pr edit ${{ github.event.pull_request.number }} --remove-label "New Browser Test Expectations" + run: gh pr edit ${{ needs.setup.outputs.pr-number }} --remove-label "New Browser Test Expectations" env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file