codeant-ai-for-open-source[bot] commented on code in PR #37975:
URL: https://github.com/apache/superset/pull/37975#discussion_r2807340302
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx:
##########
@@ -189,6 +189,23 @@ test('should render the error', async () => {
expect(screen.getByText('Error: Something went wrong')).toBeInTheDocument();
});
+test('should only allow page size of 50 in pagination', async () => {
+ fetchWithData();
+ await waitForRender();
+ // The page size selector should only show 50 as an option
+ const pageSizeSelector = document.querySelector(
+ '.ant-pagination-options .ant-select-selection-item',
+ );
+ if (pageSizeSelector) {
+ expect(pageSizeSelector.textContent).toContain('50');
+ }
Review Comment:
**Suggestion:** The test only asserts the page size text when the pagination
selector exists, so if the selector is missing (e.g., due to a regression where
pagination is not rendered), the `if` guard prevents any assertion from running
and the test will still pass, masking real failures. Remove the conditional and
explicitly assert that the selector exists before checking its text so the test
fails when pagination is not present. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Pagination regression in DrillDetailPane may pass Jest tests.
- ⚠️ Test `should only allow page size` yields false confidence.
```
</details>
```suggestion
expect(pageSizeSelector).not.toBeNull();
expect(pageSizeSelector?.textContent).toContain('50');
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test file
`superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx`,
focusing on
`test('should only allow page size of 50 in pagination', ...)` at lines
192–207.
2. Within this test, after `await waitForRender();`, the code executes
`document.querySelector('.ant-pagination-options
.ant-select-selection-item')` at lines
196–198 to locate the page size selector in the rendered DrillDetailPane.
3. In any regression where DrillDetailPane stops rendering this element (for
example, the
Ant Design pagination is removed or its class name changes so the selector
no longer
matches), `document.querySelector` returns `null`, so `pageSizeSelector` is
`null` at line
196.
4. Because of the `if (pageSizeSelector) { ... }` guard at lines 199–201,
the assertion on
its text is skipped when `pageSizeSelector` is `null`, and the test still
passes as long
as the later negative assertions (lines 203–206) remain true, thereby
failing to detect
that pagination is missing or broken.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx
**Line:** 199:201
**Comment:**
*Logic Error: The test only asserts the page size text when the
pagination selector exists, so if the selector is missing (e.g., due to a
regression where pagination is not rendered), the `if` guard prevents any
assertion from running and the test will still pass, masking real failures.
Remove the conditional and explicitly assert that the selector exists before
checking its text so the test fails when pagination is not present.
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%2F37975&comment_hash=26f71edee8715e277301b8c0342a9fc2986eb0b04f9c70e3472b90eba5cb8df7&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37975&comment_hash=26f71edee8715e277301b8c0342a9fc2986eb0b04f9c70e3472b90eba5cb8df7&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]
