codeant-ai-for-open-source[bot] commented on code in PR #37298:
URL: https://github.com/apache/superset/pull/37298#discussion_r2713396518
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx:
##########
@@ -37,6 +37,37 @@ import {
import type { RootState } from 'src/views/store';
import type { Store } from 'redux';
+// Mock TableExploreTree to avoid complex tree rendering in tests
+jest.mock('../TableExploreTree', () => ({
+ __esModule: true,
+ default: () => (
+ <div data-test="mock-table-explore-tree">TableExploreTree</div>
Review Comment:
**Suggestion:** The mock component uses the attribute `data-test` while the
tests query `data-testid` via `screen.queryByTestId`. This mismatch means the
mock won't be found by `queryByTestId` and the test logic that waits for or
asserts on that element will be incorrect or flaky; change the attribute to
`data-testid` so the test queries can reliably find the mock. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ SelectView tests fail or become flaky in CI.
- ⚠️ Developer local test runs unreliable, causing reruns.
```
</details>
```suggestion
<div data-testid="mock-table-explore-tree">TableExploreTree</div>
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the test suite that includes this file (file:
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx).
The
mock is defined at lines 40-46 (jest.mock of TableExploreTree).
2. A test (for example the test at ~line 144 "renders a TableElement") calls
await
switchToSelectView() (helper defined at lines 48-69) which, after clicking
the segmented
item, executes:
- waitFor assertion at lines 55-59 that checks
screen.queryByTestId('mock-table-explore-tree') is not in the document.
3. Because the mock component renders a div with
data-test="mock-table-explore-tree" (line
44) instead of data-testid, screen.queryByTestId('mock-table-explore-tree')
(the query
used in the waitFor at lines 55-59) will always return null immediately —
even when the
mocked tree is still present in the DOM. The waitFor condition passes
prematurely.
4. After the premature pass, the helper proceeds to call
screen.getByRole('button', {
name: 'Change' }) at line 60. If the UI has not actually finished switching
views,
getByRole will throw (element not found) and the test fails or becomes flaky.
Explanation: The current mismatch (data-test vs data-testid) at lines 40-46
makes the
test's query (lines 55-59) invalid. Replacing data-test with data-testid at
the mock (line
44) aligns the mock with the query and prevents the waitFor from passing
before the actual
UI update completes.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx
**Line:** 44:44
**Comment:**
*Possible Bug: The mock component uses the attribute `data-test` while
the tests query `data-testid` via `screen.queryByTestId`. This mismatch means
the mock won't be found by `queryByTestId` and the test logic that waits for or
asserts on that element will be incorrect or flaky; change the attribute to
`data-testid` so the test queries can reliably find the mock.
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
