-
Notifications
You must be signed in to change notification settings - Fork 393
Shard Update Test Expectations PR #6100
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d0c44e8
feat: Refactor Playwright expectations update workflow for improved c…
Myestery bc1a9ec
Merge branch 'main' into shard-update-expectations
snomiao a05ac48
fix workflow issues and create modify ui to test snapshot
Myestery acba423
Merge remote-tracking branch 'origin/main' into shard-update-expectat…
Myestery 28985a0
Add dispatch trigger for testing
Myestery e9be12f
fix expectations commiting
Myestery 401379e
remove unnesessary caching
Myestery 520443b
reset toolbox
Myestery File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,74 +12,201 @@ 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' || | ||
| github.event.comment.author_association == 'MEMBER' || | ||
| 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" | ||
|
|
||
| - name: Add Starting Reaction | ||
| 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 | ||
|
Comment on lines
+67
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you look at @snomiao's recent cache removal change, it can sometimes be more expensive to try to save time like this. |
||
| - name: Generate cache key | ||
| id: cache-key | ||
| run: echo "key=$(date +%s)" >> $GITHUB_OUTPUT | ||
DrJKL marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - name: Save cache | ||
| uses: actions/cache/save@5a3ec84eff668545956fd18022155c47e93e2684 | ||
| with: | ||
| path: | | ||
| ComfyUI | ||
| dist | ||
| node_modules | ||
Myestery marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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] | ||
DrJKL marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| with: | ||
| fail-on-cache-miss: true | ||
| path: | | ||
| ComfyUI | ||
| dist | ||
| node_modules | ||
| key: comfyui-setup-${{ needs.setup.outputs.cache-key }} | ||
|
|
||
| - name: Setup ComfyUI server (from cache) | ||
| uses: ./.github/actions/setup-comfyui-server | ||
| with: | ||
| launch_server: true | ||
| skip_checkout: 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/ | ||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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() | ||
DrJKL marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| ref: ${{ needs.setup.outputs.branch }} | ||
|
|
||
| # Download all snapshot artifacts from shards | ||
| - name: Download all snapshots | ||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| merged_count=0 | ||
| for shard_dir in ./downloaded-snapshots/snapshots-shard-*; do | ||
| # Check if glob matched any directories | ||
| if [ ! -e "$shard_dir" ]; then | ||
| echo "::warning::No shard directories found matching pattern" | ||
| continue | ||
| fi | ||
|
|
||
| if [ ! -d "$shard_dir" ]; then | ||
| echo "::warning::Expected directory but found file: $shard_dir" | ||
| 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)" | ||
| continue | ||
| fi | ||
|
|
||
| echo "Copying from $shard_dir..." | ||
| if ! cp -r "$shard_dir"/* browser_tests/; then | ||
| echo "::error::Failed to copy snapshots from $shard_dir to browser_tests/" | ||
| exit 1 | ||
| fi | ||
| ((merged_count++)) | ||
| done | ||
|
|
||
| echo "Snapshot merge complete. Merged snapshots from $merged_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 +216,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 }} | ||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
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
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.
Uh oh!
There was an error while loading. Please reload this page.