codeant-ai-for-open-source[bot] commented on code in PR #38776:
URL: https://github.com/apache/superset/pull/38776#discussion_r2967577980
##########
superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx:
##########
@@ -81,7 +86,10 @@ const setup = (overrides = {}) => (
<QueryHistory {...mockedProps} {...overrides} />
);
-afterEach(() => fetchMock.clearHistory().removeRoutes());
+afterEach(() => {
+ fetchMock.clearHistory().removeRoutes();
+ cleanupExtensions();
+});
Review Comment:
**Suggestion:** The mocked feature-flag function is never reset between
tests, so its previous implementation can leak into later tests. When a prior
test leaves backend persistence enabled, this test file can make unexpected
network requests (without matching fetch routes), causing flaky or failing
tests. Reset the mock in `afterEach` instead of only clearing call history
inside individual tests. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ QueryHistory tests make unintended `/api/v1/query/` requests.
- ⚠️ Test isolation weakens across same-file test cases.
- ⚠️ CI failures become order-dependent and flaky.
```
</details>
```suggestion
mockedIsFeatureEnabled.mockReset();
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx:105-107`,
run
test `fetches the query history when the persistence mode is enabled`; it
sets
`mockedIsFeatureEnabled.mockImplementation(...)` to return true for
`FeatureFlag.SqllabBackendPersistence`.
2. At the end of that test (`QueryHistory.test.tsx:117`), only
`isFeatureEnabledMock.mockClear()` is called, which clears call history but
keeps
implementation; `afterEach` (`QueryHistory.test.tsx:89-92`) also does not
reset
`mockedIsFeatureEnabled`.
3. Next test `renders contributed toolbar action in queryHistory slot`
(`QueryHistory.test.tsx:254+`) renders `QueryHistory` without setting
feature-flag
behavior, so it still inherits the previous mock implementation.
4. In component code
`superset-frontend/src/SqlLab/components/QueryHistory/index.tsx:79-83`,
`useEditorQueriesQuery` executes with `skip: !isFeatureEnabled(...)`; stale
`true`
disables skip and triggers query fetching, which hits `/api/v1/query/` from
`superset-frontend/src/hooks/apiResources/queries.ts:114-117` unexpectedly
for a test that
never configured that route.
```
</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/QueryHistory/QueryHistory.test.tsx
**Line:** 92:92
**Comment:**
*Possible Bug: The mocked feature-flag function is never reset between
tests, so its previous implementation can leak into later tests. When a prior
test leaves backend persistence enabled, this test file can make unexpected
network requests (without matching fetch routes), causing flaky or failing
tests. Reset the mock in `afterEach` instead of only clearing call history
inside individual tests.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38776&comment_hash=c792ec8962d15c1a69a382f8f2773bcfe0a5aae539938738ac507d581d7f8b25&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38776&comment_hash=c792ec8962d15c1a69a382f8f2773bcfe0a5aae539938738ac507d581d7f8b25&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/TableExploreTree/TableExploreTree.test.tsx:
##########
@@ -74,6 +79,7 @@ beforeEach(() => {
afterEach(() => {
jest.clearAllMocks();
fetchMock.clearHistory();
+ cleanupExtensions();
Review Comment:
**Suggestion:** Route mocks are never removed between tests, so per-test
overrides (like the delayed schemas response) can leak into later tests and
make them order-dependent or flaky. Keep `cleanupExtensions()` but also remove
fetch routes in teardown so each test starts with a clean mock registry. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ TableExploreTree test suite becomes order-dependent.
- ⚠️ Schema-loading tests can become timing-flaky.
- ⚠️ CI reliability degrades in SqlLab component tests.
```
</details>
```suggestion
fetchMock.removeRoutes();
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run `TableExploreTree` tests in
`superset-frontend/src/SqlLab/components/TableExploreTree/TableExploreTree.test.tsx`;
each
`beforeEach` registers 4 `fetchMock.get(...)` routes (lines 58-76).
2. In test `shows loading skeleton while fetching schemas` (lines 217-229),
a second
schemas matcher `glob:*/api/v1/database/1/schemas/?*` is added with delayed
response.
3. `afterEach` (lines 79-83) only calls `fetchMock.clearHistory()` and
`cleanupExtensions()`, so route definitions are not removed between tests.
4. Subsequent tests (for example lines 238-266) call `renderComponent()`
(lines 103-107),
which triggers the same schema endpoint; route resolution now depends on
accumulated prior
matchers, creating order-dependent timing/behavior and potential flakiness.
```
</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/TableExploreTree/TableExploreTree.test.tsx
**Line:** 82:82
**Comment:**
*Logic Error: Route mocks are never removed between tests, so per-test
overrides (like the delayed schemas response) can leak into later tests and
make them order-dependent or flaky. Keep `cleanupExtensions()` but also remove
fetch routes in teardown so each test starts with a clean mock registry.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38776&comment_hash=57dc6b07f87029c06ae9eae8367a161fa19af617fb369341ec1bc645599a88d9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38776&comment_hash=57dc6b07f87029c06ae9eae8367a161fa19af617fb369341ec1bc645599a88d9&reaction=dislike'>👎</a>
--
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]
