bito-code-review[bot] commented on code in PR #40746:
URL: https://github.com/apache/superset/pull/40746#discussion_r3472084221
##########
superset-frontend/src/features/tags/BulkTagModal.test.tsx:
##########
@@ -36,77 +36,80 @@ const mockedProps = {
resourceName: 'dashboard',
};
-afterEach(() => {
- fetchMock.clearHistory().removeRoutes();
- jest.clearAllMocks();
-});
-
-test('should render', () => {
- const { container } = render(<BulkTagModal {...mockedProps} />);
- expect(container).toBeInTheDocument();
-});
+// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
+describe('BulkTagModal', () => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Test pattern regression</b></div>
<div id="fix">
This diff reintroduces `describe()` blocks which contradicts the project's
stated testing strategy in AGENTS.md: 'Use `test()` instead of `describe()`'
with reference to 'avoid nesting when testing' principles. The eslint-disable
comment on line 39 acknowledges this migration goes against project guidance.
Flat test structures reduce cognitive load and improve test isolation.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset-frontend/src/features/tags/BulkTagModal.test.tsx
+++ superset-frontend/src/features/tags/BulkTagModal.test.tsx
@@ -36,8 +36,6 @@ const mockedProps = {
resourceName: 'dashboard',
};
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('BulkTagModal', () => {
afterEach(() => {
fetchMock.clearHistory().removeRoutes();
jest.clearAllMocks();
@@ -45,7 +43,6 @@ describe('BulkTagModal', () => {
test('should render', () => {
const { container } = render(<BulkTagModal {...mockedProps} />);
expect(container).toBeInTheDocument();
});
-
test('renders the correct title and message', () => {
render(<BulkTagModal {...mockedProps} />);
@@ -62,7 +59,6 @@ test('renders tags input field', async () => {
expect(tagsInput).toBeInTheDocument();
});
test('calls onHide when the Cancel button is clicked', () => {
render(<BulkTagModal {...mockedProps} />);
const cancelButton = screen.getByText('Cancel');
fireEvent.click(cancelButton);
expect(mockedProps.onHide).toHaveBeenCalled();
});
-
test('submits the selected tags and shows success toast', async () => {
fetchMock.post('glob:*/api/v1/tag/bulk_create', {
result: {
@@ -94,7 +89,6 @@ test('submits the selected tags and shows success toast',
async () => {
expect(mockedProps.onHide).toHaveBeenCalled();
});
test('handles API errors gracefully', async () => {
fetchMock.post('glob:*/api/v1/tag/bulk_create', 500);
render(<BulkTagModal {...mockedProps} />);
const tagsInput = await screen.findByRole('combobox', { name: /tags/i });
fireEvent.change(tagsInput, { target: { value: 'Test Tag' } });
fireEvent.keyDown(tagsInput, { key: 'Enter', code: 'Enter' });
fireEvent.click(screen.getByText('Save'));
await waitFor(() => {
expect(mockedProps.addDangerToast).toHaveBeenCalledWith(
'Failed to tag items',
);
});
});
-});
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/01c0dc0/AGENTS.md#L43";>AGENTS.md:43</a>
</li>
</ul>
</details>
<small><i>Code Review Run #0d2268</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
