From 82e777bb7eda37fe541f33eff82009b9cc6b3008 Mon Sep 17 00:00:00 2001 From: Johnpaul Chiwetelu <49923152+Myestery@users.noreply.github.com> Date: Fri, 24 Oct 2025 05:19:21 +0100 Subject: [PATCH] Patch Update expectations CI (#6250) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pull request updates the Playwright snapshot update workflow to improve efficiency and clarity. The workflow now only uploads and merges changed snapshot files from each shard, reducing unnecessary artifact size and processing. It also adds better logging and summaries for easier debugging and review. **Snapshot handling improvements:** * Only changed snapshot files are identified, staged, and uploaded as artifacts per shard, instead of uploading all snapshot files. This reduces artifact size and upload time. * During the merge step, only the changed files from each shard are merged back into the `browser_tests` directory, preserving directory structure and avoiding redundant operations. **Logging and debugging enhancements:** * Added steps to list downloaded snapshot files and summarize the changes after merging, making it easier to see what was updated and debug any issues. Sample run is here https://github.com/Myestery/ComfyUI_frontend/actions/runs/18768857441 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6250-Patch-Update-expectations-CI-2966d73d365081b790a0fad66649a10b) by [Unito](https://www.unito.io) --- .../pr-update-playwright-expectations.yaml | 134 ++++++++++++------ 1 file changed, 89 insertions(+), 45 deletions(-) diff --git a/.github/workflows/pr-update-playwright-expectations.yaml b/.github/workflows/pr-update-playwright-expectations.yaml index 8874703ae..f32b97bc4 100644 --- a/.github/workflows/pr-update-playwright-expectations.yaml +++ b/.github/workflows/pr-update-playwright-expectations.yaml @@ -120,13 +120,54 @@ jobs: run: pnpm exec playwright test --update-snapshots --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} continue-on-error: true - # Upload updated snapshots from this shard - - name: Upload updated snapshots + # Identify and stage only changed snapshot files + - name: Stage changed snapshot files + id: changed-snapshots + run: | + echo "==========================================" + echo "STAGING CHANGED SNAPSHOTS (Shard ${{ matrix.shardIndex }})" + echo "==========================================" + + # Get list of changed snapshot files + changed_files=$(git diff --name-only browser_tests/ 2>/dev/null | grep -E '\-snapshots/' || echo "") + + if [ -z "$changed_files" ]; then + echo "No snapshot changes in this shard" + echo "has-changes=false" >> $GITHUB_OUTPUT + exit 0 + fi + + echo "✓ Found changed files:" + echo "$changed_files" + file_count=$(echo "$changed_files" | wc -l) + echo "Count: $file_count" + echo "has-changes=true" >> $GITHUB_OUTPUT + echo "" + + # Create staging directory + mkdir -p /tmp/changed_snapshots_shard + + # Copy only changed files, preserving directory structure + echo "Copying changed files to staging directory..." + while IFS= read -r file; do + # Create parent directories + mkdir -p "/tmp/changed_snapshots_shard/$(dirname "$file")" + # Copy file + cp "$file" "/tmp/changed_snapshots_shard/$file" + echo " → $file" + done <<< "$changed_files" + + echo "" + echo "Staged files for upload:" + find /tmp/changed_snapshots_shard -type f + + # Upload ONLY the changed files from this shard + - name: Upload changed snapshots uses: actions/upload-artifact@v4 - if: always() + if: steps.changed-snapshots.outputs.has-changes == 'true' with: name: snapshots-shard-${{ matrix.shardIndex }} - path: browser_tests/**/*-snapshots/ + path: /tmp/changed_snapshots_shard/ retention-days: 1 - name: Upload test report @@ -155,12 +196,23 @@ jobs: path: ./downloaded-snapshots merge-multiple: false - # Merge snapshots from all shards into browser_tests directory - - name: Merge snapshots + - name: List downloaded files + run: | + echo "==========================================" + echo "DOWNLOADED SNAPSHOT FILES" + echo "==========================================" + find ./downloaded-snapshots -type f + echo "" + echo "Total files: $(find ./downloaded-snapshots -type f | wc -l)" + + # Merge only the changed files into browser_tests + - name: Merge changed snapshots run: | set -euo pipefail - echo "Merging snapshots from all shards..." + echo "==========================================" + echo "MERGING CHANGED SNAPSHOTS" + echo "==========================================" # Verify target directory exists if [ ! -d "browser_tests" ]; then @@ -168,58 +220,50 @@ jobs: 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 + # For each shard's changed files, copy them directly + for shard_dir in ./downloaded-snapshots/snapshots-shard-*/; do 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)) + shard_name=$(basename "$shard_dir") + file_count=$(find "$shard_dir" -type f | wc -l) + + if [ "$file_count" -eq 0 ]; then + echo " $shard_name: no files" 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 + echo "Processing $shard_name ($file_count file(s))..." + + # Copy files directly, preserving directory structure + # Since we only have changed files, just copy them all + cp -v -r "$shard_dir"* browser_tests/ 2>&1 | sed 's/^/ /' + merged_count=$((merged_count + 1)) - echo "Successfully merged shard $merged_count" + echo " ✓ Merged" + echo "" done - echo "Snapshot merge complete. Merged: $merged_count shard(s), Skipped: $skipped_count shard(s)" + echo "==========================================" + echo "MERGE COMPLETE" + echo "==========================================" + echo "Shards merged: $merged_count" - - name: Debugging info + - name: Show changes run: | - echo "PR: ${{ needs.setup.outputs.pr-number }}" - echo "Branch: ${{ needs.setup.outputs.branch }}" - git status - + echo "==========================================" + echo "CHANGES SUMMARY" + echo "==========================================" + echo "" + echo "Changed files in browser_tests:" + git diff --name-only browser_tests/ | head -20 || echo "No changes" + echo "" + echo "Total changes:" + git diff --name-only browser_tests/ | wc -l || echo "0" + - name: Commit updated expectations run: | git config --global user.name 'github-actions'