fix(test): changed test use unsaved changes prompt#35447
fix(test): changed test use unsaved changes prompt#35447sadpandajoe merged 39 commits intoapache:masterfrom
Conversation
This reverts commit 35aaeb6.
![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 #c65c3d
Actionable Suggestions - 1
-
superset-frontend/src/hooks/useUnsavedChangesPrompt/useUnsavedChangesPrompt.test.tsx - 1
- Test isolation failure · Line 33-33
Review Details
-
Files reviewed - 1 · Commit Range:
19151d1..b1897dd- superset-frontend/src/hooks/useUnsavedChangesPrompt/useUnsavedChangesPrompt.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
|
|
||
| expect(result.current.showModal).toBe(false); | ||
| }); | ||
| test('should not show modal initially', () => { |
There was a problem hiding this comment.
The PR removes the describe block wrapper and converts tests to standalone test functions, but this creates a critical test isolation issue. All tests now share the same history instance and wrapper configuration, which can cause test pollution where state from one test affects subsequent tests. The history object accumulates navigation state across tests, and the shared wrapper component may retain state. This will lead to flaky tests and false positives/negatives in CI/CD pipelines. Add proper test isolation by reinitializing the history instance before each test using beforeEach.
Code suggestion
Check the AI-generated fix before applying
-
-const history = createMemoryHistory({
- initialEntries: ['/dashboard'],
-});
-
+let history = createMemoryHistory({
+ initialEntries: ['/dashboard'],
+});
+
+beforeEach(() => {
+ history = createMemoryHistory({ initialEntries: ['/dashboard'] });
+});
+
Code Review Run #c65c3d
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
msyavuz
left a comment
msyavuz
left a comment
There was a problem hiding this comment.
Looks good to me with a small nit
| let history = createMemoryHistory({ | ||
| initialEntries: ['/dashboard'], | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| history = createMemoryHistory({ initialEntries: ['/dashboard'] }); | ||
| }); |
There was a problem hiding this comment.
beforeEach should work on a flat structure as well right?
There was a problem hiding this comment.
Yes, beforeEach works perfectly fine with a flat test structure.
It runs before each test - every test gets a fresh history instance
There was a problem hiding this comment.
Ok, but i don't quite understand this change still? If const was working before do we need this let and reassign in beforeEach?
3 participants
After adding PR #35307, linter checks do not pass
SUMMARY
This is what is corrected in this
testing guidelines
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION