test(modals): flatten test structure to follow best practices #35501
Conversation
- Remove nested describe/it blocks in favor of flat test() calls - Follow "avoid nesting when testing" best practices - Improves test readability and maintainability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #f2afaf
Actionable Suggestions - 2
-
superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx - 1
- Missing test isolation · Line 175-177
-
superset-frontend/src/pages/ThemeList/ThemeList.test.tsx - 1
- Test isolation issue · Line 103-105
Review Details
-
Files reviewed - 2 · Commit Range:
5e324c3..5e324c3- superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx
- superset-frontend/src/pages/ThemeList/ThemeList.test.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
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 documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| }); | ||
|
|
||
| describe('Modal.useModal integration', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
| test('uses Modal.useModal hook for confirmation dialogs', () => { |
There was a problem hiding this comment.
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.
Code suggestion
Check the AI-generated fix before applying
| }); | |
| describe('Modal.useModal integration', () => { | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| test('uses Modal.useModal hook for confirmation dialogs', () => { | |
| }); | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| test('uses Modal.useModal hook for confirmation dialogs', () => { |
Code Review Run #f2afaf
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| beforeEach(() => { | ||
| fetchMock.resetHistory(); | ||
| }); |
There was a problem hiding this comment.
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.
Code suggestion
Check the AI-generated fix before applying
| beforeEach(() => { | |
| fetchMock.resetHistory(); | |
| }); | |
| beforeEach(() => { | |
| fetchMock.resetHistory(); | |
| jest.clearAllMocks(); | |
| }); |
Code Review Run #f2afaf
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
2 participants
SUMMARY
Refactors modal test files to follow the "avoid nesting when testing" pattern by removing nested
describe()/it()blocks in favor of flattest()calls.Changes:
EmbeddedModal.test.tsx- remove nested describe blocks, use top-leveltest()callsThemeList.test.tsx- remove nested describe blocks, use top-leveltest()callsBenefits:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Test structure refactoring only, no functional changes
TESTING INSTRUCTIONS
Both test suites should pass with all tests (13 tests for ThemeList, 14 tests for EmbeddedModal).
ADDITIONAL INFORMATION
🤖 Generated with Claude Code