diff --git a/.github/actions/setup-comfyui-server/action.yml b/.github/actions/setup-comfyui-server/action.yml new file mode 100644 index 0000000000..d1aa1bd578 --- /dev/null +++ b/.github/actions/setup-comfyui-server/action.yml @@ -0,0 +1,55 @@ +name: Setup ComfyUI Server +description: 'Setup ComfyUI server for continuous integration (with ComfyUI_devtools node installed)' +inputs: + extra_server_params: + description: 'Additional parameters to pass to ComfyUI server' + required: false + default: '' + launch_server: + description: 'Whether to launch the server after setup' + required: false + default: 'false' +runs: + using: 'composite' + steps: + # Note: this workflow assume frontend repo is checked out and is built in ../dist + + # Checkout ComfyUI repo, install the dev_tools node and start server + - name: Checkout ComfyUI + uses: actions/checkout@v5 + with: + repository: 'comfyanonymous/ComfyUI' + path: 'ComfyUI' + + - name: Install ComfyUI_devtools from frontend repo + shell: bash + run: | + mkdir -p ComfyUI/custom_nodes/ComfyUI_devtools + if ! cp -r ./tools/devtools/* ComfyUI/custom_nodes/ComfyUI_devtools/; then + echo "::error::Failed to copy ComfyUI_devtools from ./tools/devtools/" + echo "::error::This action assumes the ComfyUI_frontend repository is checked out in the current working directory." + echo "::error::Please ensure you have run 'actions/checkout@v5' before calling this action." + exit 1 + fi + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: '3.10' + + - name: Install Python requirements + shell: bash + working-directory: ComfyUI + run: | + python -m pip install --upgrade pip + pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu + pip install -r requirements.txt + pip install wait-for-it + + - name: Start ComfyUI server + if: ${{ inputs.launch_server == 'true' }} + shell: bash + working-directory: ComfyUI + run: | + python main.py --cpu --multi-user --front-end-root ../dist ${{ inputs.extra_server_params }} & + wait-for-it --service 127.0.0.1:8188 -t 600 diff --git a/.github/actions/setup-frontend/action.yml b/.github/actions/setup-frontend/action.yml index 3ebc12eb3f..6787552ea7 100644 --- a/.github/actions/setup-frontend/action.yml +++ b/.github/actions/setup-frontend/action.yml @@ -1,31 +1,16 @@ -name: Setup Frontend -description: 'Setup ComfyUI frontend development environment' +name: Setup ComfyUI Frontend +description: 'Install nodejs/pnpm/dependencies and optionally build ComfyUI_frontend' inputs: - extra_server_params: - description: 'Additional parameters to pass to ComfyUI server' + include_build_step: + description: 'Include the build step to build the frontend. Set to true for workflows that need a built frontend' required: false - default: '' + default: 'false' runs: using: 'composite' steps: - - name: Checkout ComfyUI - uses: actions/checkout@v4 - with: - repository: 'comfyanonymous/ComfyUI' - path: 'ComfyUI' - - - name: Checkout ComfyUI_frontend - uses: actions/checkout@v4 - with: - repository: 'Comfy-Org/ComfyUI_frontend' - path: 'ComfyUI_frontend' - - - name: Copy ComfyUI_devtools from frontend repo - shell: bash - run: | - mkdir -p ComfyUI/custom_nodes/ComfyUI_devtools - cp -r ComfyUI_frontend/tools/devtools/* ComfyUI/custom_nodes/ComfyUI_devtools/ + # Note: this workflow assume frontend repo is checked out in the root of the workspace + # Install pnpm, Node.js, build frontend - name: Install pnpm uses: pnpm/action-setup@v4 with: @@ -36,32 +21,25 @@ runs: with: node-version: 'lts/*' cache: 'pnpm' - cache-dependency-path: 'ComfyUI_frontend/pnpm-lock.yaml' + cache-dependency-path: './pnpm-lock.yaml' - - name: Setup Python - uses: actions/setup-python@v4 + # Restore tool caches before running any build/lint operations + - name: Restore tool output cache + uses: actions/cache/restore@v4 with: - python-version: '3.10' + path: | + ./.cache + ./tsconfig.tsbuildinfo + key: tool-cache-${{ runner.os }}-${{ hashFiles('./pnpm-lock.yaml') }}-${{ hashFiles('./src/**/*.{ts,vue,js,mts}', './*.config.*') }} + restore-keys: | + tool-cache-${{ runner.os }}-${{ hashFiles('./pnpm-lock.yaml') }}- + tool-cache-${{ runner.os }}- - - name: Install Python requirements + - name: Install dependencies shell: bash - working-directory: ComfyUI - run: | - python -m pip install --upgrade pip - pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu - pip install -r requirements.txt - pip install wait-for-it + run: pnpm install --frozen-lockfile - - name: Build & Install ComfyUI_frontend + - name: Build ComfyUI_frontend + if: ${{ inputs.include_build_step == 'true' }} shell: bash - working-directory: ComfyUI_frontend - run: | - pnpm install --frozen-lockfile - pnpm build - - - name: Start ComfyUI server - shell: bash - working-directory: ComfyUI - run: | - python main.py --cpu --multi-user --front-end-root ../ComfyUI_frontend/dist ${{ inputs.extra_server_params }} & - wait-for-it --service 127.0.0.1:8188 -t 600 \ No newline at end of file + run: pnpm build diff --git a/.github/actions/setup-playwright/action.yml b/.github/actions/setup-playwright/action.yml index ddd1a76055..89629fb2c4 100644 --- a/.github/actions/setup-playwright/action.yml +++ b/.github/actions/setup-playwright/action.yml @@ -6,7 +6,6 @@ runs: - name: Detect Playwright version id: detect-version shell: bash - working-directory: ComfyUI_frontend run: | PLAYWRIGHT_VERSION=$(pnpm ls @playwright/test --json | jq --raw-output '.[0].devDependencies["@playwright/test"].version') echo "playwright-version=$PLAYWRIGHT_VERSION" >> $GITHUB_OUTPUT @@ -22,10 +21,8 @@ runs: if: steps.cache-playwright-browsers.outputs.cache-hit != 'true' shell: bash run: pnpm exec playwright install chromium --with-deps - working-directory: ComfyUI_frontend - name: Install Playwright Browsers (operating system dependencies) if: steps.cache-playwright-browsers.outputs.cache-hit == 'true' shell: bash run: pnpm exec playwright install-deps - working-directory: ComfyUI_frontend \ No newline at end of file diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index 08ad707274..76a9eb0f3d 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -11,6 +11,10 @@ on: pull_request: types: [labeled] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: wait-for-ci: runs-on: ubuntu-latest @@ -73,10 +77,10 @@ jobs: with: label_trigger: "claude-review" prompt: | - Read the file .claude/commands/comprehensive-pr-review.md and follow ALL the instructions exactly. - - CRITICAL: You must post individual inline comments using the gh api commands shown in the file. - DO NOT create a summary comment. + Read the file .claude/commands/comprehensive-pr-review.md and follow ALL the instructions exactly. + + CRITICAL: You must post individual inline comments using the gh api commands shown in the file. + DO NOT create a summary comment. Each issue must be posted as a separate inline comment on the specific line of code. anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} claude_args: "--max-turns 256 --allowedTools 'Bash(git:*),Bash(gh api:*),Bash(gh pr:*),Bash(gh repo:*),Bash(jq:*),Bash(echo:*),Read,Write,Edit,Glob,Grep,WebFetch'" @@ -86,3 +90,9 @@ jobs: COMMIT_SHA: ${{ github.event.pull_request.head.sha }} BASE_SHA: ${{ github.event.pull_request.base.sha }} REPOSITORY: ${{ github.repository }} + + - name: Remove claude-review label + if: always() + run: gh pr edit ${{ github.event.pull_request.number }} --remove-label "claude-review" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/lint-and-format.yaml b/.github/workflows/lint-and-format.yaml index 1f20ab92ed..62956cadbf 100644 --- a/.github/workflows/lint-and-format.yaml +++ b/.github/workflows/lint-and-format.yaml @@ -4,6 +4,10 @@ on: pull_request: branches-ignore: [wip/*, draft/*, temp/*] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + permissions: contents: write pull-requests: write diff --git a/.github/workflows/tests-ci.yaml b/.github/workflows/tests-ci.yaml index 47b7ee712f..c3aa236344 100644 --- a/.github/workflows/tests-ci.yaml +++ b/.github/workflows/tests-ci.yaml @@ -7,70 +7,37 @@ on: branches-ignore: [wip/*, draft/*, temp/*, vue-nodes-migration, sno-playwright-*] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: setup: runs-on: ubuntu-latest outputs: cache-key: ${{ steps.cache-key.outputs.key }} steps: - - name: Checkout ComfyUI + - name: Checkout repository uses: actions/checkout@v5 + + # Setup Test Environment, build frontend but do not start server yet + - name: Setup ComfyUI server + uses: ./.github/actions/setup-comfyui-server + - name: Setup frontend + uses: ./.github/actions/setup-frontend with: - repository: 'comfyanonymous/ComfyUI' - path: 'ComfyUI' - ref: master - - - name: Checkout ComfyUI_frontend - uses: actions/checkout@v5 - with: - repository: 'Comfy-Org/ComfyUI_frontend' - path: 'ComfyUI_frontend' - - - name: Copy ComfyUI_devtools from frontend repo - run: | - mkdir -p ComfyUI/custom_nodes/ComfyUI_devtools - cp -r ComfyUI_frontend/tools/devtools/* ComfyUI/custom_nodes/ComfyUI_devtools/ - - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10 - - - uses: actions/setup-node@v4 - with: - node-version: lts/* - cache: 'pnpm' - cache-dependency-path: 'ComfyUI_frontend/pnpm-lock.yaml' - - - name: Cache tool outputs - uses: actions/cache@v4 - with: - path: | - ComfyUI_frontend/.cache - ComfyUI_frontend/tsconfig.tsbuildinfo - key: playwright-setup-cache-${{ runner.os }}-${{ hashFiles('ComfyUI_frontend/pnpm-lock.yaml') }}-${{ hashFiles('ComfyUI_frontend/src/**/*.{ts,vue,js}', 'ComfyUI_frontend/*.config.*') }} - restore-keys: | - playwright-setup-cache-${{ runner.os }}-${{ hashFiles('ComfyUI_frontend/pnpm-lock.yaml') }}- - playwright-setup-cache-${{ runner.os }}- - playwright-tools-cache-${{ runner.os }}- - - - name: Build ComfyUI_frontend - run: | - pnpm install --frozen-lockfile - pnpm build - working-directory: ComfyUI_frontend + include_build_step: true + - name: Setup Playwright + uses: ./.github/actions/setup-playwright # Setup Playwright and cache browsers + # Save the entire workspace as cache for later test jobs to restore - 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 - ComfyUI_frontend + path: . key: comfyui-setup-${{ steps.cache-key.outputs.key }} # Sharded chromium tests @@ -85,54 +52,35 @@ jobs: shardIndex: [1, 2, 3, 4, 5, 6, 7, 8] shardTotal: [8] steps: + # download built frontend repo from setup job - name: Wait for cache propagation run: sleep 10 - - name: Restore cached setup - uses: actions/cache/restore@v4 + uses: actions/cache/restore@5a3ec84eff668545956fd18022155c47e93e2684 with: fail-on-cache-miss: true - path: | - ComfyUI - ComfyUI_frontend + path: . key: comfyui-setup-${{ needs.setup.outputs.cache-key }} - - name: Install pnpm - uses: pnpm/action-setup@v4 + # Setup Test Environment for this runner, start server, use cached built frontend ./dist from 'setup' job + - name: Setup ComfyUI server + uses: ./.github/actions/setup-comfyui-server with: - version: 10 - - - uses: actions/setup-python@v4 - with: - python-version: '3.10' - cache: 'pip' - - - name: Install requirements - run: | - python -m pip install --upgrade pip - pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu - pip install -r requirements.txt - pip install wait-for-it - working-directory: ComfyUI - - + launch_server: true + - name: Setup nodejs, pnpm, reuse built frontend + uses: ./.github/actions/setup-frontend - name: Setup Playwright - uses: ./ComfyUI_frontend/.github/actions/setup-playwright - - - name: Start ComfyUI server - run: | - python main.py --cpu --multi-user --front-end-root ../ComfyUI_frontend/dist & - wait-for-it --service 127.0.0.1:8188 -t 600 - working-directory: ComfyUI + uses: ./.github/actions/setup-playwright + # Run sharded tests and upload sharded reports - name: Run Playwright tests (Shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }}) id: playwright run: pnpm exec playwright test --project=chromium --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --reporter=blob - working-directory: ComfyUI_frontend env: - PLAYWRIGHT_BLOB_OUTPUT_DIR: ../blob-report + PLAYWRIGHT_BLOB_OUTPUT_DIR: ./blob-report - - uses: actions/upload-artifact@v4 + - name: Upload blob report + uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} with: name: blob-report-chromium-${{ matrix.shardIndex }} @@ -151,45 +99,27 @@ jobs: matrix: browser: [chromium-2x, chromium-0.5x, mobile-chrome] steps: + # download built frontend repo from setup job - name: Wait for cache propagation run: sleep 10 - - name: Restore cached setup - uses: actions/cache/restore@v4 + uses: actions/cache/restore@5a3ec84eff668545956fd18022155c47e93e2684 with: fail-on-cache-miss: true - path: | - ComfyUI - ComfyUI_frontend + path: . key: comfyui-setup-${{ needs.setup.outputs.cache-key }} - - name: Install pnpm - uses: pnpm/action-setup@v4 + # Setup Test Environment for this runner, start server, use cached built frontend ./dist from 'setup' job + - name: Setup ComfyUI server + uses: ./.github/actions/setup-comfyui-server with: - version: 10 - - - uses: actions/setup-python@v4 - with: - python-version: '3.10' - cache: 'pip' - - - name: Install requirements - run: | - python -m pip install --upgrade pip - pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu - pip install -r requirements.txt - pip install wait-for-it - working-directory: ComfyUI - + launch_server: true + - name: Setup nodejs, pnpm, reuse built frontend + uses: ./.github/actions/setup-frontend - name: Setup Playwright - uses: ./ComfyUI_frontend/.github/actions/setup-playwright - - - name: Start ComfyUI server - run: | - python main.py --cpu --multi-user --front-end-root ../ComfyUI_frontend/dist & - wait-for-it --service 127.0.0.1:8188 -t 600 - working-directory: ComfyUI + uses: ./.github/actions/setup-playwright + # Run tests and upload reports - name: Run Playwright tests (${{ matrix.browser }}) id: playwright run: | @@ -199,13 +129,13 @@ jobs: --reporter=list \ --reporter=html \ --reporter=json - working-directory: ComfyUI_frontend - - uses: actions/upload-artifact@v4 + - name: Upload Playwright report + uses: actions/upload-artifact@v4 if: always() with: name: playwright-report-${{ matrix.browser }} - path: ComfyUI_frontend/playwright-report/ + path: ./playwright-report/ retention-days: 30 # Merge sharded test reports @@ -214,32 +144,19 @@ jobs: runs-on: ubuntu-latest if: ${{ always() && !cancelled() }} steps: - - name: Checkout ComfyUI_frontend + - name: Checkout repository uses: actions/checkout@v5 - with: - repository: 'Comfy-Org/ComfyUI_frontend' - path: 'ComfyUI_frontend' - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10 - - - uses: actions/setup-node@v4 - with: - node-version: lts/* - cache: 'pnpm' - cache-dependency-path: 'ComfyUI_frontend/pnpm-lock.yaml' - - - name: Install dependencies - run: | - pnpm install --frozen-lockfile - working-directory: ComfyUI_frontend + # Setup Test Environment, we only need playwright to merge reports + - name: Setup frontend + uses: ./.github/actions/setup-frontend + - name: Setup Playwright + uses: ./.github/actions/setup-playwright - name: Download blob reports uses: actions/download-artifact@v4 with: - path: ComfyUI_frontend/all-blob-reports + path: ./all-blob-reports pattern: blob-report-chromium-* merge-multiple: true @@ -250,7 +167,6 @@ jobs: # Generate JSON report separately with explicit output path PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-report/report.json \ pnpm exec playwright merge-reports --reporter=json ./all-blob-reports - working-directory: ComfyUI_frontend - name: Build failed screenshot manifest if: ${{ needs.playwright-tests-chromium-sharded.result == 'failure' }} @@ -276,7 +192,7 @@ jobs: uses: actions/upload-artifact@v4 with: name: playwright-report-chromium - path: ComfyUI_frontend/playwright-report/ + path: ./playwright-report/ retention-days: 30 #### BEGIN Deployment and commenting (non-forked PRs only) @@ -292,11 +208,11 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v5 - + - name: Get start time id: start-time run: echo "time=$(date -u '+%m/%d/%Y, %I:%M:%S %p')" >> $GITHUB_OUTPUT - + - name: Post starting comment env: GITHUB_TOKEN: ${{ github.token }} @@ -307,7 +223,7 @@ jobs: "${{ github.head_ref }}" \ "starting" \ "${{ steps.start-time.outputs.time }}" - + # Deploy and comment for non-forked PRs only deploy-and-comment: needs: [playwright-tests, merge-reports] @@ -319,23 +235,20 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v5 - + - name: Download all playwright reports uses: actions/download-artifact@v4 with: pattern: playwright-report-* path: reports - - - name: Make deployment script executable - run: chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh - + - name: Deploy reports and comment on PR env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} GITHUB_TOKEN: ${{ github.token }} run: | - ./scripts/cicd/pr-playwright-deploy-and-comment.sh \ + bash ./scripts/cicd/pr-playwright-deploy-and-comment.sh \ "${{ github.event.pull_request.number }}" \ "${{ github.head_ref }}" \ "completed" diff --git a/.github/workflows/update-locales-for-given-custom-node-repository.yaml b/.github/workflows/update-locales-for-given-custom-node-repository.yaml index ec085eab59..b9d1b33b91 100644 --- a/.github/workflows/update-locales-for-given-custom-node-repository.yaml +++ b/.github/workflows/update-locales-for-given-custom-node-repository.yaml @@ -21,90 +21,64 @@ jobs: update-locales: runs-on: ubuntu-latest steps: - - name: Checkout ComfyUI + - name: Checkout repository uses: actions/checkout@v5 + + # Setup playwright environment with custom node repository + - name: Setup ComfyUI Server (without launching) + uses: ./.github/actions/setup-comfyui-server + - name: Setup frontend + uses: ./.github/actions/setup-frontend with: - repository: comfyanonymous/ComfyUI - path: ComfyUI - ref: master - - name: Checkout ComfyUI_frontend - uses: actions/checkout@v5 - with: - repository: Comfy-Org/ComfyUI_frontend - path: ComfyUI_frontend - - name: Copy ComfyUI_devtools from frontend repo - run: | - mkdir -p ComfyUI/custom_nodes/ComfyUI_devtools - cp -r ComfyUI_frontend/tools/devtools/* ComfyUI/custom_nodes/ComfyUI_devtools/ + include_build_step: 'true' + - name: Setup Playwright + uses: ./.github/actions/setup-playwright + + # Install the custom node repository - name: Checkout custom node repository uses: actions/checkout@v5 with: repository: ${{ inputs.owner }}/${{ inputs.repository }} path: 'ComfyUI/custom_nodes/${{ inputs.repository }}' - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10 - - uses: actions/setup-node@v4 - with: - node-version: 'lts/*' - cache: 'pnpm' - - uses: actions/setup-python@v4 - with: - python-version: '3.10' - - name: Install ComfyUI requirements - run: | - python -m pip install --upgrade pip - pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu - pip install -r requirements.txt - pip install wait-for-it - working-directory: ComfyUI - - name: Install custom node requirements + - name: Install custom node Python requirements + working-directory: ComfyUI/custom_nodes/${{ inputs.repository }} run: | if [ -f "requirements.txt" ]; then pip install -r requirements.txt fi - working-directory: ComfyUI/custom_nodes/${{ inputs.repository }} - - name: Build & Install ComfyUI_frontend - run: | - pnpm install --frozen-lockfile - pnpm build - rm -rf ../ComfyUI/web/* - mv dist/* ../ComfyUI/web/ - working-directory: ComfyUI_frontend - - name: Start ComfyUI server - run: | - python main.py --cpu --multi-user & - wait-for-it --service 127.0.0.1:8188 -t 600 + + # Start ComfyUI Server + - name: Start ComfyUI Server + shell: bash working-directory: ComfyUI - - name: Setup Playwright - uses: ./ComfyUI_frontend/.github/actions/setup-playwright + run: | + python main.py --cpu --multi-user --front-end-root ../dist --custom-node-path ../ComfyUI/custom_nodes/${{ inputs.repository }} & + wait-for-it --service + - name: Start dev server # Run electron dev server as it is a superset of the web dev server # We do want electron specific UIs to be translated. run: pnpm dev:electron & - working-directory: ComfyUI_frontend + - name: Capture base i18n run: pnpm exec tsx scripts/diff-i18n capture - working-directory: ComfyUI_frontend - name: Update en.json run: pnpm collect-i18n env: PLAYWRIGHT_TEST_URL: http://localhost:5173 - working-directory: ComfyUI_frontend - name: Update translations run: pnpm locale env: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - working-directory: ComfyUI_frontend - name: Diff base vs updated i18n run: pnpm exec tsx scripts/diff-i18n diff - working-directory: ComfyUI_frontend - name: Update i18n in custom node repository run: | LOCALE_DIR=ComfyUI/custom_nodes/${{ inputs.repository }}/locales/ install -d "$LOCALE_DIR" cp -rf ComfyUI_frontend/temp/diff/* "$LOCALE_DIR" + + # Git ops for pushing changes and creating PR - name: Check and create fork of custom node repository run: | # Try to fork the repository diff --git a/.github/workflows/update-locales.yaml b/.github/workflows/update-locales.yaml index 6ee0933121..9ffa702cae 100644 --- a/.github/workflows/update-locales.yaml +++ b/.github/workflows/update-locales.yaml @@ -16,36 +16,33 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v5 - - - name: Setup Frontend + + # Setup playwright environment + - name: Setup ComfyUI Frontend uses: ./.github/actions/setup-frontend - - - name: Cache tool outputs - uses: actions/cache@v4 with: - path: | - ComfyUI_frontend/.cache - ComfyUI_frontend/.cache - key: i18n-tools-cache-${{ runner.os }}-${{ hashFiles('ComfyUI_frontend/pnpm-lock.yaml') }} - restore-keys: | - i18n-tools-cache-${{ runner.os }}- + include_build_step: true + - name: Setup ComfyUI Server + uses: ./.github/actions/setup-comfyui-server + with: + launch_server: true - name: Setup Playwright uses: ./.github/actions/setup-playwright + - name: Start dev server # Run electron dev server as it is a superset of the web dev server # We do want electron specific UIs to be translated. run: pnpm dev:electron & - working-directory: ComfyUI_frontend + + # Update locales, collect new strings and update translations using OpenAI, then commit changes - name: Update en.json run: pnpm collect-i18n env: PLAYWRIGHT_TEST_URL: http://localhost:5173 - working-directory: ComfyUI_frontend - name: Update translations run: pnpm locale env: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - working-directory: ComfyUI_frontend - name: Commit updated locales run: | git config --global user.name 'github-actions' @@ -59,4 +56,3 @@ jobs: git add src/locales/ git diff --staged --quiet || git commit -m "Update locales [skip ci]" git push origin HEAD:${{ github.head_ref }} - working-directory: ComfyUI_frontend diff --git a/.github/workflows/update-node-definitions-locales.yaml b/.github/workflows/update-node-definitions-locales.yaml index b063159ddf..ce991d09ea 100644 --- a/.github/workflows/update-node-definitions-locales.yaml +++ b/.github/workflows/update-node-definitions-locales.yaml @@ -13,24 +13,32 @@ jobs: update-locales: runs-on: ubuntu-latest steps: - - uses: Comfy-Org/ComfyUI_frontend_setup_action@v3 + - name: Checkout repository + uses: actions/checkout@v5 + # Setup playwright environment + - name: Setup ComfyUI Server (and start) + uses: ./.github/actions/setup-comfyui-server + with: + launch_server: true + - name: Setup frontend + uses: ./.github/actions/setup-frontend + with: + include_build_step: true - name: Setup Playwright uses: ./.github/actions/setup-playwright + - name: Start dev server # Run electron dev server as it is a superset of the web dev server # We do want electron specific UIs to be translated. run: pnpm dev:electron & - working-directory: ComfyUI_frontend - name: Update en.json run: pnpm collect-i18n -- scripts/collect-i18n-node-defs.ts env: PLAYWRIGHT_TEST_URL: http://localhost:5173 - working-directory: ComfyUI_frontend - name: Update translations run: pnpm locale env: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - working-directory: ComfyUI_frontend - name: Create Pull Request uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e with: @@ -44,4 +52,3 @@ jobs: branch: update-locales-node-defs-${{ github.event.inputs.trigger_type }}-${{ github.run_id }} base: main labels: dependencies - path: ComfyUI_frontend \ No newline at end of file diff --git a/.github/workflows/update-playwright-expectations.yaml b/.github/workflows/update-playwright-expectations.yaml index ece83e3900..f9780bba39 100644 --- a/.github/workflows/update-playwright-expectations.yaml +++ b/.github/workflows/update-playwright-expectations.yaml @@ -15,12 +15,16 @@ on: issue_comment: types: [created] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: test: 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' || @@ -29,17 +33,46 @@ jobs: ) && startsWith(github.event.comment.body, '/update-playwright') ) steps: + - name: Find Update Comment + uses: peter-evans/find-comment@b30e6a3c0ed37e7c023ccd3f1db5c6c0b0c23aad + id: "find-update-comment" + with: + issue-number: ${{ github.event.number || github.event.issue.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 }} + 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 uses: actions/checkout@v5 - - - name: Pull Request Checkout - if: github.event.issue.pull_request && github.event_name == 'issue_comment' - run: gh pr checkout ${{ github.event.issue.number }} - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + ref: ${{ steps.get-branch.outputs.branch }} - name: Setup Frontend uses: ./.github/actions/setup-frontend + with: + include_build_step: true + + - name: Setup ComfyUI Server + uses: ./.github/actions/setup-comfyui-server + with: + launch_server: true - name: Setup Playwright uses: ./.github/actions/setup-playwright @@ -96,12 +129,10 @@ jobs: with: run-id: ${{ steps.locate-manifest.outputs.run_id }} name: failed-screenshot-tests - path: ComfyUI_frontend/ci-rerun + path: ci-rerun - name: Re-run failed screenshot tests and update snapshots id: playwright-tests - shell: bash - working-directory: ComfyUI_frontend continue-on-error: true run: | set -euo pipefail @@ -183,27 +214,20 @@ jobs: if: always() with: name: playwright-report - path: ComfyUI_frontend/playwright-report/ + path: ./playwright-report/ retention-days: 30 - name: Debugging info - working-directory: ComfyUI_frontend run: | - echo "Branch: ${{ github.head_ref }}" + echo "PR: ${{ github.event.issue.number }}" + echo "Branch: ${{ steps.get-branch.outputs.branch }}" git status - name: Commit updated expectations id: commit - working-directory: ComfyUI_frontend run: | git config --global user.name 'github-actions' git config --global user.email 'github-actions@github.com' - if [ "${{ github.event_name }}" = "issue_comment" ]; then - true - else - git fetch origin ${{ github.head_ref }} - git checkout -B ${{ github.head_ref }} origin/${{ github.head_ref }} - fi git add browser_tests if git diff --cached --quiet; then echo "No expectation updates detected; skipping commit." @@ -215,16 +239,11 @@ jobs: echo "count=$changed_count" >> $GITHUB_OUTPUT git commit -m "[automated] Update test expectations" - if [ "${{ github.event_name }}" = "issue_comment" ]; then - git push - else - git push origin HEAD:${{ github.head_ref }} - fi + git push origin ${{ steps.get-branch.outputs.branch }} fi - name: Generate workflow summary if: always() - working-directory: ComfyUI_frontend run: | echo "## Snapshot Update Summary" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY @@ -248,3 +267,18 @@ jobs: echo "---" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "**Strategy:** Selective snapshot update (only failed tests re-run)" >> $GITHUB_STEP_SUMMARY + + - 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 }} + 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" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/vitest-tests.yaml b/.github/workflows/vitest-tests.yaml index 46155d9121..3941451886 100644 --- a/.github/workflows/vitest-tests.yaml +++ b/.github/workflows/vitest-tests.yaml @@ -6,6 +6,10 @@ on: pull_request: branches-ignore: [wip/*, draft/*, temp/*] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: test: runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 59c9af1cd7..0d060af1f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ - Routing/i18n/entry: `src/router.ts`, `src/i18n.ts`, `src/main.ts`. - Tests: unit/component in `tests-ui/` and `src/components/**/*.{test,spec}.ts`; E2E in `browser_tests/`. - Public assets: `public/`. Build output: `dist/`. -- Config: `vite.config.mts`, `vitest.config.ts`, `playwright.config.ts`, `eslint.config.js`, `.prettierrc`. +- Config: `vite.config.mts`, `vitest.config.ts`, `playwright.config.ts`, `eslint.config.ts`, `.prettierrc`. ## Build, Test, and Development Commands - `pnpm dev`: Start Vite dev server. diff --git a/CLAUDE.md b/CLAUDE.md index 74e656f005..0b187fbfcc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -126,6 +126,5 @@ const value = api.getServerFeature('config_name', defaultValue) // Get config - NEVER use `--no-verify` flag when committing - NEVER delete or disable tests to make them pass - NEVER circumvent quality checks -- NEVER use `dark:` prefix - always use `dark-theme:` for dark mode styles, for example: `dark-theme:text-white dark-theme:bg-black` -- NEVER use `:class="[]"` to merge class names - always use `import { cn } from '@/utils/tailwindUtil'`, for example: `
` - +- NEVER use `dark:` or `dark-theme:` tailwind variants. Instead use a semantic value from the `style.css` theme, e.g. `bg-node-component-surface` +- NEVER use `:class="[]"` to merge class names - always use `import { cn } from '@/utils/tailwindUtil'`, for example: `` diff --git a/CODEOWNERS b/CODEOWNERS index d3517e2ab1..d754859b10 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -54,3 +54,10 @@ # Translations /src/locales/ @Yorha4D @KarryCharon @shinshin86 @Comfy-Org/comfy_maintainer + +# LLM Instructions (blank on purpose) +.claude/ +.cursor/ +.cursorrules +**/AGENTS.md +**/CLAUDE.md \ No newline at end of file diff --git a/browser_tests/assets/vueNodes/linked-int-widget.json b/browser_tests/assets/vueNodes/linked-int-widget.json new file mode 100644 index 0000000000..9aa7b0f9a9 --- /dev/null +++ b/browser_tests/assets/vueNodes/linked-int-widget.json @@ -0,0 +1,90 @@ +{ + "id": "95ea19ba-456c-46e8-aa40-dc3ff135b746", + "revision": 0, + "last_node_id": 11, + "last_link_id": 10, + "nodes": [ + { + "id": 10, + "type": "KSampler", + "pos": [494.3333740234375, 142.3333282470703], + "size": [444, 399], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "name": "model", + "type": "MODEL", + "link": null + }, + { + "name": "positive", + "type": "CONDITIONING", + "link": null + }, + { + "name": "negative", + "type": "CONDITIONING", + "link": null + }, + { + "name": "latent_image", + "type": "LATENT", + "link": null + }, + { + "name": "seed", + "type": "INT", + "widget": { + "name": "seed" + }, + "link": 10 + } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": null + } + ], + "properties": { + "Node name for S&R": "KSampler" + }, + "widgets_values": [67, "randomize", 20, 8, "euler", "simple", 1] + }, + { + "id": 11, + "type": "PrimitiveInt", + "pos": [24.333343505859375, 149.6666717529297], + "size": [444, 125], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "INT", + "type": "INT", + "links": [10] + } + ], + "properties": { + "Node name for S&R": "PrimitiveInt" + }, + "widgets_values": [67, "randomize"] + } + ], + "links": [[10, 11, 0, 10, 4, "INT"]], + "groups": [], + "config": {}, + "extra": { + "ds": { + "scale": 1, + "offset": [0, 0] + }, + "frontendVersion": "1.28.6" + }, + "version": 0.4 +} diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 19796f4c4c..d46b31a98a 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -1,6 +1,5 @@ import type { APIRequestContext, Locator, Page } from '@playwright/test' -import { expect } from '@playwright/test' -import { test as base } from '@playwright/test' +import { test as base, expect } from '@playwright/test' import dotenv from 'dotenv' import * as fs from 'fs' @@ -130,7 +129,8 @@ export class ComfyPage { // Buttons public readonly resetViewButton: Locator - public readonly queueButton: Locator + public readonly queueButton: Locator // Run button in Legacy UI + public readonly runButton: Locator // Run button (renamed "Queue" -> "Run") // Inputs public readonly workflowUploadInput: Locator @@ -165,6 +165,9 @@ export class ComfyPage { this.widgetTextBox = page.getByPlaceholder('text').nth(1) this.resetViewButton = page.getByRole('button', { name: 'Reset View' }) this.queueButton = page.getByRole('button', { name: 'Queue Prompt' }) + this.runButton = page + .getByTestId('queue-button') + .getByRole('button', { name: 'Run' }) this.workflowUploadInput = page.locator('#comfy-file-input') this.visibleToasts = page.locator('.p-toast-message:visible') @@ -1086,12 +1089,6 @@ export class ComfyPage { const targetPosition = await targetSlot.getPosition() - // Debug: Log the positions we're trying to use - console.log('Drag positions:', { - source: sourcePosition, - target: targetPosition - }) - await this.dragAndDrop(sourcePosition, targetPosition) await this.nextFrame() } diff --git a/browser_tests/fixtures/VueNodeHelpers.ts b/browser_tests/fixtures/VueNodeHelpers.ts index bc4f32452c..64c03b156a 100644 --- a/browser_tests/fixtures/VueNodeHelpers.ts +++ b/browser_tests/fixtures/VueNodeHelpers.ts @@ -119,4 +119,24 @@ export class VueNodeHelpers { await this.page.waitForSelector('[data-node-id]') } } + + /** + * Get a specific widget by node title and widget name + */ + getWidgetByName(nodeTitle: string, widgetName: string): Locator { + return this.getNodeByTitle(nodeTitle).locator( + `_vue=[widget.name="${widgetName}"]` + ) + } + + /** + * Get controls for input number widgets (increment/decrement buttons and input) + */ + getInputNumberControls(widget: Locator) { + return { + input: widget.locator('input'), + incrementButton: widget.locator('button').first(), + decrementButton: widget.locator('button').last() + } + } } diff --git a/browser_tests/tests/minimap.spec.ts b/browser_tests/tests/minimap.spec.ts index df967d911e..d1ab05fc53 100644 --- a/browser_tests/tests/minimap.spec.ts +++ b/browser_tests/tests/minimap.spec.ts @@ -66,12 +66,22 @@ test.describe('Minimap', () => { await comfyPage.nextFrame() await expect(minimapContainer).not.toBeVisible() + + // Open zoom controls dropdown again + await zoomControlsButton.click() + await comfyPage.nextFrame() + await expect(toggleButton).toContainText('Show Minimap') await toggleButton.click() await comfyPage.nextFrame() await expect(minimapContainer).toBeVisible() + + // Open zoom controls dropdown again to verify button text + await zoomControlsButton.click() + await comfyPage.nextFrame() + await expect(toggleButton).toContainText('Hide Minimap') }) diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-varying-content-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-varying-content-chromium-linux.png index 2548e66ae8..74c9f0b4b5 100644 Binary files a/browser_tests/tests/templates.spec.ts-snapshots/template-grid-varying-content-chromium-linux.png and b/browser_tests/tests/templates.spec.ts-snapshots/template-grid-varying-content-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.png b/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.png index 6eb1e94f50..abdc6321d1 100644 Binary files a/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.png and b/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-create-group-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.png b/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.png index ec0efa7b5e..4ad0c7c77a 100644 Binary files a/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.png and b/browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts-snapshots/vue-nodes-paned-with-touch-mobile-chrome-linux.png b/browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts-snapshots/vue-nodes-paned-with-touch-mobile-chrome-linux.png index 02f7dfd5bd..16a40d83d1 100644 Binary files a/browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts-snapshots/vue-nodes-paned-with-touch-mobile-chrome-linux.png and b/browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts-snapshots/vue-nodes-paned-with-touch-mobile-chrome-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/canvas/zoom.spec.ts-snapshots/zoomed-in-ctrl-shift-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/canvas/zoom.spec.ts-snapshots/zoomed-in-ctrl-shift-chromium-linux.png index 528d65f516..15065b8f28 100644 Binary files a/browser_tests/tests/vueNodes/interactions/canvas/zoom.spec.ts-snapshots/zoomed-in-ctrl-shift-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/canvas/zoom.spec.ts-snapshots/zoomed-in-ctrl-shift-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts index a95f9cf19b..8989dc6329 100644 --- a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts +++ b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts @@ -788,4 +788,171 @@ test.describe('Vue Node Link Interaction', () => { targetSlot: 2 }) }) + + test.describe('Release actions (Shift-drop)', () => { + test('Context menu opens and endpoint is pinned on Shift-drop', async ({ + comfyPage, + comfyMouse + }) => { + await comfyPage.setSetting( + 'Comfy.LinkRelease.ActionShift', + 'context menu' + ) + + const samplerNode = (await comfyPage.getNodeRefsByType('KSampler'))[0] + expect(samplerNode).toBeTruthy() + + const outputCenter = await getSlotCenter( + comfyPage.page, + samplerNode.id, + 0, + false + ) + + const dropPos = { x: outputCenter.x + 180, y: outputCenter.y - 140 } + + await comfyMouse.move(outputCenter) + await comfyPage.page.keyboard.down('Shift') + try { + await comfyMouse.drag(dropPos) + await comfyMouse.drop() + } finally { + await comfyPage.page.keyboard.up('Shift').catch(() => {}) + } + + // Context menu should be visible + const contextMenu = comfyPage.page.locator('.litecontextmenu') + await expect(contextMenu).toBeVisible() + + // Pinned endpoint should not change with mouse movement while menu is open + const before = await comfyPage.page.evaluate(() => { + const snap = window['app']?.canvas?.linkConnector?.state?.snapLinksPos + return Array.isArray(snap) ? [snap[0], snap[1]] : null + }) + expect(before).not.toBeNull() + + // Move mouse elsewhere and verify snap position is unchanged + await comfyMouse.move({ x: dropPos.x + 160, y: dropPos.y + 100 }) + const after = await comfyPage.page.evaluate(() => { + const snap = window['app']?.canvas?.linkConnector?.state?.snapLinksPos + return Array.isArray(snap) ? [snap[0], snap[1]] : null + }) + expect(after).toEqual(before) + }) + + test('Context menu -> Search pre-filters by link type and connects after selection', async ({ + comfyPage, + comfyMouse + }) => { + await comfyPage.setSetting( + 'Comfy.LinkRelease.ActionShift', + 'context menu' + ) + await comfyPage.setSetting('Comfy.NodeSearchBoxImpl', 'default') + + const samplerNode = (await comfyPage.getNodeRefsByType('KSampler'))[0] + expect(samplerNode).toBeTruthy() + + const outputCenter = await getSlotCenter( + comfyPage.page, + samplerNode.id, + 0, + false + ) + const dropPos = { x: outputCenter.x + 200, y: outputCenter.y - 120 } + + await comfyMouse.move(outputCenter) + await comfyPage.page.keyboard.down('Shift') + try { + await comfyMouse.drag(dropPos) + await comfyMouse.drop() + } finally { + await comfyPage.page.keyboard.up('Shift').catch(() => {}) + } + + // Open Search from the context menu + await comfyPage.clickContextMenuItem('Search') + + // Search box opens with prefilled type filter based on link type (LATENT) + await expect(comfyPage.searchBox.input).toBeVisible() + const chips = comfyPage.searchBox.filterChips + // Ensure at least one filter chip exists and it matches the link type + const chipCount = await chips.count() + expect(chipCount).toBeGreaterThan(0) + await expect(chips.first()).toContainText('LATENT') + + // Choose a compatible node and verify it auto-connects + await comfyPage.searchBox.fillAndSelectFirstNode('VAEDecode') + await comfyPage.nextFrame() + + // KSampler output should now have an outgoing link + const samplerOutput = await samplerNode.getOutput(0) + expect(await samplerOutput.getLinkCount()).toBe(1) + + // One of the VAEDecode nodes should have an incoming link on input[0] + const vaeNodes = await comfyPage.getNodeRefsByType('VAEDecode') + let linked = false + for (const vae of vaeNodes) { + const details = await getInputLinkDetails(comfyPage.page, vae.id, 0) + if (details) { + expect(details.originId).toBe(samplerNode.id) + linked = true + break + } + } + expect(linked).toBe(true) + }) + + test('Search box opens on Shift-drop and connects after selection', async ({ + comfyPage, + comfyMouse + }) => { + await comfyPage.setSetting('Comfy.LinkRelease.ActionShift', 'search box') + + const samplerNode = (await comfyPage.getNodeRefsByType('KSampler'))[0] + expect(samplerNode).toBeTruthy() + + const outputCenter = await getSlotCenter( + comfyPage.page, + samplerNode.id, + 0, + false + ) + const dropPos = { x: outputCenter.x + 140, y: outputCenter.y - 100 } + + await comfyMouse.move(outputCenter) + await comfyPage.page.keyboard.down('Shift') + try { + await comfyMouse.drag(dropPos) + await comfyMouse.drop() + } finally { + await comfyPage.page.keyboard.up('Shift').catch(() => {}) + } + + // Search box should open directly + await expect(comfyPage.searchBox.input).toBeVisible() + await expect(comfyPage.searchBox.filterChips.first()).toContainText( + 'LATENT' + ) + + // Select a compatible node and verify connection + await comfyPage.searchBox.fillAndSelectFirstNode('VAEDecode') + await comfyPage.nextFrame() + + const samplerOutput = await samplerNode.getOutput(0) + expect(await samplerOutput.getLinkCount()).toBe(1) + + const vaeNodes = await comfyPage.getNodeRefsByType('VAEDecode') + let linked = false + for (const vae of vaeNodes) { + const details = await getInputLinkDetails(comfyPage.page, vae.id, 0) + if (details) { + expect(details.originId).toBe(samplerNode.id) + linked = true + break + } + } + expect(linked).toBe(true) + }) + }) }) diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.png index eb3725e5c6..f2085123e4 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.png index ff85a019bb..d83bf025e6 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.png index b69b3d3993..b99cf7b352 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.png index 023d075adc..9523f2f17f 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.png index d267102166..a811f21947 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.png index a0271f86b2..34c4b43593 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.png index 3f0d5f72a4..838996caa0 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.png index f1dbcf18f9..459b0eab70 100644 Binary files a/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png index 6cefdc2a98..279db6bdb6 100644 Binary files a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png and b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png index 172c373fc8..dd727958e8 100644 Binary files a/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png and b/browser_tests/tests/vueNodes/interactions/node/move.spec.ts-snapshots/vue-node-moved-node-touch-mobile-chrome-linux.png differ diff --git a/browser_tests/tests/vueNodes/interactions/node/select.spec.ts b/browser_tests/tests/vueNodes/interactions/node/select.spec.ts index 2af6765896..4402236d24 100644 --- a/browser_tests/tests/vueNodes/interactions/node/select.spec.ts +++ b/browser_tests/tests/vueNodes/interactions/node/select.spec.ts @@ -49,4 +49,36 @@ test.describe('Vue Node Selection', () => { expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(0) }) } + + test('should select pinned node without dragging', async ({ comfyPage }) => { + const PIN_HOTKEY = 'p' + const PIN_INDICATOR = '[data-testid="node-pin-indicator"]' + + // Select a node by clicking its title + const checkpointNodeHeader = comfyPage.page.getByText('Load Checkpoint') + await checkpointNodeHeader.click() + + // Pin it using the hotkey (as a user would) + await comfyPage.page.keyboard.press(PIN_HOTKEY) + + const checkpointNode = comfyPage.vueNodes.getNodeByTitle('Load Checkpoint') + const pinIndicator = checkpointNode.locator(PIN_INDICATOR) + await expect(pinIndicator).toBeVisible() + + expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(1) + + const initialPos = await checkpointNodeHeader.boundingBox() + if (!initialPos) throw new Error('Failed to get header position') + + await comfyPage.dragAndDrop( + { x: initialPos.x + 10, y: initialPos.y + 10 }, + { x: initialPos.x + 100, y: initialPos.y + 100 } + ) + + const finalPos = await checkpointNodeHeader.boundingBox() + if (!finalPos) throw new Error('Failed to get header position after drag') + expect(finalPos).toEqual(initialPos) + + expect(await comfyPage.vueNodes.getSelectedNodeCount()).toBe(1) + }) }) diff --git a/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts index 74ec17cc90..2d67ca99f1 100644 --- a/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts +++ b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts @@ -20,6 +20,9 @@ test.describe('Vue Node Bypass', () => { const checkpointNode = comfyPage.vueNodes.getNodeByTitle('Load Checkpoint') await expect(checkpointNode).toHaveClass(BYPASS_CLASS) + await expect(comfyPage.canvas).toHaveScreenshot( + 'vue-node-bypassed-state.png' + ) await comfyPage.page.keyboard.press(BYPASS_HOTKEY) await expect(checkpointNode).not.toHaveClass(BYPASS_CLASS) diff --git a/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png new file mode 100644 index 0000000000..a4391bbe85 Binary files /dev/null and b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-color-blue-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-color-blue-chromium-linux.png index 3ccb97b5a7..4e9b998d62 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-color-blue-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-color-blue-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.png index 0873ebfbbd..f4a49418cd 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.png index 4326b8f7e7..46be17201f 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/error.spec.ts b/browser_tests/tests/vueNodes/nodeStates/error.spec.ts index f4f8e10fe0..b8c718239c 100644 --- a/browser_tests/tests/vueNodes/nodeStates/error.spec.ts +++ b/browser_tests/tests/vueNodes/nodeStates/error.spec.ts @@ -3,7 +3,7 @@ import { comfyPageFixture as test } from '../../../fixtures/ComfyPage' -const ERROR_CLASS = /border-error/ +const ERROR_CLASS = /border-node-stroke-error/ test.describe('Vue Node Error', () => { test.beforeEach(async ({ comfyPage }) => { @@ -17,16 +17,21 @@ test.describe('Vue Node Error', () => { await comfyPage.setup() await comfyPage.loadWorkflow('missing/missing_nodes') - // Close missing nodes warning dialog - await comfyPage.page.getByRole('button', { name: 'Close' }).click() - await comfyPage.page.waitForSelector('.comfy-missing-nodes', { - state: 'hidden' - }) - // Expect error state on missing unknown node const unknownNode = comfyPage.page.locator('[data-node-id]').filter({ hasText: 'UNKNOWN NODE' }) await expect(unknownNode).toHaveClass(ERROR_CLASS) }) + + test('should display error state when node causes execution error', async ({ + comfyPage + }) => { + await comfyPage.setup() + await comfyPage.loadWorkflow('nodes/execution_error') + await comfyPage.runButton.click() + + const raiseErrorNode = comfyPage.vueNodes.getNodeByTitle('Raise Error') + await expect(raiseErrorNode).toHaveClass(ERROR_CLASS) + }) }) diff --git a/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-default-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-default-chromium-linux.png index 89173c6409..25186e6e7d 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-default-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-default-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-active-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-active-chromium-linux.png index e6281d3255..c00eddccf8 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-active-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-active-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-inactive-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-inactive-chromium-linux.png index f05e4f68be..d83d19c86d 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-inactive-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/lod.spec.ts-snapshots/vue-nodes-lod-inactive-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts b/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts index 37dcfd37b5..3fe656ebc9 100644 --- a/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts +++ b/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts @@ -4,7 +4,7 @@ import { } from '../../../fixtures/ComfyPage' const MUTE_HOTKEY = 'Control+m' -const MUTE_CLASS = /opacity-50/ +const MUTE_OPACITY = '0.5' test.describe('Vue Node Mute', () => { test.beforeEach(async ({ comfyPage }) => { @@ -19,10 +19,11 @@ test.describe('Vue Node Mute', () => { await comfyPage.page.keyboard.press(MUTE_HOTKEY) const checkpointNode = comfyPage.vueNodes.getNodeByTitle('Load Checkpoint') - await expect(checkpointNode).toHaveClass(MUTE_CLASS) + await expect(checkpointNode).toHaveCSS('opacity', MUTE_OPACITY) + await expect(comfyPage.canvas).toHaveScreenshot('vue-node-muted-state.png') await comfyPage.page.keyboard.press(MUTE_HOTKEY) - await expect(checkpointNode).not.toHaveClass(MUTE_CLASS) + await expect(checkpointNode).not.toHaveCSS('opacity', MUTE_OPACITY) }) test('should allow toggling mute on multiple selected nodes with hotkey', async ({ @@ -35,11 +36,11 @@ test.describe('Vue Node Mute', () => { const ksamplerNode = comfyPage.vueNodes.getNodeByTitle('KSampler') await comfyPage.page.keyboard.press(MUTE_HOTKEY) - await expect(checkpointNode).toHaveClass(MUTE_CLASS) - await expect(ksamplerNode).toHaveClass(MUTE_CLASS) + await expect(checkpointNode).toHaveCSS('opacity', MUTE_OPACITY) + await expect(ksamplerNode).toHaveCSS('opacity', MUTE_OPACITY) await comfyPage.page.keyboard.press(MUTE_HOTKEY) - await expect(checkpointNode).not.toHaveClass(MUTE_CLASS) - await expect(ksamplerNode).not.toHaveClass(MUTE_CLASS) + await expect(checkpointNode).not.toHaveCSS('opacity', MUTE_OPACITY) + await expect(ksamplerNode).not.toHaveCSS('opacity', MUTE_OPACITY) }) }) diff --git a/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png new file mode 100644 index 0000000000..1bc4b1edc3 Binary files /dev/null and b/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts b/browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts new file mode 100644 index 0000000000..bb956e3395 --- /dev/null +++ b/browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts @@ -0,0 +1,42 @@ +import { + comfyExpect as expect, + comfyPageFixture as test +} from '../../../../fixtures/ComfyPage' + +test.describe('Vue Integer Widget', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.setSetting('Comfy.VueNodes.Enabled', true) + await comfyPage.setup() + }) + + test('should be disabled and not allow changing value when link connected to slot', async ({ + comfyPage + }) => { + await comfyPage.loadWorkflow('vueNodes/linked-int-widget') + await comfyPage.vueNodes.waitForNodes() + + const seedWidget = comfyPage.vueNodes.getWidgetByName('KSampler', 'seed') + const controls = comfyPage.vueNodes.getInputNumberControls(seedWidget) + const initialValue = Number(await controls.input.inputValue()) + + // Verify widget is disabled when linked + await controls.incrementButton.click({ force: true }) + await expect(controls.input).toHaveValue(initialValue.toString()) + + await controls.decrementButton.click({ force: true }) + await expect(controls.input).toHaveValue(initialValue.toString()) + + await expect(seedWidget).toBeVisible() + + // Delete the node that is linked to the slot (freeing up the widget) + await comfyPage.vueNodes.getNodeByTitle('Int').click() + await comfyPage.vueNodes.deleteSelected() + + // Test widget works when unlinked + await controls.incrementButton.click() + await expect(controls.input).toHaveValue((initialValue + 1).toString()) + + await controls.decrementButton.click() + await expect(controls.input).toHaveValue(initialValue.toString()) + }) +}) diff --git a/browser_tests/tests/vueNodes/widgets/load/uploadWidgets.spec.ts-snapshots/vue-nodes-upload-widgets-chromium-linux.png b/browser_tests/tests/vueNodes/widgets/load/uploadWidgets.spec.ts-snapshots/vue-nodes-upload-widgets-chromium-linux.png index 6e960b8bb8..9f0cbc6498 100644 Binary files a/browser_tests/tests/vueNodes/widgets/load/uploadWidgets.spec.ts-snapshots/vue-nodes-upload-widgets-chromium-linux.png and b/browser_tests/tests/vueNodes/widgets/load/uploadWidgets.spec.ts-snapshots/vue-nodes-upload-widgets-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/widgets/widgetReactivity.spec.ts b/browser_tests/tests/vueNodes/widgets/widgetReactivity.spec.ts new file mode 100644 index 0000000000..6f3701c127 --- /dev/null +++ b/browser_tests/tests/vueNodes/widgets/widgetReactivity.spec.ts @@ -0,0 +1,51 @@ +import { + comfyExpect as expect, + comfyPageFixture as test +} from '../../../fixtures/ComfyPage' + +test.describe('Vue Widget Reactivity', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.setSetting('Comfy.VueNodes.Enabled', true) + await comfyPage.vueNodes.waitForNodes() + }) + test('Should display added widgets', async ({ comfyPage }) => { + const loadCheckpointNode = comfyPage.page.locator( + 'css=[data-testid="node-body-4"] > .lg-node-widgets > div' + ) + await comfyPage.page.evaluate(() => { + const node = window['graph']._nodes_by_id['4'] + node.widgets.push(node.widgets[0]) + }) + await expect(loadCheckpointNode).toHaveCount(2) + await comfyPage.page.evaluate(() => { + const node = window['graph']._nodes_by_id['4'] + node.widgets[2] = node.widgets[0] + }) + await expect(loadCheckpointNode).toHaveCount(3) + await comfyPage.page.evaluate(() => { + const node = window['graph']._nodes_by_id['4'] + node.widgets.splice(0, 0, node.widgets[0]) + }) + await expect(loadCheckpointNode).toHaveCount(4) + }) + test('Should hide removed widgets', async ({ comfyPage }) => { + const loadCheckpointNode = comfyPage.page.locator( + 'css=[data-testid="node-body-3"] > .lg-node-widgets > div' + ) + await comfyPage.page.evaluate(() => { + const node = window['graph']._nodes_by_id['3'] + node.widgets.pop() + }) + await expect(loadCheckpointNode).toHaveCount(5) + await comfyPage.page.evaluate(() => { + const node = window['graph']._nodes_by_id['3'] + node.widgets.length-- + }) + await expect(loadCheckpointNode).toHaveCount(4) + await comfyPage.page.evaluate(() => { + const node = window['graph']._nodes_by_id['3'] + node.widgets.splice(0, 1) + }) + await expect(loadCheckpointNode).toHaveCount(3) + }) +}) diff --git a/browser_tests/tests/widget.spec.ts-snapshots/load-audio-widget-chromium-linux.png b/browser_tests/tests/widget.spec.ts-snapshots/load-audio-widget-chromium-linux.png index fde5a2de99..89b8ae6f39 100644 Binary files a/browser_tests/tests/widget.spec.ts-snapshots/load-audio-widget-chromium-linux.png and b/browser_tests/tests/widget.spec.ts-snapshots/load-audio-widget-chromium-linux.png differ diff --git a/docs/PLAYWRIGHT_SELECTIVE_RERUN_ALTERNATIVES.md b/docs/PLAYWRIGHT_SELECTIVE_RERUN_ALTERNATIVES.md new file mode 100644 index 0000000000..0eb07eca00 --- /dev/null +++ b/docs/PLAYWRIGHT_SELECTIVE_RERUN_ALTERNATIVES.md @@ -0,0 +1,755 @@ +# Playwright Selective Test Rerun Alternatives + +This document analyzes alternatives for selectively re-running only failed Playwright tests for snapshot updates, comparing native Playwright features with the current custom manifest approach used in this project. + +## Table of Contents +- [Current Approach](#current-approach) +- [Native Playwright Features](#native-playwright-features) +- [Playwright Reporter Options](#playwright-reporter-options) +- [GitHub Actions Integration Patterns](#github-actions-integration-patterns) +- [Third-Party Solutions](#third-party-solutions) +- [Comparison and Recommendations](#comparison-and-recommendations) + +--- + +## Current Approach + +### Implementation +The project currently uses a **custom manifest-based approach** that: + +1. **Generates a manifest** of failed screenshot tests after CI runs + - Script: `scripts/cicd/build-failed-screenshot-manifest.ts` + - Parses JSON report to find tests with failed screenshot assertions + - Creates per-project text files: `ci-rerun/{project}.txt` + - Format: `file_path:line_number` (e.g., `browser_tests/menu.test.ts:42`) + +2. **Stores manifest as GitHub artifact** + - Artifact name: `failed-screenshot-tests` + - Retention: 7 days + - Only uploaded when chromium sharded tests fail + +3. **Downloads manifest in update workflow** + - Workflow: `.github/workflows/update-playwright-expectations.yaml` + - Triggered by: PR label "New Browser Test Expectations" or `/update-playwright` comment + - Falls back to full test suite if manifest not found + +4. **Re-runs only failed tests** + ```bash + for f in ci-rerun/*.txt; do + project="$(basename "$f" .txt)" + mapfile -t lines < "$f" + # Filter empty lines + pnpm exec playwright test --project="$project" --update-snapshots "${filtered[@]}" + done + ``` + +### Advantages +- ✅ Works across workflow runs and different trigger mechanisms +- ✅ Survives beyond single workflow execution +- ✅ Precise control over which tests to re-run +- ✅ Supports multiple projects with separate manifests +- ✅ Works with sharded test runs (merged report) +- ✅ Platform-agnostic approach (works on any CI/CD platform) + +### Disadvantages +- ❌ Custom implementation requires maintenance +- ❌ Requires parsing JSON report format (could break with Playwright updates) +- ❌ Additional artifact storage needed +- ❌ More complex than native solutions + +--- + +## Native Playwright Features + +### 1. `--last-failed` CLI Flag + +**Availability:** Playwright v1.44.0+ (May 2024) + +#### How It Works +```bash +# First run - execute all tests +npx playwright test + +# Second run - only re-run failed tests +npx playwright test --last-failed +``` + +Playwright maintains a `.last-run.json` file in the `test-results/` directory that tracks failed tests. + +#### CLI Examples +```bash +# Run only failed tests from last run +npx playwright test --last-failed + +# Update snapshots for only failed tests +npx playwright test --last-failed --update-snapshots + +# Combine with project filtering +npx playwright test --last-failed --project=chromium + +# Debug failed tests +npx playwright test --last-failed --debug +``` + +#### File Location and Format +- **Location:** `test-results/.last-run.json` +- **Format:** JSON object containing failed test information +- **Structure:** Contains a `failedTests: []` array with test identifiers +- **Persistence:** Cleared when all tests pass on subsequent run + +#### Advantages +- ✅ Built into Playwright (no custom code) +- ✅ Simple CLI flag +- ✅ Automatically maintained by Playwright +- ✅ Works with all Playwright features (debug, UI mode, etc.) + +#### Limitations +- ❌ **Not designed for CI/CD distributed testing** (per Playwright maintainers) +- ❌ **Intended for local development only** ("inner loop scenario") +- ❌ Cleared on new test runs (doesn't persist across clean environments) +- ❌ **GitHub Actions starts with clean environment** - `.last-run.json` not available on retry +- ❌ **Doesn't work with sharded tests** - each shard creates its own `.last-run.json` +- ❌ No native way to merge `.last-run.json` across shards +- ❌ Not designed for cross-workflow persistence + +#### CI/CD Workaround (Not Recommended) +To use `--last-failed` in GitHub Actions, you would need to: + +```yaml +- name: Run Playwright tests + id: playwright-test + run: npx playwright test + +- name: Upload last run state + if: failure() + uses: actions/upload-artifact@v4 + with: + name: last-run-state + path: test-results/.last-run.json + +# In retry workflow: +- name: Download last run state + uses: actions/download-artifact@v4 + with: + name: last-run-state + path: test-results/ + +- name: Rerun failed tests + run: npx playwright test --last-failed --update-snapshots +``` + +**Why This Isn't Ideal:** +- Playwright maintainers explicitly state this is not the intended use case +- Doesn't work well with sharded tests (multiple `.last-run.json` files) +- Requires manual artifact management +- More complex than the current custom approach for this use case + +### 2. File:Line Syntax for Specific Tests + +Playwright supports running tests at specific line numbers: + +```bash +# Run a specific test at line 42 +npx playwright test tests/example.spec.ts:42 + +# Multiple tests +npx playwright test tests/file1.spec.ts:10 tests/file2.spec.ts:25 + +# With snapshot updates +npx playwright test tests/example.spec.ts:42 --update-snapshots + +# With project selection +npx playwright test --project=chromium tests/example.spec.ts:42 +``` + +This is **exactly the format** the current custom manifest uses, making it compatible with Playwright's native CLI. + +### 3. Test Filtering Options + +```bash +# Filter by grep pattern +npx playwright test -g "screenshot" + +# Inverse grep +npx playwright test --grep-invert "mobile" + +# By project +npx playwright test --project=chromium + +# Multiple projects +npx playwright test --project=chromium --project=firefox + +# Specific directory +npx playwright test tests/screenshots/ +``` + +--- + +## Playwright Reporter Options + +### 1. JSON Reporter + +**Purpose:** Machine-readable test results + +#### Configuration +```typescript +// playwright.config.ts +export default defineConfig({ + reporter: [ + ['json', { outputFile: 'results.json' }] + ] +}) +``` + +Or via environment variable: +```bash +PLAYWRIGHT_JSON_OUTPUT_NAME=results.json npx playwright test --reporter=json +``` + +#### Output Structure +```json +{ + "stats": { + "expected": 100, + "unexpected": 5, + "flaky": 2, + "skipped": 3 + }, + "suites": [ + { + "title": "Test Suite", + "specs": [ + { + "file": "browser_tests/example.test.ts", + "line": 42, + "tests": [ + { + "projectId": "chromium", + "results": [ + { + "status": "failed", + "attachments": [ + { "contentType": "image/png" } + ] + } + ] + } + ] + } + ] + } + ] +} +``` + +**This is the format** the current `build-failed-screenshot-manifest.ts` script parses. + +#### Advantages +- ✅ Stable, documented JSON schema (`@playwright/test/reporter`) +- ✅ Includes all test metadata (file, line, project, status, attachments) +- ✅ Can be used programmatically +- ✅ Supports multiple reporters simultaneously + +#### Current Project Usage +```yaml +# In tests-ci.yaml +PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-report/report.json \ +pnpm exec playwright test --project=${{ matrix.browser }} \ + --reporter=list \ + --reporter=html \ + --reporter=json +``` + +### 2. Blob Reporter + +**Purpose:** Merging sharded test reports + +#### Configuration +```typescript +// playwright.config.ts +export default defineConfig({ + reporter: process.env.CI ? 'blob' : 'html' +}) +``` + +#### Usage with Sharding +```bash +# Run sharded test with blob output +npx playwright test --shard=1/4 --reporter=blob + +# Merge blob reports +npx playwright merge-reports --reporter=html ./all-blob-reports +npx playwright merge-reports --reporter=json ./all-blob-reports +``` + +#### Current Project Usage +```yaml +# Sharded chromium tests +- run: pnpm exec playwright test --project=chromium --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --reporter=blob + env: + PLAYWRIGHT_BLOB_OUTPUT_DIR: ../blob-report + +# Merge reports job +- run: | + pnpm exec playwright merge-reports --reporter=html ./all-blob-reports + PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-report/report.json \ + pnpm exec playwright merge-reports --reporter=json ./all-blob-reports +``` + +#### Advantages +- ✅ Designed for distributed testing +- ✅ Can merge into any reporter format (HTML, JSON, etc.) +- ✅ Preserves all test information across shards + +#### Blob Reporter and `--last-failed` +- ❌ Blob reports **do not contain** a merged `.last-run.json` +- ❌ Each shard creates its own `.last-run.json` that isn't included in blob +- ❌ GitHub issue [#30924](https://github.com/microsoft/playwright/issues/30924) requests this feature (currently unsupported) + +### 3. Multiple Reporters + +You can use multiple reporters simultaneously: + +```typescript +export default defineConfig({ + reporter: [ + ['list'], // Terminal output + ['html'], // Browse results + ['json', { outputFile: 'results.json' }], // Programmatic parsing + ['junit', { outputFile: 'results.xml' }] // CI integration + ] +}) +``` + +Or via CLI: +```bash +npx playwright test --reporter=list --reporter=html --reporter=json +``` + +--- + +## GitHub Actions Integration Patterns + +### Pattern 1: Comment-Triggered Workflow (JupyterLab Approach) + +**Example:** [jupyterlab/jupyterlab-git](https://github.com/jupyterlab/jupyterlab-git/blob/main/.github/workflows/update-integration-tests.yml) + +```yaml +name: Update Playwright Snapshots + +on: + issue_comment: + types: [created, edited] + +permissions: + contents: write + pull-requests: write + +jobs: + update-snapshots: + # Only run for authorized users on PRs with specific comment + if: > + (github.event.issue.author_association == 'OWNER' || + github.event.issue.author_association == 'COLLABORATOR' || + github.event.issue.author_association == 'MEMBER' + ) && github.event.issue.pull_request && + contains(github.event.comment.body, 'please update snapshots') + runs-on: ubuntu-latest + + steps: + - name: React to the triggering comment + run: gh api repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}/reactions --raw-field 'content=+1' + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Checkout + uses: actions/checkout@v4 + with: + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Checkout PR branch + run: gh pr checkout ${{ github.event.issue.number }} + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Setup and run tests + run: | + npm ci + npx playwright install --with-deps + npx playwright test --update-snapshots + + - name: Commit and push + run: | + git config user.name 'github-actions' + git config user.email 'github-actions@github.com' + git add . + git diff --cached --quiet || git commit -m "Update snapshots" + git push +``` + +#### Advantages +- ✅ Simple comment-based trigger +- ✅ Visual feedback (reaction on comment) +- ✅ Authorization checks built-in +- ✅ Auto-commits to PR branch + +#### Limitations +- ❌ Runs **all** tests with `--update-snapshots` (not selective) +- ❌ No integration with failed test information from CI + +### Pattern 2: Label-Based Trigger + Manifest (Current Approach) + +```yaml +name: Update Playwright Expectations + +on: + pull_request: + types: [labeled] + issue_comment: + types: [created] + +jobs: + test: + if: > + ( github.event_name == 'pull_request' && + github.event.label.name == 'New Browser Test Expectations' ) || + ( github.event.issue.pull_request && + startsWith(github.event.comment.body, '/update-playwright') ) + + steps: + # ... setup steps ... + + - name: Locate failed screenshot manifest artifact + id: locate-manifest + uses: actions/github-script@v8 + with: + script: | + const { owner, repo } = context.repo + let headSha = '' + if (context.eventName === 'pull_request') { + headSha = context.payload.pull_request.head.sha + } else if (context.eventName === 'issue_comment') { + const prNumber = context.payload.issue.number + const pr = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }) + headSha = pr.data.head.sha + } + + const { data } = await github.rest.actions.listWorkflowRuns({ + owner, repo, + workflow_id: 'tests-ci.yaml', + head_sha: headSha, + per_page: 1, + }) + const run = data.workflow_runs?.[0] + + let has = 'false' + if (run) { + const { data: { artifacts = [] } } = await github.rest.actions.listWorkflowRunArtifacts({ + owner, repo, run_id: run.id + }) + if (artifacts.some(a => a.name === 'failed-screenshot-tests' && !a.expired)) + has = 'true' + } + core.setOutput('has_manifest', has) + + - name: Download failed screenshot manifest + if: steps.locate-manifest.outputs.has_manifest == 'true' + uses: actions/download-artifact@v4 + with: + run-id: ${{ steps.locate-manifest.outputs.run_id }} + name: failed-screenshot-tests + path: ComfyUI_frontend/ci-rerun + + - name: Re-run failed screenshot tests + run: | + if [ ! -d ci-rerun ]; then + echo "No manifest found; running full suite" + pnpm exec playwright test --update-snapshots + exit 0 + fi + + for f in ci-rerun/*.txt; do + project="$(basename "$f" .txt)" + mapfile -t lines < "$f" + filtered=() + for l in "${lines[@]}"; do + [ -n "$l" ] && filtered+=("$l") + done + + if [ ${#filtered[@]} -gt 0 ]; then + echo "Re-running ${#filtered[@]} tests for project $project" + pnpm exec playwright test --project="$project" --update-snapshots "${filtered[@]}" + fi + done +``` + +#### Advantages +- ✅ **Selective** - only re-runs failed screenshot tests +- ✅ Works across different trigger mechanisms (label or comment) +- ✅ Fallback to full suite if manifest not found +- ✅ Per-project manifests support multiple browser configurations +- ✅ Handles sharded tests via merged report + +### Pattern 3: WordPress/Openverse Approach (Always Update) + +Proposed pattern (not fully implemented): +1. CI always runs with `--update-snapshots` flag +2. If snapshots change, create/update a secondary branch +3. Open PR targeting the original PR branch +4. Developer reviews snapshot changes before merging + +#### Advantages +- ✅ Always generates correct snapshots +- ✅ Snapshot changes are visible in separate PR +- ✅ No test failures due to mismatched snapshots + +#### Limitations +- ❌ Creates multiple PRs +- ❌ More complex merge workflow +- ❌ Potential for snapshot changes to mask real issues + +### Pattern 4: Manual Workflow Dispatch + +```yaml +name: Update Snapshots + +on: + workflow_dispatch: + inputs: + update-snapshots: + description: 'Update snapshots' + type: boolean + default: false + test-pattern: + description: 'Test pattern (optional)' + type: string + required: false + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup + run: | + npm ci + npx playwright install --with-deps + + - name: Run tests + run: | + if [ "${{ inputs.update-snapshots }}" = "true" ]; then + FLAGS="--update-snapshots" + fi + + PATTERN="${{ inputs.test-pattern }}" + npx playwright test ${PATTERN} ${FLAGS} +``` + +#### Advantages +- ✅ Full manual control +- ✅ Can specify test patterns +- ✅ Simple to understand + +#### Limitations +- ❌ Requires manual triggering +- ❌ Not integrated with CI failures + +--- + +## Third-Party Solutions + +### Currents.dev - Last Failed GitHub Action + +**Repository:** [currents-dev/playwright-last-failed](https://github.com/currents-dev/playwright-last-failed) + +#### Purpose +Helps run last failed Playwright tests using Currents' cloud-based caching service. + +#### Usage +```yaml +- name: Playwright Last Failed action + id: last-failed-action + uses: currents-dev/playwright-last-failed@v1 + with: + pw-output-dir: test-results + matrix-index: ${{ matrix.shard }} + matrix-total: ${{ strategy.job-total }} +``` + +#### How It Works +- Uses Currents' cloud service to persist failed test information +- Supports sharded tests via matrix parameters +- Enables selective rerun of failed tests across workflow retries + +#### Advantages +- ✅ Works with sharded tests +- ✅ Persists across workflow runs +- ✅ Supports GitHub Actions retry mechanism +- ✅ Handles distributed testing + +#### Limitations +- ❌ **Requires Currents subscription** (third-party paid service) +- ❌ Dependency on external service +- ❌ Data sent to third-party cloud +- ❌ Additional cost +- ❌ Vendor lock-in + +#### Recommendation +**Not suitable for this project** due to: +- External service dependency +- Cost implications +- The current custom solution is already working well + +--- + +## Comparison and Recommendations + +### Feature Matrix + +| Feature | Current Approach | `--last-failed` | Currents | Comment Trigger Only | +|---------|-----------------|-----------------|----------|---------------------| +| Works with sharded tests | ✅ Yes | ❌ No | ✅ Yes | ✅ Yes | +| Persists across workflows | ✅ Yes | ❌ No | ✅ Yes | N/A | +| Selective reruns | ✅ Yes | ✅ Yes | ✅ Yes | ❌ No (runs all) | +| No external dependencies | ✅ Yes | ✅ Yes | ❌ No | ✅ Yes | +| Simple implementation | ⚠️ Medium | ✅ Simple | ✅ Simple | ✅ Simple | +| Maintenance overhead | ⚠️ Medium | ✅ Low | ✅ Low | ✅ Low | +| Works in CI/CD | ✅ Yes | ⚠️ Workaround | ✅ Yes | ✅ Yes | +| Cost | ✅ Free | ✅ Free | ❌ Paid | ✅ Free | +| Supports multiple projects | ✅ Yes | ✅ Yes | ✅ Yes | ✅ Yes | + +### Why `--last-failed` Isn't Suitable (Currently) + +1. **Not designed for CI/CD:** Playwright maintainers explicitly state it's for "inner loop scenario (local development)" +2. **Doesn't work with sharded tests:** Each shard creates its own `.last-run.json` with no native merge +3. **Clean environment issue:** GitHub Actions starts fresh, losing `.last-run.json` +4. **Feature request pending:** GitHub issue [#30924](https://github.com/microsoft/playwright/issues/30924) requests blob report integration (not yet implemented) + +### Recommendations + +#### Short Term: Keep Current Approach +**Verdict: The current custom manifest approach is the best solution for this project's needs.** + +**Reasons:** +1. ✅ **Works perfectly with sharded tests** - merges results across 8 shards +2. ✅ **Persists across workflows** - artifact storage for 7 days +3. ✅ **Selective reruns** - only failed screenshot tests +4. ✅ **No external dependencies** - fully self-contained +5. ✅ **Uses stable Playwright JSON format** - typed via `@playwright/test/reporter` +6. ✅ **Already working well** - proven in production + +**Minor Improvements:** +```typescript +// Add version check to warn if JSON schema changes +import { version } from '@playwright/test/package.json' +if (major(version) !== 1) { + console.warn('Playwright major version changed - verify JSON schema compatibility') +} + +// Add more robust error handling +try { + const report: JSONReport = JSON.parse(raw) +} catch (error) { + throw new Error(`Failed to parse Playwright JSON report: ${error.message}`) +} + +// Consider adding tests for the manifest builder +// e.g., tests/cicd/build-failed-screenshot-manifest.test.ts +``` + +#### Long Term: Monitor Playwright Development + +**Watch for these features:** +1. **Blob report + `.last-run.json` merge** - GitHub issue [#30924](https://github.com/microsoft/playwright/issues/30924) +2. **Native CI/CD support for `--last-failed`** - may never happen (by design) +3. **Report merging improvements** - GitHub issue [#33094](https://github.com/microsoft/playwright/issues/33094) + +**Migration path if native support improves:** +```yaml +# Future potential approach (if Playwright adds this feature) +- name: Merge reports with last-run + run: | + npx playwright merge-reports --reporter=html ./all-blob-reports + npx playwright merge-reports --reporter=last-failed ./all-blob-reports + +- name: Upload merged last-run + uses: actions/upload-artifact@v4 + with: + name: last-run-state + path: test-results/.last-run.json + +# In update workflow +- name: Download last-run state + uses: actions/download-artifact@v4 + with: + name: last-run-state + path: test-results/ + +- name: Update snapshots for failed tests + run: npx playwright test --last-failed --update-snapshots +``` + +**However, this is speculative** - Playwright maintainers have indicated `--last-failed` is not intended for CI/CD. + +#### Alternative: Simplify to Full Suite Reruns + +If the custom manifest becomes too complex to maintain, consider: + +```yaml +- name: Re-run ALL screenshot tests + run: | + # Simple grep-based filtering for screenshot tests + npx playwright test -g "screenshot" --update-snapshots +``` + +**Trade-offs:** +- ✅ Much simpler +- ✅ No custom scripts +- ❌ Slower (runs all screenshot tests, not just failed ones) +- ❌ Potentially updates snapshots that weren't actually failing + +--- + +## Conclusion + +The current custom manifest approach is **well-designed** and **appropriate** for this project's requirements: + +1. **Handles sharded tests** - critical for CI performance +2. **Selective reruns** - saves time and resources +3. **Stable implementation** - uses documented Playwright JSON schema +4. **No external dependencies** - fully controlled + +While `--last-failed` is a nice feature for **local development**, Playwright's own documentation and maintainer comments confirm it's **not suitable for distributed CI/CD testing**, which is exactly what this project needs. + +The only potentially better solution (Currents) requires a paid external service, which adds cost and complexity without significant benefits over the current approach. + +**Recommendation: Keep the current implementation**, with minor improvements to error handling and documentation. Monitor Playwright development for native improvements, but don't expect `--last-failed` to become a viable alternative for this use case. + +--- + +## References + +### Official Playwright Documentation +- [Command Line](https://playwright.dev/docs/test-cli) +- [Reporters](https://playwright.dev/docs/test-reporters) +- [Test Sharding](https://playwright.dev/docs/test-sharding) +- [CI/CD Setup](https://playwright.dev/docs/ci-intro) + +### Community Resources +- [Playwright Solutions: How to Run Failures Only](https://playwrightsolutions.com/how-to-run-failures-only-from-the-last-playwright-run/) +- [Medium: How to Run Only Last Failed Tests](https://medium.com/@testerstalk/how-to-run-only-last-failed-tests-in-playwright-e5e41472594a) +- [Medium: Streamlining Visual Regression Testing](https://medium.com/@haleywardo/streamlining-playwright-visual-regression-testing-with-github-actions-e077fd33c27c) + +### GitHub Issues +- [#30924 - Last-failed with blob reports](https://github.com/microsoft/playwright/issues/30924) +- [#33094 - Merging main run with --last-failed](https://github.com/microsoft/playwright/issues/33094) +- [#28254 - Feature request for --last-failed](https://github.com/microsoft/playwright/issues/28254) + +### Example Implementations +- [JupyterLab Git - Update Integration Tests](https://github.com/jupyterlab/jupyterlab-git/blob/main/.github/workflows/update-integration-tests.yml) +- [WordPress Openverse - Discussion #4535](https://github.com/WordPress/openverse/issues/4535) + +### Third-Party Tools +- [Currents - Playwright Last Failed Action](https://github.com/currents-dev/playwright-last-failed) +- [Currents - Re-run Only Failed Tests](https://docs.currents.dev/guides/re-run-only-failed-tests) diff --git a/docs/SNAPSHOT_UPDATE_FROM_ACTUALS.md b/docs/SNAPSHOT_UPDATE_FROM_ACTUALS.md new file mode 100644 index 0000000000..603f5b6cc3 --- /dev/null +++ b/docs/SNAPSHOT_UPDATE_FROM_ACTUALS.md @@ -0,0 +1,482 @@ +# Snapshot Update from Actual Files (Fast Approach) + +**Date:** 2025-10-08 +**Status:** Proposed Optimization + +## Overview + +When Playwright snapshot tests fail, Playwright **already generates the new ("actual") snapshots**. Instead of re-running tests with `--update-snapshots`, we can extract these actual snapshots from the `test-results/` directory and copy them to overwrite the expected snapshots. + +**Performance improvement:** ~1-2 minutes → **~10-30 seconds** + +## How Playwright Stores Snapshots + +### Expected (Baseline) Snapshots + +Stored in: `