Skip to content

Conversation

@Myestery
Copy link
Collaborator

@Myestery Myestery commented Sep 3, 2025

This pull request updates the UI test workflow to improve scalability and reporting for Playwright tests. The main changes introduce sharding for Chromium-based tests, optimize dependency installation and caching, and add a new job to merge test reports for easier review.

Test sharding and reporting improvements:

  • Added a new job playwright-tests-chromium-sharded to run Chromium Playwright tests in parallel shards, increasing test speed and scalability. The matrix now uses shardIndex and shardTotal instead of individual browser variants.
  • Introduced a merge-reports job that collects and merges the results from all Chromium shards into a single HTML report artifact for easier review.

Dependency installation and caching enhancements:

  • Added explicit steps to install Python requirements (including PyTorch and wait-for-it), and to start the ComfyUI server before running tests, ensuring the test environment is fully prepared.
  • Improved Playwright browser caching by adding a dedicated cache step for browser binaries, reducing redundant downloads in CI runs.

Workflow maintenance:

  • Removed an unused cache step from the setup job, streamlining the workflow.## Summary

Changes

  • What:
  • Breaking:
  • Dependencies:

Review Focus

Screenshots (if applicable)

┆Issue is synchronized with this Notion page by Unito

@github-actions
Copy link

github-actions bot commented Sep 3, 2025

🎭 Playwright Test Results

All tests passed across all browsers!

⏰ Completed at: 09/03/2025, 03:20:59 PM UTC

📊 Test Reports by Browser


🎉 Your tests are passing across all browsers!

@Myestery Myestery force-pushed the playwright-sharding branch from c2fdbd2 to 52453ad Compare September 3, 2025 04:05
@Myestery Myestery marked this pull request as ready for review September 3, 2025 15:12
@Myestery Myestery requested a review from a team as a code owner September 3, 2025 15:12
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 3, 2025
Copy link
Contributor

@DrJKL DrJKL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦔 I love it. I hate waiting for these to finish so much.

Comment on lines +125 to +132
- name: Cache Playwright browsers
uses: actions/cache@v4
with:
path: ~/.cache/ms-playwright
key: playwright-browsers-${{ runner.os }}-${{ hashFiles('ComfyUI_frontend/pnpm-lock.yaml') }}-chromium
restore-keys: |
playwright-browsers-${{ runner.os }}-${{ hashFiles('ComfyUI_frontend/pnpm-lock.yaml') }}-
playwright-browsers-${{ runner.os }}-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official recommendation is to not cache the browsers: https://playwright.dev/docs/ci#caching-browsers

@snomiao
Copy link
Member

snomiao commented Sep 3, 2025

LGTM, love simple solution

@christian-byrne
Copy link
Contributor

I will merge now so I can time it with BP changes. Please address the review comments in a followup where necessary.

@christian-byrne christian-byrne merged commit caee383 into main Sep 3, 2025
22 checks passed
@christian-byrne christian-byrne deleted the playwright-sharding branch September 3, 2025 19:16

playwright-tests:
# Sharded chromium tests
playwright-tests-chromium-sharded:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://playwright.dev/docs/test-sharding#github-actions-example

We should add a timeout for the upper limit of test run time. Hitting the timeout can be a signal that we need to increase shard totals or rethink our browser test suite.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also add a comment explaining this if we do it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true

@benceruleanlu benceruleanlu mentioned this pull request Sep 4, 2025
snomiao pushed a commit that referenced this pull request Sep 12, 2025
* [feat] Enhance Playwright testing workflow with sharding and report merging

* feat: shard playwright tests into 8 processes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants