Skip to content

Commit 09b13ba

Browse files
sadpandajoeclaude
andcommitted
fix(sqllab): fix Playwright tab counting with nav-scoped CSS selectors
Move getTabCount/getTabNames/getTab from EditableTabs to base Tabs class using .ant-tabs-nav-scoped CSS selectors instead of getByRole('tab'), which over-counted by matching remove buttons and inner tabs (Results/Query history). Also fix ensureEditorReady race condition after page reload where ace editor hasn't mounted yet but tabs exist in the nav bar. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 39e8998 commit 09b13ba

File tree

3 files changed

+51
-39
lines changed

3 files changed

+51
-39
lines changed

superset-frontend/playwright/components/core/EditableTabs.ts

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,32 +34,13 @@ export class EditableTabs extends Tabs {
3434
* Clicks the add-tab button rendered by antd in editable-card mode.
3535
*/
3636
async addTab(): Promise<void> {
37-
await this.element
38-
.getByRole('button', { name: 'Add tab' })
39-
.click();
40-
}
41-
42-
/**
43-
* Returns the number of tabs (excludes the add button).
44-
*/
45-
async getTabCount(): Promise<number> {
46-
return this.element.getByRole('tab').count();
47-
}
48-
49-
/**
50-
* Returns the text content of all tabs.
51-
*/
52-
async getTabNames(): Promise<string[]> {
53-
return this.element.getByRole('tab').allTextContents();
37+
await this.element.getByRole('button', { name: 'Add tab' }).click();
5438
}
5539

5640
/**
5741
* Clicks the remove button on the last tab.
5842
*/
5943
async removeLastTab(): Promise<void> {
60-
await this.element
61-
.locator('.ant-tabs-tab-remove')
62-
.last()
63-
.click();
44+
await this.nav.locator('.ant-tabs-tab-remove').last().click();
6445
}
6546
}

superset-frontend/playwright/components/core/Tabs.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,30 +21,58 @@ import { Locator, Page } from '@playwright/test';
2121

2222
/**
2323
* Tabs component for Ant Design tab navigation.
24+
*
25+
* Expects the locator to point to the `.ant-tabs` wrapper element
26+
* (not the inner tablist) so that `nav` can scope to the outer tab bar
27+
* and exclude nested/inner tabs (e.g. Results / Query history in SQL Lab).
2428
*/
2529
export class Tabs {
2630
readonly page: Page;
2731
private readonly locator: Locator;
2832

2933
constructor(page: Page, locator?: Locator) {
3034
this.page = page;
31-
// Default to the tablist role if no specific locator provided
3235
this.locator = locator ?? page.getByRole('tablist');
3336
}
3437

3538
/**
36-
* Gets the tablist element locator
39+
* The root element locator for this tabs component.
3740
*/
3841
get element(): Locator {
3942
return this.locator;
4043
}
4144

4245
/**
43-
* Gets a tab by name, scoped to this tablist's container
46+
* The tab navigation bar for this component.
47+
* Scoped to the first `.ant-tabs-nav` descendant so that queries
48+
* only hit this component's tabs, never nested/inner tab bars.
49+
*/
50+
protected get nav(): Locator {
51+
return this.locator.locator('.ant-tabs-nav').first();
52+
}
53+
54+
/**
55+
* Returns the number of tabs.
56+
* Counts `.ant-tabs-tab` wrappers in the nav bar — one per physical tab,
57+
* regardless of inner role="tab" elements (btn, remove button, etc.).
58+
*/
59+
async getTabCount(): Promise<number> {
60+
return this.nav.locator('.ant-tabs-tab').count();
61+
}
62+
63+
/**
64+
* Returns the text content of all tabs.
65+
*/
66+
async getTabNames(): Promise<string[]> {
67+
return this.nav.locator('.ant-tabs-tab-btn').allTextContents();
68+
}
69+
70+
/**
71+
* Gets a tab button by name, scoped to this component's nav bar.
4472
* @param tabName - The name/label of the tab
4573
*/
4674
getTab(tabName: string): Locator {
47-
return this.locator.getByRole('tab', { name: tabName });
75+
return this.nav.locator('.ant-tabs-tab-btn').filter({ hasText: tabName });
4876
}
4977

5078
/**

superset-frontend/playwright/pages/SqlLabPage.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,22 +75,26 @@ export class SqlLabPage {
7575
* is in the empty state ("Add a new tab to create SQL Query").
7676
* Waits for the ace editor to be ready before returning.
7777
*
78+
* Checks tab count in the nav bar (renders quickly) rather than ace editor
79+
* presence (renders later) to avoid a race condition after page reload
80+
* where the editor hasn't mounted yet but tabs already exist.
81+
*
7882
* SQL Lab renders type="card" in empty state (no .ant-tabs-nav-add button),
7983
* so we click the SQL Lab-specific [data-test="add-tab-icon"] instead of
8084
* using EditableTabs.addTab() which requires type="editable-card".
8185
*/
8286
async ensureEditorReady(): Promise<void> {
83-
const editorLocator = this.page.locator(SqlLabPage.SELECTORS.ACE_EDITOR);
84-
const editorCount = await editorLocator.count();
85-
if (editorCount === 0) {
87+
const tabCount = await this.getTabCount();
88+
if (tabCount === 0) {
8689
// Empty state: click the add-tab icon directly (works in both card modes)
8790
await this.editorTabs.element
8891
.locator(SqlLabPage.SELECTORS.ADD_TAB_ICON)
8992
.first()
9093
.click();
91-
// Wait for ace editor to render after tab creation
92-
await editorLocator.first().waitFor({ state: 'visible' });
9394
}
95+
// Wait for the ace editor to render (either from existing tab or newly created one)
96+
const editorLocator = this.page.locator(SqlLabPage.SELECTORS.ACE_EDITOR);
97+
await editorLocator.first().waitFor({ state: 'visible' });
9498
await this.getEditor().waitForReady();
9599
}
96100

@@ -149,21 +153,20 @@ export class SqlLabPage {
149153
await this.editorTabs.removeLastTab();
150154
// Wait for tab count to decrease
151155
await this.page.waitForFunction(
152-
([count, expected]) => {
153-
const tabs = document.querySelectorAll(
154-
'[data-test="sql-editor-tabs"] [role="tab"]',
155-
);
156-
return tabs.length === expected;
156+
([selector, expected]) => {
157+
const container = document.querySelector(selector);
158+
if (!container) return false;
159+
const nav = container.querySelector(':scope > .ant-tabs-nav');
160+
if (!nav) return false;
161+
return nav.querySelectorAll('.ant-tabs-tab').length === expected;
157162
},
158-
[countBefore, countBefore - 1] as const,
163+
[SqlLabPage.SELECTORS.SQL_EDITOR_TABS, countBefore - 1] as const,
159164
{ timeout: 5000 },
160165
);
161166
}
162167

163168
getTab(name: string): Locator {
164-
// Use hasText (substring) rather than getByRole name (accessible name)
165-
// because getTabNames() returns text content which may include close-icon text.
166-
return this.editorTabs.element.locator('[role="tab"]', { hasText: name });
169+
return this.editorTabs.getTab(name);
167170
}
168171

169172
// ── Database Selection (Left Sidebar) ──

0 commit comments

Comments
 (0)