mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
refactor: address PR review feedback
- Extract isWidgetPromoted into promotionUtils.ts for shared use - Use enterSubgraph utility in E2E test instead of manual page.evaluate - Use expect.poll instead of comfyExpect().toPass() for single assertion - Assert exact promoted ring count (4) instead of > 0 - Switch unit test from VTU mount to VTL render/screen
This commit is contained in:
@@ -496,33 +496,25 @@ test.describe('Subgraph Nested Scenarios', { tag: ['@subgraph'] }, () => {
|
||||
const outerRings = outerNode.locator(`.${PROMOTED_BORDER_CLASS}`)
|
||||
await comfyExpect(outerRings).toHaveCount(0)
|
||||
|
||||
// Navigate into Sub 0 programmatically to avoid click interception
|
||||
// from canvas overlay elements (z-999 layer).
|
||||
await comfyPage.page.evaluate(() => {
|
||||
const graph = window.app!.graph!
|
||||
const node = graph.getNodeById('5')
|
||||
if (node?.isSubgraphNode()) {
|
||||
window.app!.canvas.setGraph(node.subgraph)
|
||||
}
|
||||
})
|
||||
await comfyPage.nextFrame()
|
||||
await comfyPage.vueNodes.enterSubgraph('5')
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
|
||||
// Node 6 (Sub 1) has widgets promoted up to Sub 0 (node 5).
|
||||
// At least one widget should carry the promoted ring.
|
||||
// Node 6 (Sub 1) has 4 proxyWidgets (string_a, value, value,
|
||||
// string_a) promoted up to Sub 0 (node 5). Each should carry the
|
||||
// promoted ring.
|
||||
const intermediateNode = comfyPage.vueNodes.getNodeLocator('6')
|
||||
await comfyExpect(intermediateNode).toBeVisible()
|
||||
|
||||
const intermediateRings = intermediateNode.locator(
|
||||
`.${PROMOTED_BORDER_CLASS}`
|
||||
)
|
||||
await comfyExpect(async () => {
|
||||
const count = await intermediateRings.count()
|
||||
expect(
|
||||
count,
|
||||
'Node 6 (Sub 1) should have at least one promoted ring'
|
||||
).toBeGreaterThan(0)
|
||||
}).toPass({ timeout: 5000 })
|
||||
await expect
|
||||
.poll(() => intermediateRings.count(), {
|
||||
timeout: 5000,
|
||||
message:
|
||||
'Node 6 (Sub 1) should show promoted rings for all 4 proxyWidgets'
|
||||
})
|
||||
.toBe(4)
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { fromAny } from '@total-typescript/shoehorn'
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { render, screen } from '@testing-library/vue'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { nextTick } from 'vue'
|
||||
import { defineComponent, h } from 'vue'
|
||||
import { describe, expect, test, vi } from 'vitest'
|
||||
|
||||
import type {
|
||||
@@ -24,6 +23,28 @@ vi.mock('@/renderer/core/canvas/canvasStore', () => ({
|
||||
})
|
||||
}))
|
||||
|
||||
const PROMOTED_CLASS = 'ring-component-node-widget-promoted'
|
||||
|
||||
const WidgetStub = defineComponent({
|
||||
props: { widget: { type: Object, default: undefined } },
|
||||
setup(props) {
|
||||
return () =>
|
||||
h('div', {
|
||||
'data-testid': 'widget-stub',
|
||||
class: props.widget?.borderStyle
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock(
|
||||
'@/renderer/extensions/vueNodes/widgets/registry/widgetRegistry',
|
||||
() => ({
|
||||
getComponent: () => WidgetStub,
|
||||
shouldExpand: () => false,
|
||||
shouldRenderAsVue: () => true
|
||||
})
|
||||
)
|
||||
|
||||
function createMockWidget(
|
||||
overrides: Partial<SafeWidgetData> = {}
|
||||
): SafeWidgetData {
|
||||
@@ -58,12 +79,12 @@ function createMockNodeData(
|
||||
}
|
||||
}
|
||||
|
||||
function mountComponent(nodeData: VueNodeData, setupStores?: () => void) {
|
||||
function renderComponent(nodeData: VueNodeData, setupStores?: () => void) {
|
||||
const pinia = createTestingPinia({ stubActions: false })
|
||||
setActivePinia(pinia)
|
||||
setupStores?.()
|
||||
|
||||
return mount(NodeWidgets, {
|
||||
return render(NodeWidgets, {
|
||||
props: { nodeData },
|
||||
global: {
|
||||
plugins: [pinia],
|
||||
@@ -73,15 +94,6 @@ function mountComponent(nodeData: VueNodeData, setupStores?: () => void) {
|
||||
})
|
||||
}
|
||||
|
||||
function getBorderStyles(wrapper: ReturnType<typeof mount>) {
|
||||
return fromAny<{ processedWidgets: unknown[] }, unknown>(
|
||||
wrapper.vm
|
||||
).processedWidgets.map(
|
||||
(entry) =>
|
||||
(entry as { simplified: { borderStyle?: string } }).simplified.borderStyle
|
||||
)
|
||||
}
|
||||
|
||||
describe('promoted widget indicator on nested subgraphs', () => {
|
||||
test('shows promoted ring when promotion was stored without disambiguatingSourceNodeId', async () => {
|
||||
// Scenario: SubBNode (id=3) inside SubA promotes a widget from
|
||||
@@ -102,7 +114,7 @@ describe('promoted widget indicator on nested subgraphs', () => {
|
||||
slotName: 'text'
|
||||
})
|
||||
const nodeData = createMockNodeData('SubgraphNode', [promotedWidget], '3')
|
||||
const wrapper = mountComponent(nodeData, () => {
|
||||
renderComponent(nodeData, () => {
|
||||
// Store promotion WITHOUT disambiguatingSourceNodeId, as would
|
||||
// happen for a first-level nested promotion where the inner node
|
||||
// is not itself a SubgraphNode.
|
||||
@@ -111,9 +123,11 @@ describe('promoted widget indicator on nested subgraphs', () => {
|
||||
sourceWidgetName: 'text'
|
||||
})
|
||||
})
|
||||
await nextTick()
|
||||
const borderStyles = getBorderStyles(wrapper)
|
||||
|
||||
expect(borderStyles.some((style) => style?.includes('promoted'))).toBe(true)
|
||||
const widgets = screen.getAllByTestId('widget-stub')
|
||||
const hasPromotedRing = widgets.some((el) =>
|
||||
el.classList.contains(PROMOTED_CLASS)
|
||||
)
|
||||
expect(hasPromotedRing).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -27,6 +27,30 @@ export function getWidgetName(w: IBaseWidget): string {
|
||||
return isPromotedWidgetView(w) ? w.sourceWidgetName : w.name
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a widget is promoted by any subgraph node in the given graph.
|
||||
* Handles nested subgraph promotions where the stored key may omit the
|
||||
* disambiguatingSourceNodeId — checks both key shapes (with and without).
|
||||
*/
|
||||
export function isWidgetPromoted(
|
||||
graphId: string,
|
||||
sourceNodeId: string,
|
||||
sourceWidgetName: string,
|
||||
disambiguatingSourceNodeId?: string
|
||||
): boolean {
|
||||
const store = usePromotionStore()
|
||||
if (
|
||||
disambiguatingSourceNodeId &&
|
||||
store.isPromotedByAny(graphId, {
|
||||
sourceNodeId,
|
||||
sourceWidgetName,
|
||||
disambiguatingSourceNodeId
|
||||
})
|
||||
)
|
||||
return true
|
||||
return store.isPromotedByAny(graphId, { sourceNodeId, sourceWidgetName })
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if the given promotion entry corresponds to a linked promotion
|
||||
* on the subgraph node. Linked promotions are driven by subgraph input
|
||||
|
||||
@@ -116,7 +116,7 @@ import {
|
||||
stripGraphPrefix,
|
||||
useWidgetValueStore
|
||||
} from '@/stores/widgetValueStore'
|
||||
import { usePromotionStore } from '@/stores/promotionStore'
|
||||
import { isWidgetPromoted } from '@/core/graph/subgraph/promotionUtils'
|
||||
import { useMissingModelStore } from '@/platform/missingModel/missingModelStore'
|
||||
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
|
||||
import type {
|
||||
@@ -144,7 +144,6 @@ const { shouldHandleNodePointerEvents, forwardEventToCanvas } =
|
||||
const { isSelectInputsMode } = useAppMode()
|
||||
const canvasStore = useCanvasStore()
|
||||
const { bringNodeToFront } = useNodeZIndex()
|
||||
const promotionStore = usePromotionStore()
|
||||
const executionErrorStore = useExecutionErrorStore()
|
||||
const missingModelStore = useMissingModelStore()
|
||||
|
||||
@@ -391,21 +390,14 @@ const processedWidgets = computed((): ProcessedWidget[] => {
|
||||
: mergedOptions
|
||||
|
||||
const sourceWidgetName = widget.storeName ?? widget.name
|
||||
// Promotions stored for non-SubgraphNode inner nodes omit the
|
||||
// disambiguatingSourceNodeId, but the widget's storeNodeId causes
|
||||
// one to be computed here. Check both key shapes to handle nested
|
||||
// subgraph promotions correctly (#10612).
|
||||
const isPromoted =
|
||||
graphId &&
|
||||
(promotionStore.isPromotedByAny(graphId, {
|
||||
sourceNodeId: hostNodeId,
|
||||
isWidgetPromoted(
|
||||
graphId,
|
||||
hostNodeId,
|
||||
sourceWidgetName,
|
||||
disambiguatingSourceNodeId: promotionSourceNodeId
|
||||
}) ||
|
||||
promotionStore.isPromotedByAny(graphId, {
|
||||
sourceNodeId: hostNodeId,
|
||||
sourceWidgetName
|
||||
}))
|
||||
promotionSourceNodeId
|
||||
)
|
||||
const borderStyle = isPromoted
|
||||
? 'ring ring-component-node-widget-promoted'
|
||||
: mergedOptions.advanced
|
||||
|
||||
Reference in New Issue
Block a user