fix: Stabilize flaky link visibility toggle test #5447
Closed
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
🎭 Playwright Test Results⏰ Completed at: 09/16/2025, 01:16:22 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
The test was flaky because it wasn't properly waiting for the canvas to update after changing the link render mode setting. The setting change triggers a canvas redraw via setDirty(), but the actual redraw happens asynchronously. Changes: - Add explicit wait for canvas.links_render_mode to update - Capture initial link render mode to properly verify state changes - Add double frame wait plus 100ms delay to ensure canvas redraw completes - Improve wait condition for visible links to handle undefined initial state - Add timeout protection to prevent test hanging This ensures the test reliably waits for both the setting change and the visual update before taking screenshots, eliminating the race condition that was causing intermittent failures.
snomiao
force-pushed
the
sno-fix-flaky-toggle-link-visibility
branch
from
107af0a to
f66afd7
Compare
5 tasks
The test was flaky because it wasn't properly waiting for the canvas to update after changing the link render mode setting. The setting change triggers a canvas redraw via setDirty(), but the actual redraw happens asynchronously. Changes: - Add explicit wait for canvas.links_render_mode to update - Capture initial link render mode to properly verify state changes - Add double frame wait plus 100ms delay to ensure canvas redraw completes - Improve wait condition for visible links to handle undefined initial state - Add timeout protection to prevent test hanging This ensures the test reliably waits for both the setting change and the visual update before taking screenshots, eliminating the race condition that was causing intermittent failures.
snomiao
force-pushed
the
sno-fix-flaky-toggle-link-visibility
branch
from
f66afd7 to
a6f3709
Compare
…fy-Org/ComfyUI_frontend into sno-fix-flaky-toggle-link-visibility
Replace fixed delays with frame counter monitoring to ensure canvas has completed rendering before taking screenshots. This eliminates race conditions and makes the test more reliable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
|
problem prob solved by waiting for link_hidden status by page.evaluate but this impl is a bit too hacky. |
Contributor
|
This should be fixed on |
Member
Author
|
close this PR as already fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR fixes a flaky test in the graph canvas menu that was intermittently failing in CI. The test was checking link visibility toggle functionality but wasn't properly waiting for the canvas to update after changing settings.
Problem
The test
Can toggle link visibilitywas flaky because:nextFrame()The issue was that changing the
Comfy.LinkRenderModesetting triggers an async canvas redraw viasetDirty(), but the actual redraw might not complete within a single frame.Solution
Added proper synchronization:
waitForFunctionto wait untilcanvas.links_render_modeactually changesnextFrame()twice to ensure the canvas redraw completesTesting
The fix ensures that:
This should eliminate the race condition that was causing intermittent failures.
Related Code
The link visibility toggle is handled by:
Comfy.Canvas.ToggleLinkVisibilityinuseCoreCommands.tsuseLitegraphSettings.tswatchesComfy.LinkRenderModeand updates canvascanvas.setDirty(false, true)to trigger background redrawFixes the flaky test issue reported in CI.
┆Issue is synchronized with this Notion page by Unito