codeant-ai-for-open-source[bot] commented on code in PR #38732:
URL: https://github.com/apache/superset/pull/38732#discussion_r2973524352
##########
superset-frontend/src/features/home/Menu.test.tsx:
##########
@@ -30,6 +30,14 @@ jest.mock('@apache-superset/core/theme', () => ({
useTheme: jest.fn(),
}));
+jest.mock('antd', () => ({
+ ...jest.requireActual('antd'),
+ Grid: {
+ ...jest.requireActual('antd').Grid,
+ useBreakpoint: () => ({ md: true }),
+ },
+}));
Review Comment:
**Suggestion:** The breakpoint mock forces desktop mode (`md: true`), so the
new small-screen navigation logic introduced by this PR is never executed in
tests. This creates a false-positive test suite where mobile regressions can
pass unnoticed. Mock the breakpoint as small-screen for this file (or make it
configurable per test) so the vertical/inline behavior is actually validated.
[logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ⚠️ Mobile nav branch never exercised in CI tests.
- ❌ Small-screen regressions can merge undetected.
- ⚠️ PR target behavior lacks direct automated verification.
```
</details>
```suggestion
jest.mock('antd', () => {
const antd = jest.requireActual('antd');
return {
...antd,
Grid: {
...antd.Grid,
useBreakpoint: () => ({ md: false }),
},
};
});
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run `npm test -- Menu.test.tsx` from `superset-frontend` (Jest entry in
`superset-frontend/package.json:85`), executing
`src/features/home/Menu.test.tsx`.
2. In test setup, `useBreakpoint` is hardcoded to desktop at
`src/features/home/Menu.test.tsx:33-39` with `md: true`.
3. Render path in `src/features/home/Menu.tsx:199` uses `screens =
useBreakpoint()`, then
always takes desktop branches at `Menu.tsx:283-287` (adds popup icon),
`Menu.tsx:354`
(`mode='horizontal'`), and `Menu.tsx:381` (`align='flex-end'`).
4. Introduce a mobile-only regression (e.g., break the `md:false` branch at
`Menu.tsx:354`
or `Menu.tsx:381`) and rerun the same tests; they still pass because no test
executes
small-screen behavior.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/home/Menu.test.tsx
**Line:** 33:39
**Comment:**
*Logic Error: The breakpoint mock forces desktop mode (`md: true`), so
the new small-screen navigation logic introduced by this PR is never executed
in tests. This creates a false-positive test suite where mobile regressions can
pass unnoticed. Mock the breakpoint as small-screen for this file (or make it
configurable per test) so the vertical/inline behavior is actually validated.
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%2F38732&comment_hash=44005b4c68d52455e5e09eae141cd115501cf6690efceb0be5272dfd0e293d18&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38732&comment_hash=44005b4c68d52455e5e09eae141cd115501cf6690efceb0be5272dfd0e293d18&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]
