codeant-ai-for-open-source[bot] commented on code in PR #36346:
URL: https://github.com/apache/superset/pull/36346#discussion_r2583560429
##########
superset-frontend/src/dashboard/components/gridComponents/Column/Column.test.tsx:
##########
@@ -180,7 +223,7 @@ test('should call deleteComponent when deleted', () => {
test('should pass its own width as availableColumnCount to children', () => {
const { getByTestId } = setup();
expect(getByTestId('mock-dashboard-component')).toHaveTextContent(
- props.component.meta.width,
+ `${columnWithoutChildren.meta.width}`,
Review Comment:
**Suggestion:** The test that asserts the `availableColumnCount` passed to
`DashboardComponent` uses `columnWithoutChildren.meta.width` in its
expectation, but this specific test does not render `Column` with
`columnWithoutChildren` as the `component` prop, so it is effectively asserting
against an unrelated object and may fail or become brittle if
`columnWithoutChildren`'s width diverges from the layout used by the store.
Using the width from `mockLayout.present.COLUMN_ID.meta.width` (the layout
actually backing the rendered component when calling `setup()` without
overrides) makes the test assertion consistent with the component under test.
[logic error]
**Severity Level:** Minor ⚠️
```suggestion
`${mockLayout.present.COLUMN_ID.meta.width}`,
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
In this test, `setup()` is called without overrides, so the `Column` is
rendered with `component: mockLayout.present.COLUMN_ID`, not
`columnWithoutChildren`. That means `columnWithoutChildren.meta.width` is never
used by the component in this scenario; it's just a local test helper.
Asserting against `columnWithoutChildren.meta.width` couples the expectation to
an unrelated object and can become flat-out wrong if `columnWithoutChildren`'s
width diverges from `mockLayout.present.COLUMN_ID.meta.width`. Switching the
assertion to use `mockLayout.present.COLUMN_ID.meta.width` aligns the test with
the actual data driving the rendered component and fixes a real
logic/brittleness issue in the test, not just style.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/gridComponents/Column/Column.test.tsx
**Line:** 226:226
**Comment:**
*Logic Error: The test that asserts the `availableColumnCount` passed
to `DashboardComponent` uses `columnWithoutChildren.meta.width` in its
expectation, but this specific test does not render `Column` with
`columnWithoutChildren` as the `component` prop, so it is effectively asserting
against an unrelated object and may fail or become brittle if
`columnWithoutChildren`'s width diverges from the layout used by the store.
Using the width from `mockLayout.present.COLUMN_ID.meta.width` (the layout
actually backing the rendered component when calling `setup()` without
overrides) makes the test assertion consistent with the component under test.
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>
--
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]
