Fix dialog support test to expect 'oss-nightly' in CI #8338
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:XS
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/29/2026, 02:27:09 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests:
|
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (23)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
📝 WalkthroughWalkthroughAdd IS_NIGHTLY=true to CI/workflow steps and local test scripts, update a test expectation to use the Changes
Sequence Diagram(s)(omitted — changes are configuration and test expectation updates plus a minor import refactor; no new multi-component control flow requiring diagrams) Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Myestery
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 25.9 kB (baseline 25.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 973 kB (baseline 973 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 471 kB (baseline 471 kB) • 🟢 -8 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.89 kB (baseline 2.89 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • ⚪ 0 BReusable component library chunks
Status: 4 added / 4 removed Data & Services — 2.7 MB (baseline 2.7 MB) • 🔴 +1 BStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 25.3 kB (baseline 25.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.05 MB (baseline 7.05 MB) • 🟢 -198 BBundles that do not match a named category
Status: 34 added / 34 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@browser_tests/tests/dialog.spec.ts`:
- Around line 322-324: The test currently hardcodes 'oss-nightly' when asserting
the tf_42243568391700 search param, which fails outside CI; change the assertion
to compute the expected tag using the same environment/branch logic the app uses
(rather than a literal string). In dialog.spec.ts locate the code using
newPage.url() and url.searchParams.get('tf_42243568391700') and replace the
right-hand side with a derived expectedTag (for example by importing or reusing
the app's tag-determination helper or by reading the same CI/branch environment
variables the app inspects) and then assert
expect(url.searchParams.get('tf_42243568391700')).toBe(expectedTag).
github-actions
bot
removed
the
New Browser Test Expectations
- Test both nightly and stable build scenarios - Mock __IS_NIGHTLY__ global to true/false before page loads - Expect 'oss-nightly' when IS_NIGHTLY is true - Expect 'oss' when IS_NIGHTLY is false - Uses Object.defineProperty to properly set the global value
Myestery
force-pushed
the
fix-nigthly-test
branch
from
6bf3aec to
4b2804a
Compare
dosubot
bot
added
size:M
coderabbitai
bot
requested review from
Yorha4D,
christian-byrne and
shinshin86
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@browser_tests/tests/dialog.spec.ts`:
- Around line 341-369: The two tests duplicate the same flow (mocking
__IS_NIGHTLY__, opening Support via comfyPage.menu.topbar.triggerTopbarCommand,
waiting for new page, and asserting the tf_42243568391700 search param) so
extract/parameterize that logic: create a table of cases (e.g. {name, isNightly,
expectedTag}), loop to generate a test.describe for each case, move the
page.addInitScript into the case-specific test.beforeEach passing isNightly as
an argument, and reuse a single test body that calls comfyPage.setSetting, waits
for the page event, asserts URL contains support.comfy.org and that
url.searchParams.get('tf_42243568391700') equals expectedTag; this removes
duplication while keeping unique test names and behaviors.
- Add viewport clipping to mobile baseline tests to reduce flakiness - Clip top 15% of viewport for all @mobile screenshot tests - Affects: - mobileBaseline.spec.ts: empty canvas and default workflow tests - pan.spec.ts: touch pan test - move.spec.ts: touch node move test - This helps reduce test flakiness from variable mobile UI elements
dosubot
bot
added
size:L
Myestery
added
the
New Browser Test Expectations
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@browser_tests/tests/mobileBaseline.spec.ts`:
- Around line 32-41: Duplicate clipping logic (viewportSize/clipRegion computed
twice) — extract the logic into a small helper (e.g., computeClipRegion or
getClipRegion) that calls comfyPage.page.viewportSize() and returns the same
clipRegion shape ({x, y, width, height} | undefined) and replace both inline
blocks with calls to that helper; alternatively import and use the shared helper
if it exists. Ensure the helper uses Math.floor/Math.ceil exactly as in the
original and name it so callers reference it clearly (computeClipRegion or
getClipRegion) to remove the duplication.
In `@browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts`:
- Around line 15-24: Extract the duplicated viewport clipping calculation into a
shared helper (e.g., getMobileClipRegion) that accepts a Playwright Page-like
object (or comfyPage) and returns the clip region type { x:number; y:number;
width:number; height:number } | undefined by calling page.viewportSize();
replace the inline logic in pan.spec.ts, mobileBaseline.spec.ts, and
move.spec.ts with calls to that helper so all tests reuse the same
Math.floor/Math.ceil 0.15/0.85 clipping math and keep behavior consistent.
- Set IS_NIGHTLY=true for all Playwright test runs - Update CI workflows to pass IS_NIGHTLY=true to test commands - Update package.json test:browser scripts to include IS_NIGHTLY=true - Update dialog test to expect 'oss-nightly' instead of 'oss' - Also update playwright expectations workflow to use IS_NIGHTLY=true This ensures consistent test behavior regardless of where tests run, since Playwright tests now always run with IS_NIGHTLY=true.
Myestery
force-pushed
the
fix-nigthly-test
branch
from
3fd2122 to
664be8f
Compare
dosubot
bot
added
size:S
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
DrJKL
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
The test was failing because the frontend was built without IS_NIGHTLY=true, so the compiled code had __IS_NIGHTLY__ set to false. Setting IS_NIGHTLY=true during test runtime didn't affect the already-compiled code. This fix ensures the frontend is built with IS_NIGHTLY=true during the build phase in the CI workflows, so the compiled code will have the correct value.
Myestery
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Configure Playwright tests to always run as nightly builds by setting
IS_NIGHTLY=trueenvironment variable.Changes
Set
IS_NIGHTLY=truefor all Playwright test runs:ci-tests-e2e.yamlandpr-update-playwright-expectations.yaml) to passIS_NIGHTLY=truepackage.jsontest:browser scripts to includeIS_NIGHTLY=trueUpdated dialog test expectation:
'oss'to'oss-nightly'inbrowser_tests/tests/dialog.spec.tsWhy this approach?
Context
As defined in
vite.config.mts:59, theIS_NIGHTLYflag is set totruewhen running in CI on the main branch.By setting
IS_NIGHTLY=truefor all test runs, we ensure consistent test behavior regardless of where they run.Test plan