Skip to content

Conversation

christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Sep 26, 2025

Summary

  • Regenerate 3 Playwright screenshot baselines to reflect UI changes from New Workflow Templates Modal #5142
  • Also fixed this case (test case for responsive sizing) as it was using outdated logic. New logic: ensures that the nav is collapsed on mobile but visible on tablet and desktop screen sizes. It also ensures that, at all the main breakpoints, at least 1 card is visible (covers bug that the case was originally written for wherein the nav was fully covering the cards at narrow screen widths).

┆Issue is synchronized with this Notion page by Unito

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Sep 26, 2025
Copy link

github-actions bot commented Sep 26, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 09/26/2025, 08:05:37 PM UTC

📈 Summary

  • Total Tests: 464
  • Passed: 434 ✅
  • Failed: 0
  • Flaky: 1 ⚠️
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 427 / ❌ 0 / ⚠️ 1 / ⏭️ 29
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

@christian-byrne christian-byrne added area:testing New Browser Test Expectations New browser test screenshot should be set by github action labels Sep 26, 2025
@christian-byrne
Copy link
Contributor Author

christian-byrne commented Sep 26, 2025

Changes:

  • Fixed this case (test case for responsive sizing) as it was using outdated logic. New logic: ensures that the nav is collapsed on mobile but visible on tablet and desktop screen sizes. It also ensures that, at all the main breakpoints, at least 1 card is visible (covers bug that the case was originally written for wherein the nav was fully covering the cards at narrow screen widths).

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Sep 26, 2025
@christian-byrne christian-byrne changed the title [ci] Regenerate Playwright screenshot baselines [ci] Update browser tests for new Templates modal Sep 26, 2025
Copy link
Collaborator

@Myestery Myestery left a comment

Choose a reason for hiding this comment

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

LGTM!

// Open templates dialog
await comfyPage.executeCommand('Comfy.BrowseTemplates')
await expect(comfyPage.templates.content).toBeVisible()

Copy link
Collaborator

Choose a reason for hiding this comment

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

These tests will make sense more on the base modal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test case was originally about ensuring the grid was visible. The bug from a long time ago was the nav bar fully covering the grid. I think checking the template base modal visibility is covered in other test cases, so it should error rather than fail expectation in this case if not visible.

@christian-byrne christian-byrne merged commit 5f7c7ca into main Sep 26, 2025
29 checks passed
@christian-byrne christian-byrne deleted the regenerate-playwright-screenshots-20250926-115310 branch September 26, 2025 20:10
christian-byrne added a commit that referenced this pull request Sep 27, 2025
Fixes flaky templates modal test introduced in
#5802 by ensuring the
templates modal is visible before querying the visible card count and
asserting that it is greater than 0.

If we immediately count the number of cards after executing the "load
templates" command, it's possible that there are 0 visible due to them
loading (rather than being caused by a legitimate bug).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5808-fix-flaky-templates-browser-test-27b6d73d365081e58c65f608944976a0)
by [Unito](https://www.unito.io)
christian-byrne added a commit that referenced this pull request Sep 27, 2025
## Summary
- Regenerate 3 Playwright screenshot baselines to reflect UI changes
from #5142
- Also fixed [this
case](https://f01efc75.comfyui-playwright-chromium.pages.dev/#?testId=35f0453d615a452757ca-379124415c5b7e9060d2)
(test case for responsive sizing) as it was using outdated logic. New
logic: ensures that the nav is collapsed on mobile but visible on tablet
and desktop screen sizes. It also ensures that, at all the main
breakpoints, at least 1 card is visible (covers bug that the case was
originally written for wherein the nav was fully covering the cards at
narrow screen widths).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5802-ci-Regenerate-Playwright-screenshot-baselines-27a6d73d365081768211da0d24bad2c3)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <[email protected]>
christian-byrne added a commit that referenced this pull request Sep 27, 2025
Fixes flaky templates modal test introduced in
#5802 by ensuring the
templates modal is visible before querying the visible card count and
asserting that it is greater than 0.

If we immediately count the number of cards after executing the "load
templates" command, it's possible that there are 0 visible due to them
loading (rather than being caused by a legitimate bug).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5808-fix-flaky-templates-browser-test-27b6d73d365081e58c65f608944976a0)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:templates area:testing New Browser Test Expectations New browser test screenshot should be set by github action size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants