Skip to content

Commit 3e35b8f

Browse files
fix: address CodeRabbit review — remove .or(body) fallback, use TestIds, stricter assertions
1 parent d9c53b2 commit 3e35b8f

2 files changed

Lines changed: 62 additions & 65 deletions

File tree

browser_tests/fixtures/selectors.ts

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -5,70 +5,71 @@
55

66
export const TestIds = {
77
sidebar: {
8-
toolbar: 'side-toolbar',
9-
nodeLibrary: 'node-library-tree',
10-
nodeLibrarySearch: 'node-library-search',
11-
workflows: 'workflows-sidebar',
12-
modeToggle: 'mode-toggle'
8+
toolbar: "side-toolbar",
9+
nodeLibrary: "node-library-tree",
10+
nodeLibrarySearch: "node-library-search",
11+
workflows: "workflows-sidebar",
12+
modeToggle: "mode-toggle",
1313
},
1414
tree: {
15-
folder: 'tree-folder',
16-
leaf: 'tree-leaf',
17-
node: 'tree-node'
15+
folder: "tree-folder",
16+
leaf: "tree-leaf",
17+
node: "tree-node",
1818
},
1919
canvas: {
20-
main: 'graph-canvas',
21-
contextMenu: 'canvas-context-menu',
22-
toggleMinimapButton: 'toggle-minimap-button',
23-
toggleLinkVisibilityButton: 'toggle-link-visibility-button'
20+
main: "graph-canvas",
21+
contextMenu: "canvas-context-menu",
22+
toggleMinimapButton: "toggle-minimap-button",
23+
toggleLinkVisibilityButton: "toggle-link-visibility-button",
2424
},
2525
dialogs: {
26-
settings: 'settings-dialog',
27-
settingsContainer: 'settings-container',
28-
settingsTabAbout: 'settings-tab-about',
29-
confirm: 'confirm-dialog',
30-
errorOverlay: 'error-overlay',
31-
missingNodeCard: 'missing-node-card',
32-
about: 'about-panel',
33-
whatsNewSection: 'whats-new-section'
26+
settings: "settings-dialog",
27+
settingsContainer: "settings-container",
28+
settingsTabAbout: "settings-tab-about",
29+
confirm: "confirm-dialog",
30+
errorOverlay: "error-overlay",
31+
missingNodeCard: "missing-node-card",
32+
about: "about-panel",
33+
whatsNewSection: "whats-new-section",
3434
},
3535
topbar: {
36-
queueButton: 'queue-button',
37-
queueModeMenuTrigger: 'queue-mode-menu-trigger',
38-
saveButton: 'save-workflow-button'
36+
queueButton: "queue-button",
37+
queueModeMenuTrigger: "queue-mode-menu-trigger",
38+
saveButton: "save-workflow-button",
39+
subscribeButton: "topbar-subscribe-button",
3940
},
4041
nodeLibrary: {
41-
bookmarksSection: 'node-library-bookmarks-section'
42+
bookmarksSection: "node-library-bookmarks-section",
4243
},
4344
propertiesPanel: {
44-
root: 'properties-panel'
45+
root: "properties-panel",
4546
},
4647
node: {
47-
titleInput: 'node-title-input'
48+
titleInput: "node-title-input",
4849
},
4950
selectionToolbox: {
50-
colorPickerButton: 'color-picker-button',
51-
colorPickerCurrentColor: 'color-picker-current-color',
52-
colorBlue: 'blue',
53-
colorRed: 'red'
51+
colorPickerButton: "color-picker-button",
52+
colorPickerCurrentColor: "color-picker-current-color",
53+
colorBlue: "blue",
54+
colorRed: "red",
5455
},
5556
widgets: {
56-
decrement: 'decrement',
57-
increment: 'increment',
58-
domWidgetTextarea: 'dom-widget-textarea',
59-
subgraphEnterButton: 'subgraph-enter-button'
57+
decrement: "decrement",
58+
increment: "increment",
59+
domWidgetTextarea: "dom-widget-textarea",
60+
subgraphEnterButton: "subgraph-enter-button",
6061
},
6162
breadcrumb: {
62-
subgraph: 'subgraph-breadcrumb'
63+
subgraph: "subgraph-breadcrumb",
6364
},
6465
templates: {
65-
content: 'template-workflows-content',
66-
workflowCard: (id: string) => `template-workflow-${id}`
66+
content: "template-workflows-content",
67+
workflowCard: (id: string) => `template-workflow-${id}`,
6768
},
6869
user: {
69-
currentUserIndicator: 'current-user-indicator'
70-
}
71-
} as const
70+
currentUserIndicator: "current-user-indicator",
71+
},
72+
} as const;
7273

7374
/**
7475
* Helper type for accessing nested TestIds (excludes function values)
@@ -89,4 +90,4 @@ export type TestIdValue =
8990
(typeof TestIds.templates)[keyof typeof TestIds.templates],
9091
(id: string) => string
9192
>
92-
| (typeof TestIds.user)[keyof typeof TestIds.user]
93+
| (typeof TestIds.user)[keyof typeof TestIds.user];

browser_tests/tests/cloud.spec.ts

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,28 @@
1-
import { expect } from '@playwright/test'
1+
import { expect } from "@playwright/test";
22

3-
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
3+
import { comfyPageFixture as test } from "../fixtures/ComfyPage";
4+
import { TestIds } from "../fixtures/selectors";
45

5-
test.describe('Cloud distribution UI @cloud', () => {
6-
test('subscribe button is visible for free-tier users @cloud', async ({
7-
comfyPage
6+
test.describe("Cloud distribution UI @cloud", () => {
7+
test("subscribe button is visible for free-tier users @cloud", async ({
8+
comfyPage,
89
}) => {
910
const subscribeButton = comfyPage.page.getByTestId(
10-
'topbar-subscribe-button'
11-
)
12-
// In cloud mode with a free-tier user, the subscribe button should render.
13-
// It may not be visible if the user is not free-tier, so we check the DOM.
14-
// Full visibility depends on subscription state which requires auth mocking.
15-
await expect(
16-
subscribeButton.or(comfyPage.page.locator('body'))
17-
).toBeVisible()
18-
})
11+
TestIds.topbar.subscribeButton,
12+
);
13+
await expect(subscribeButton).toBeAttached();
14+
});
1915

20-
test('bottom panel toggle is hidden in cloud mode @cloud', async ({
21-
comfyPage
16+
test("bottom panel toggle is hidden in cloud mode @cloud", async ({
17+
comfyPage,
2218
}) => {
23-
const sideToolbar = comfyPage.page.getByTestId('side-toolbar')
24-
await expect(sideToolbar).toBeVisible()
19+
const sideToolbar = comfyPage.page.getByTestId(TestIds.sidebar.toolbar);
20+
await expect(sideToolbar).toBeVisible();
2521

2622
// In cloud mode, the bottom panel toggle button should not be rendered
27-
const bottomPanelToggle = sideToolbar.getByRole('button', {
28-
name: /bottom panel|terminal/i
29-
})
30-
await expect(bottomPanelToggle).toBeHidden()
31-
})
32-
})
23+
const bottomPanelToggle = sideToolbar.getByRole("button", {
24+
name: /bottom panel|terminal/i,
25+
});
26+
await expect(bottomPanelToggle).toHaveCount(0);
27+
});
28+
});

0 commit comments

Comments
 (0)