bito-code-review[bot] commented on code in PR #35501:
URL: https://github.com/apache/superset/pull/35501#discussion_r2403514884
##########
superset-frontend/src/pages/ThemeList/ThemeList.test.tsx:
##########
@@ -100,162 +100,153 @@ const renderThemesList = (props = {}) =>
},
);
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('ThemesList', () => {
- beforeEach(() => {
- fetchMock.resetHistory();
- });
+beforeEach(() => {
+ fetchMock.resetHistory();
+});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Test isolation issue</b></div>
<div id="fix">
The removal of the nested `describe('Modal.useModal integration')` block
also removed the `jest.clearAllMocks()` call from its `beforeEach`. This could
cause test pollution where spies and mocks from Modal-related tests
(`useModalSpy`, `confirmSpy`) could affect other tests. Add
`jest.clearAllMocks()` to the global `beforeEach` to maintain proper test
isolation.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```suggestion
beforeEach(() => {
fetchMock.resetHistory();
jest.clearAllMocks();
});
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/35501#issuecomment-3367433870>#f2afaf</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx:
##########
@@ -174,84 +174,78 @@ test('adds extension to DashboardEmbedModal', async () =>
{
extensionsRegistry.set('embedded.modal', undefined);
});
-describe('Modal.useModal integration', () => {
- beforeEach(() => {
- jest.clearAllMocks();
- });
+test('uses Modal.useModal hook for confirmation dialogs', () => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing test isolation</b></div>
<div id="fix">
The test refactoring removes the `beforeEach` hook that clears mocks between
tests, which could lead to test pollution and unreliable test results. The
`jest.clearAllMocks()` call was removed from the test setup, but multiple tests
use spies (`jest.spyOn(Modal, 'useModal')` and `jest.spyOn(Modal, 'confirm')`)
that need to be reset between test runs. This could cause spies to retain call
counts and mock implementations from previous tests, leading to false positives
or negatives in test assertions.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```suggestion
});
beforeEach(() => {
jest.clearAllMocks();
});
test('uses Modal.useModal hook for confirmation dialogs', () => {
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/35501#issuecomment-3367433870>#f2afaf</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
