You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2025/12/15 19:44:49 UTC
[PR] fix(tests): resolve flaky "should edit correctly" test in chart list [superset]
rusackas opened a new pull request, #36641:
URL: https://github.com/apache/superset/pull/36641
### SUMMARY
Fixes a flaky Cypress test in `cypress/e2e/chart_list/list.test.ts` that was intermittently failing with:
```
CypressError: Timed out retrying after 8050ms: `cy.click()` failed because this element is detached from the DOM.
```
**Root Cause:** After switching from card to list view with `setGridMode('list')`, React re-renders the entire view. The test was immediately trying to click the edit button (`cy.getBySel('edit-alt').eq(1).click()`) before the DOM stabilized, causing the element to be detached between the query and the click.
**Fix:**
1. Add wait for loading indicator to disappear after mode change
2. Add visibility check for table rows before interacting
3. Scope the edit button selector to a specific table row instead of querying all edit buttons globally, making it more resilient to DOM changes during re-renders
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Test fix only
### TESTING INSTRUCTIONS
1. Run the Cypress test multiple times to verify it's no longer flaky:
```bash
cd superset-frontend/cypress-base
npm run cypress-run-chrome -- --spec "cypress/e2e/chart_list/list.test.ts"
```
2. The "should edit correctly" test should pass consistently
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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(tests): resolve flaky "should edit correctly" test in chart list [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36641:
URL: https://github.com/apache/superset/pull/36641#issuecomment-3657320602
CodeAnt AI finished reviewing your 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] fix(tests): resolve flaky "should edit correctly" test in chart list [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36641:
URL: https://github.com/apache/superset/pull/36641#issuecomment-3657320477
## Nitpicks 🔍
<table>
<tr><td>🔒 <strong>No security issues identified</strong></td></tr>
<tr><td>⚡ <strong>Recommended areas for review</strong><br><br>
- [ ] <a href='https://github.com/apache/superset/pull/36641/files#diff-77302dca82ea8e2784d0fb12c98fb4b8a462de136edbc20977280cf84c130fcaR208-R214'><strong>Potential timing flake</strong></a><br>The test now waits for '.loading' to disappear and checks table rows are visible, but the click on a row-scoped edit button can still race if the specific row isn't yet rendered/stable. Verify the row exists and is stable before interacting (e.g., assert row count or content). Consider using Cypress commands that scope and retry reliably (within / chained assertions).<br>
- [ ] <a href='https://github.com/apache/superset/pull/36641/files#diff-77302dca82ea8e2784d0fb12c98fb4b8a462de136edbc20977280cf84c130fcaR209-R213'><strong>Selector robustness</strong></a><br>The selector uses a positional index `.eq(1)` which assumes row ordering and presence. If dataset or ordering changes tests may break or click the wrong row. Prefer selecting the row by its visible content (the chart name) or add an explicit assertion about the number or identity of rows before acting.<br>
</td></tr>
</table>
--
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(tests): resolve flaky "should edit correctly" test in chart list [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36641:
URL: https://github.com/apache/superset/pull/36641#issuecomment-3657312956
CodeAnt AI is reviewing your 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] fix(tests): resolve flaky "should edit correctly" test in chart list [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #36641:
URL: https://github.com/apache/superset/pull/36641
--
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(tests): resolve flaky "should edit correctly" test in chart list [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 #36641:
URL: https://github.com/apache/superset/pull/36641#discussion_r2620626441
##########
superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts:
##########
@@ -206,7 +206,11 @@ describe('Charts list', () => {
// edits in list-view
setGridMode('list');
- cy.getBySel('edit-alt').eq(1).click();
+ // Wait for list view to fully render after mode change
+ cy.get('.loading').should('not.exist');
+ cy.getBySel('table-row').should('be.visible');
+ // Use a more specific selector scoped to a table row to avoid detached element issues
+ cy.getBySel('table-row').eq(1).find('[data-test="edit-alt"]').click();
Review Comment:
**Suggestion:** The test assumes the chart to edit is always on the second row (`eq(1)`), which may not hold if the row ordering changes or additional charts are present, causing the test to edit the wrong chart or be flaky; instead, it should target the row by the chart title that was just edited in card view. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
cy.contains('[data-test="table-row"]', '1 - Sample chart | EDITED')
.find('[data-test="edit-alt"]')
.click();
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current test targets the second table-row by index (eq(1)), which is fragile: row ordering or extra rows can make the test click the wrong row and produce flaky failures.
The proposed change scopes the selector to the row that contains the exact edited chart title ('1 - Sample chart | EDITED'), making the action deterministic and resilient to ordering changes. This is a correctness/stability improvement for an end-to-end test, not just a stylistic tweak.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts
**Line:** 213:213
**Comment:**
*Logic Error: The test assumes the chart to edit is always on the second row (`eq(1)`), which may not hold if the row ordering changes or additional charts are present, causing the test to edit the wrong chart or be flaky; instead, it should target the row by the chart title that was just edited in card view.
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(tests): resolve flaky "should edit correctly" test in chart list [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36641:
URL: https://github.com/apache/superset/pull/36641#issuecomment-3657313217
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