You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sadpandajoe (via GitHub)" <gi...@apache.org> on 2026/01/28 19:18:15 UTC
[PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
sadpandajoe opened a new pull request, #37529:
URL: https://github.com/apache/superset/pull/37529
### SUMMARY
Fix skipped Dashboard Export Playwright E2E tests by creating a reusable `Menu` core component that reliably handles Ant Design dropdown submenu interactions.
**Problem:** The export tests were skipped because Playwright's `.hover()` didn't reliably trigger Ant Design's submenu popups. The submenu uses `onMouseEnter`/`onMouseLeave` with internal state, making hover-based interactions flaky.
**Solution:** Created a `Menu` component (`playwright/components/core/Menu.ts`) with a fallback chain:
1. **Hover** (primary) - native Playwright hover on `.ant-dropdown-menu-submenu-title`
2. **Keyboard** (fallback) - focus + ArrowRight for accessibility
3. **dispatchEvent** (last resort) - synthetic mouseover/mouseenter events matching Cypress approach
Additional improvements:
- `DashboardPage.waitForChartsToLoad()` waits for all loading indicators (count=0) before interacting with menus
- Popup selectors are scoped by expected item text to avoid hitting wrong popups
- Tests use existing `Toast` component for success toast assertions
- Download files are cleaned up in `afterEach`
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Test infrastructure change only.
**Before:** 3 tests skipped (`test.describe.skip`)
**After:** 2 tests enabled and passing (merged duplicate toast test into export test)
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
PLAYWRIGHT_ADMIN_PASSWORD=admin INCLUDE_EXPERIMENTAL=true npx playwright test --grep "Dashboard Export"
```
Both tests should pass:
- `should download ZIP and show success toast when clicking Export YAML`
- `should download example bundle and show success toast when clicking Export as Example`
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3814966644
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3814139968
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #37529:
URL: https://github.com/apache/superset/pull/37529#discussion_r2739119922
##########
superset-frontend/playwright/components/core/Menu.ts:
##########
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Locator, Page } from '@playwright/test';
+
+/**
+ * Menu component for Ant Design dropdown menus.
+ * Uses hover as primary approach (most natural user interaction).
+ * Falls back to keyboard navigation, then dispatchEvent if hover fails.
+ *
+ * This component handles menu content only - not the trigger that opens the menu.
+ * The calling page object should open the menu first, then use this component.
+ *
+ * @example
+ * // In a page object
+ * async selectDownloadOption(optionText: string): Promise<void> {
+ * await this.openHeaderActionsMenu();
+ * const menu = new Menu(this.page, '[data-test="header-actions-menu"]');
+ * await menu.selectSubmenuItem('Download', optionText);
+ * }
+ */
+export class Menu {
+ private readonly page: Page;
+ private readonly locator: Locator;
+
+ private static readonly SELECTORS = {
+ SUBMENU: '.ant-dropdown-menu-submenu',
+ SUBMENU_POPUP: '.ant-dropdown-menu-submenu-popup',
+ SUBMENU_TITLE: '.ant-dropdown-menu-submenu-title',
+ } as const;
+
+ private static readonly TIMEOUTS = {
+ SUBMENU_OPEN: 5000,
+ } as const;
+
+ constructor(page: Page, selector: string);
+ constructor(page: Page, locator: Locator);
+ constructor(page: Page, selectorOrLocator: string | Locator) {
+ this.page = page;
+ if (typeof selectorOrLocator === 'string') {
+ this.locator = page.locator(selectorOrLocator);
+ } else {
+ this.locator = selectorOrLocator;
+ }
+ }
+
+ /**
+ * Opens a submenu and selects an item within it.
+ * Uses hover as primary approach, falls back to keyboard then dispatchEvent.
+ *
+ * @param submenuText - The text of the submenu to open (e.g., "Download")
+ * @param itemText - The text of the item to select (e.g., "Export YAML")
+ * @param options - Optional timeout settings
+ */
+ async selectSubmenuItem(
+ submenuText: string,
+ itemText: string,
+ options?: { timeout?: number },
+ ): Promise<void> {
+ const timeout = options?.timeout ?? Menu.TIMEOUTS.SUBMENU_OPEN;
+
+ // Try hover first (most natural user interaction)
+ let popup = await this.openSubmenuWithHover(submenuText, itemText, timeout);
+
+ // Fallback to keyboard navigation
+ if (!popup) {
+ popup = await this.openSubmenuWithKeyboard(
+ submenuText,
+ itemText,
+ timeout,
+ );
+ }
+
+ // Last resort: dispatchEvent
+ if (!popup) {
+ popup = await this.openSubmenuWithDispatchEvent(
+ submenuText,
+ itemText,
+ timeout,
+ );
+ }
+
+ if (!popup) {
+ throw new Error(
+ `Failed to open submenu "${submenuText}". Tried hover, keyboard, and dispatchEvent.`,
+ );
+ }
+
+ // Use dispatchEvent instead of click to bypass viewport and pointer interception
+ // issues. Ant Design renders submenu popups in a portal that can be positioned
+ // outside the viewport or behind chart content (e.g., large tables with z-index).
+ await popup
+ .getByText(itemText, { exact: true })
+ .dispatchEvent('click', { timeout });
+ }
+
+ /**
+ * Opens a submenu using native Playwright hover.
+ * Returns the popup locator if successful, null otherwise.
+ */
+ private async openSubmenuWithHover(
+ submenuText: string,
+ itemText: string,
+ timeout: number,
+ ): Promise<Locator | null> {
+ try {
+ const submenuTitle = this.getSubmenuTitle(submenuText);
+ await submenuTitle.hover();
+
+ // Find the popup that contains the expected item (scopes to correct popup)
+ const popup = this.page
+ .locator(Menu.SELECTORS.SUBMENU_POPUP)
+ .filter({ hasText: itemText });
+ await popup.waitFor({ state: 'visible', timeout });
+
+ // Allow Ant Design's slide-in animation to complete before clicking.
+ // Without this, the element may be "not stable" and clicks can fail.
+ await this.page.waitForTimeout(150);
Review Comment:
The hardcoded 150ms timeout for waiting for animations to complete is a magic number. Consider defining this as a named constant (e.g., `ANIMATION_COMPLETE_DELAY`) in the TIMEOUTS object for better maintainability and to make it easier to adjust if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3814388982
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3831229578
Thanks for the help with this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #37529:
URL: https://github.com/apache/superset/pull/37529
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3813430535
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 0f7ef7bdf03838c52b65e7b6287db28825a9f0e4 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/697a6266bad3d0000849d0e4 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-37529--superset-docs-preview.netlify.app](https://deploy-preview-37529--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-37529--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37529:
URL: https://github.com/apache/superset/pull/37529#discussion_r2739301270
##########
superset-frontend/playwright/components/core/Menu.ts:
##########
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Locator, Page } from '@playwright/test';
+
+/**
+ * Menu component for Ant Design dropdown menus.
+ * Uses hover as primary approach (most natural user interaction).
+ * Falls back to keyboard navigation, then dispatchEvent if hover fails.
+ *
+ * This component handles menu content only - not the trigger that opens the menu.
+ * The calling page object should open the menu first, then use this component.
+ *
+ * @example
+ * // In a page object
+ * async selectDownloadOption(optionText: string): Promise<void> {
+ * await this.openHeaderActionsMenu();
+ * const menu = new Menu(this.page, '[data-test="header-actions-menu"]');
+ * await menu.selectSubmenuItem('Download', optionText);
+ * }
+ */
+export class Menu {
+ private readonly page: Page;
+ private readonly locator: Locator;
+
+ private static readonly SELECTORS = {
+ SUBMENU: '.ant-dropdown-menu-submenu',
+ SUBMENU_POPUP: '.ant-dropdown-menu-submenu-popup',
+ SUBMENU_TITLE: '.ant-dropdown-menu-submenu-title',
+ } as const;
+
+ private static readonly TIMEOUTS = {
+ SUBMENU_OPEN: 5000,
+ } as const;
+
+ constructor(page: Page, selector: string);
+ constructor(page: Page, locator: Locator);
+ constructor(page: Page, selectorOrLocator: string | Locator) {
+ this.page = page;
+ if (typeof selectorOrLocator === 'string') {
+ this.locator = page.locator(selectorOrLocator);
+ } else {
+ this.locator = selectorOrLocator;
+ }
+ }
+
+ /**
+ * Opens a submenu and selects an item within it.
+ * Uses hover as primary approach, falls back to keyboard then dispatchEvent.
+ *
+ * @param submenuText - The text of the submenu to open (e.g., "Download")
+ * @param itemText - The text of the item to select (e.g., "Export YAML")
+ * @param options - Optional timeout settings
+ */
+ async selectSubmenuItem(
+ submenuText: string,
+ itemText: string,
+ options?: { timeout?: number },
+ ): Promise<void> {
+ const timeout = options?.timeout ?? Menu.TIMEOUTS.SUBMENU_OPEN;
+
+ // Try hover first (most natural user interaction)
+ let popup = await this.openSubmenuWithHover(submenuText, itemText, timeout);
+
+ // Fallback to keyboard navigation
+ if (!popup) {
+ popup = await this.openSubmenuWithKeyboard(
+ submenuText,
+ itemText,
+ timeout,
+ );
+ }
+
+ // Last resort: dispatchEvent
+ if (!popup) {
+ popup = await this.openSubmenuWithDispatchEvent(
+ submenuText,
+ itemText,
+ timeout,
+ );
+ }
+
+ if (!popup) {
+ throw new Error(
+ `Failed to open submenu "${submenuText}". Tried hover, keyboard, and dispatchEvent.`,
+ );
+ }
+
+ // Use dispatchEvent instead of click to bypass viewport and pointer interception
+ // issues. Ant Design renders submenu popups in a portal that can be positioned
+ // outside the viewport or behind chart content (e.g., large tables with z-index).
+ await popup
+ .getByText(itemText, { exact: true })
+ .dispatchEvent('click', { timeout });
Review Comment:
**Suggestion:** The `timeout` option passed to `dispatchEvent` is incorrectly being treated as Playwright options, but for locators it is actually the event init object, so this value is ignored by Playwright and does not constrain how long it waits, which can lead to tests not timing out as intended when the item is slow to become actionable. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard export E2E tests may be flaky.
- ⚠️ Playwright menu selection may mask timing failures.
- ⚠️ Tests for Download → Export as Example affected.
```
</details>
```suggestion
.dispatchEvent('click');
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Dashboard page test flow that uses the Menu helper:
- Caller: superset-frontend/playwright/pages/DashboardPage.ts at the call site found by
Grep:
"await menu.selectSubmenuItem('Download', optionText);" (file: DashboardPage.ts,
around line 125).
2. Execution enters the Menu.selectSubmenuItem implementation in:
- superset-frontend/playwright/components/core/Menu.ts at the method starting on line
71 (selectSubmenuItem) and reaches the final selection code at lines 108-110 where
dispatchEvent is invoked.
3. The code calls popup.getByText(...).dispatchEvent('click', { timeout }) (Menu.ts lines
108-110).
- Playwright's Locator.dispatchEvent accepts an event init object (MouseEventInit) and
does not interpret the passed object as a Playwright timeout; therefore the { timeout }
argument becomes event data, not a timeout for Playwright's action.
4. Reproduce failure: run the Dashboard Export Playwright E2E that triggers Download →
Export as Example:
- Command: PLAYWRIGHT_ADMIN_PASSWORD=admin INCLUDE_EXPERIMENTAL=true npx playwright
test --grep "Dashboard Export" (per PR testing instructions).
- If the submenu popup becomes visible but the item is not yet actionable (e.g., due to
animation, overlay, or slow rendering), dispatchEvent('click', { timeout }) will not be
subject to a Playwright timeout and will not wait or fail as intended. The test may
either:
a) Appear to succeed locally because dispatchEvent returns quickly (no waiting for
actionable element), or
b) Mask flakiness: the intended Playwright timeout isn't applied so failures that
should be caught (e.g., element not attached/visible) may be hidden or tests may hang
elsewhere.
5. Evidence from Grep: the exact call exists in Menu.ts (found at
superset-frontend/playwright/components/core/Menu.ts:110 by the search), and
Menu.selectSubmenuItem is actively used by DashboardPage.ts
(superset-frontend/playwright/pages/DashboardPage.ts:125), so this is on the real E2E path
for dashboard export tests.
6. Conclusion / why the suggested change is correct:
- Removing the { timeout } parameter makes the dispatchEvent call semantically correct
(it passes only the event type and eventInit if needed). If a Playwright-style
waiting/timeout is required, it must be done with Playwright waiting APIs (e.g.,
waitFor, waitForSelector) before calling dispatchEvent. The suggested improved_code
removes the misleading timeout parameter to avoid silently passing it as event data.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/components/core/Menu.ts
**Line:** 110:110
**Comment:**
*Logic Error: The `timeout` option passed to `dispatchEvent` is incorrectly being treated as Playwright options, but for locators it is actually the event init object, so this value is ignored by Playwright and does not constrain how long it waits, which can lead to tests not timing out as intended when the item is slow to become actionable.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3819843166
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3813377581
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3813776832
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dashboard): fix Export as Example with app prefix and enable Dashboard Export E2E tests [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37529:
URL: https://github.com/apache/superset/pull/37529#issuecomment-3814616378
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(playwright): fix Dashboard Export tests with reliable Menu component [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37529:
URL: https://github.com/apache/superset/pull/37529#discussion_r2738213292
##########
superset-frontend/playwright/pages/DashboardPage.ts:
##########
@@ -63,6 +64,34 @@ export class DashboardPage {
});
}
+ /**
+ * Wait for all charts on the dashboard to finish loading.
+ * Waits until no loading indicators are visible.
+ */
+ async waitForChartsToLoad(options?: { timeout?: number }): Promise<void> {
+ const timeout = options?.timeout ?? TIMEOUT.PAGE_LOAD;
+ const loadingIndicators = this.page.locator('[aria-label="Loading"]');
+
+ // Wait until all loading indicators are gone (count reaches 0)
+ await loadingIndicators
+ .first()
+ .waitFor({ state: 'hidden', timeout })
+ .catch(() => {
+ // No loading indicators found - charts already loaded
+ });
+
+ // Double-check no loading indicators remain
+ await this.page
+ .waitForFunction(
+ selector => document.querySelectorAll(selector).length === 0,
+ '[aria-label="Loading"]',
+ { timeout },
+ )
+ .catch(() => {
+ // Timeout is acceptable - proceed if indicators are gone or minimal
+ });
Review Comment:
**Suggestion:** In `waitForChartsToLoad`, calling `loadingIndicators.first().waitFor({ state: 'hidden', timeout })` and catching all errors means that when there are no `[aria-label="Loading"]` elements (or they never appear), Playwright will still wait the full timeout before throwing and being caught, turning a no-op into a long delay and potentially causing unnecessary timeouts in tests that don't show loading spinners; additionally, swallowing timeouts from the second `waitForFunction` hides situations where charts never finish loading, so callers proceed while the UI is still loading. A more robust approach is to first check if any loading indicators exist and, if so, wait once for the count to reach zero, letting that timeout propagate instead of being silently ignored. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Playwright dashboard export tests experience long unnecessary delays.
- ⚠️ Test flakiness hidden when charts never finish loading.
- ⚠️ CI job timeouts and longer test runs.
- ⚠️ Affects all tests using waitForChartsToLoad helper.
```
</details>
```suggestion
// If there are no loading indicators, return immediately.
if ((await loadingIndicators.count()) === 0) {
return;
}
// Wait until all loading indicators are gone (count reaches 0)
await this.page.waitForFunction(
selector => document.querySelectorAll(selector).length === 0,
'[aria-label="Loading"]',
{ timeout },
);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Playwright test that navigates to a dashboard (tests call
DashboardPage.gotoBySlug / gotoById and then DashboardPage.waitForChartsToLoad). The
DashboardPage implementation is at superset-frontend/playwright/pages/DashboardPage.ts
lines 45-126; the waitForChartsToLoad method starts at line 71 (see file read).
2. For dashboards that render immediately without any spinner elements, the page contains
zero elements matching '[aria-label="Loading"]'. In this state, the code executes
loadingIndicators.first().waitFor({ state: 'hidden', timeout }) at lines 75-79. Because
.first().waitFor waits for the locator to become hidden, Playwright will wait up to the
provided timeout before resolving or rejecting.
3. The code catches the rejection (.catch(() => { /* No loading indicators found - charts
already loaded */ })) at lines 79-81, swallowing the timeout. This means the test run will
incur the full timeout delay (TIMEOUT.PAGE_LOAD) before proceeding even though no spinner
exists.
4. After that, the second check uses page.waitForFunction(...) at lines 84-90 which is
also .catch()-ed. If charts actually never finish loading (spinner stuck or long-running
loads), the timeouts are swallowed and the test proceeds silently without failing,
potentially causing later flakiness or assertions to fail unexpectedly.
5. The proposed improved_code (check count() then waitForFunction without swallowing)
avoids the initial unnecessary wait when count() === 0 and lets the waitForFunction
timeout propagate if loading indicators are present but never clear. This reproduces
locally by running the Dashboard Export Playwright tests (per PR testing instructions)
against a dashboard with no spinners and observing the extra TIMEOUT.PAGE_LOAD delay
before tests continue.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/pages/DashboardPage.ts
**Line:** 79:92
**Comment:**
*Performance: In `waitForChartsToLoad`, calling `loadingIndicators.first().waitFor({ state: 'hidden', timeout })` and catching all errors means that when there are no `[aria-label="Loading"]` elements (or they never appear), Playwright will still wait the full timeout before throwing and being caught, turning a no-op into a long delay and potentially causing unnecessary timeouts in tests that don't show loading spinners; additionally, swallowing timeouts from the second `waitForFunction` hides situations where charts never finish loading, so callers proceed while the UI is still loading. A more robust approach is to first check if any loading indicators exist and, if so, wait once for the count to reach zero, letting that timeout propagate instead of being silently ignored.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org