codeant-ai-for-open-source[bot] commented on code in PR #38567:
URL: https://github.com/apache/superset/pull/38567#discussion_r2943427310
##########
superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts:
##########
@@ -40,10 +41,10 @@ test('isMatrixifyEnabled should return false when no
matrixify configuration exi
test('isMatrixifyEnabled should return false when layout controls are
disabled', () => {
const formData = {
viz_type: 'table',
- matrixify_mode_rows: 'disabled',
- matrixify_mode_columns: 'disabled',
+ matrixify_mode_rows: 'disabled' as const,
+ matrixify_mode_columns: 'disabled' as const,
matrixify_rows: [createMetric('Revenue')],
Review Comment:
**Suggestion:** This test can pass for the wrong reason because
`isMatrixifyEnabled` first checks `matrixify_enable`, and it's currently
omitted. That means the assertion may still pass even if disabled-axis handling
regresses, since the function returns false before evaluating
`matrixify_mode_rows`/`matrixify_mode_columns`. Set `matrixify_enable` to
`true` so the test actually validates the disabled layout-mode path. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Disabled-axis branch lacks direct unit-test coverage.
- ⚠️ CI may miss regressions in mode handling.
- ⚠️ Test intent mismatches actual executed path.
```
</details>
```suggestion
matrixify_enable: true,
matrixify_mode_rows: 'disabled' as const,
matrixify_mode_columns: 'disabled' as const,
matrixify_rows: [createMetric('Revenue')],
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the test `isMatrixifyEnabled should return false when layout controls
are disabled`
in
`superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts:41-50`.
2. Observe its fixture omits `matrixify_enable` (`matrixify.test.ts:42-47`),
so
`isMatrixifyEnabled(formData)` is called with `matrixify_enable` undefined.
3. In
`superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts:187-190`,
`isMatrixifyEnabled` immediately returns `false` when
`formData.matrixify_enable !==
true`.
4. Because of that early return, disabled-axis logic
(`matrixify.ts:192-197`) is not
exercised by this test, so the test can pass without validating the intended
disabled-mode
branch.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts
**Line:** 44:46
**Comment:**
*Logic Error: This test can pass for the wrong reason because
`isMatrixifyEnabled` first checks `matrixify_enable`, and it's currently
omitted. That means the assertion may still pass even if disabled-axis handling
regresses, since the function returns false before evaluating
`matrixify_mode_rows`/`matrixify_mode_columns`. Set `matrixify_enable` to
`true` so the test actually validates the disabled layout-mode path.
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%2F38567&comment_hash=8fe75b031def78db193bc99c079929e0699b5965abe183c49f7581f14c8c3b4b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38567&comment_hash=8fe75b031def78db193bc99c079929e0699b5965abe183c49f7581f14c8c3b4b&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]
