codeant-ai-for-open-source[bot] commented on code in PR #40449: URL: https://github.com/apache/superset/pull/40449#discussion_r3534508661
########## superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx: ########## @@ -0,0 +1,143 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen, fireEvent } from 'spec/helpers/testing-library'; +import { DrillDownBreadcrumb } from './DrillDownBreadcrumb'; +import { DrillDownLevel } from './types'; + +const hierarchy = ['country', 'region', 'city']; + +const drillStack: DrillDownLevel[] = [ + { + column: 'country', + filters: [{ col: 'country', op: '==', val: 'USA' }], + label: 'USA', + }, + { + column: 'region', + filters: [{ col: 'region', op: '==', val: 'Texas' }], + label: 'Texas', + }, +]; + +test('breadcrumb returns null when drillStack is empty and no selectedLeaf', () => { + const { container } = render( + <DrillDownBreadcrumb + hierarchy={hierarchy} + drillStack={[]} + onJumpTo={jest.fn()} + />, + ); + + expect(container.firstChild).toBeNull(); +}); + +test('breadcrumb renders hierarchy[0] and drill labels', () => { + render( + <DrillDownBreadcrumb + hierarchy={hierarchy} + drillStack={drillStack} + onJumpTo={jest.fn()} + />, + ); + + // The root column name should be rendered + expect(screen.getByText('country')).toBeInTheDocument(); + // Drill labels should be rendered + expect(screen.getByText('USA')).toBeInTheDocument(); + expect(screen.getByText('Texas')).toBeInTheDocument(); +}); + +test('clicking a breadcrumb segment calls onJumpTo with correct depth', () => { + const onJumpTo = jest.fn(); + + render( + <DrillDownBreadcrumb + hierarchy={hierarchy} + drillStack={drillStack} + onJumpTo={onJumpTo} + />, + ); + + // Click the root segment (hierarchy[0]) — should call onJumpTo(0) + fireEvent.click(screen.getByText('country')); + expect(onJumpTo).toHaveBeenCalledWith(0); + + // Click the first drill label 'USA' — should call onJumpTo(1) + fireEvent.click(screen.getByText('USA')); + expect(onJumpTo).toHaveBeenCalledWith(1); +}); + +test('keyboard Enter on breadcrumb calls onJumpTo', () => { + const onJumpTo = jest.fn(); + + render( + <DrillDownBreadcrumb + hierarchy={hierarchy} + drillStack={drillStack} + onJumpTo={onJumpTo} + />, + ); + + // Press Enter on the root segment + fireEvent.keyDown(screen.getByText('country'), { key: 'Enter' }); + expect(onJumpTo).toHaveBeenCalledWith(0); + + // Press Enter on the 'USA' segment + fireEvent.keyDown(screen.getByText('USA'), { key: 'Enter' }); + expect(onJumpTo).toHaveBeenCalledWith(1); +}); + +test('selectedLeaf is rendered as non-clickable text', () => { + const onJumpTo = jest.fn(); + + render( + <DrillDownBreadcrumb + hierarchy={hierarchy} + drillStack={drillStack} + selectedLeaf="Austin" + onJumpTo={onJumpTo} + />, + ); + + expect(screen.getByText('Austin')).toBeInTheDocument(); + + // The leaf should not have role="button" + const leafElement = screen.getByText('Austin'); + expect(leafElement).not.toHaveAttribute('role', 'button'); +}); + +test('last drill label is not clickable when there is no selectedLeaf', () => { + const onJumpTo = jest.fn(); + + render( + <DrillDownBreadcrumb + hierarchy={hierarchy} + drillStack={drillStack} + onJumpTo={onJumpTo} + />, + ); + + // 'Texas' is the last item and no selectedLeaf, so it should not be clickable + const texasElement = screen.getByText('Texas'); + expect(texasElement).not.toHaveAttribute('role', 'button'); + + fireEvent.click(texasElement); + // onJumpTo should only have been called for the root click test, not for Texas Review Comment: **Suggestion:** The assertion is correct, but the inline comment is factually wrong for this test case and can mislead future edits: this test never performs a root click, so saying the callback should only be called from a root click contradicts the actual setup. Update the comment to match the test’s behavior (no callback calls expected at all). [comment mismatch] <details> <summary><b>Severity Level:</b> Minor 🧹</summary> ```mdx - ⚠️ Test documentation for drill breadcrumb behavior is misleading. - ⚠️ Future maintainers may misinterpret expected callback usage. - ⚠️ Confusion when updating drill-down click handling tests. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open `superset-frontend/src/components/Chart/DrillDown/DrillDownBreadcrumb.test.tsx` and locate the test `last drill label is not clickable when there is no selectedLeaf` at lines 125–143 (as read from the repository). 2. Observe that within this test, `onJumpTo` is defined as `const onJumpTo = jest.fn();` at line 126 and is passed to `DrillDownBreadcrumb` in the render call at lines 128–133. 3. Note that the only user interaction in this test is a click on the `Texas` breadcrumb element at line 140, and the final assertion at line 142 is `expect(onJumpTo).not.toHaveBeenCalled();`, confirming that `onJumpTo` is expected to have zero calls in this test. 4. Compare this behavior with the inline comment at line 141: `// onJumpTo should only have been called for the root click test, not for Texas`. Since this specific test never performs a root click, the comment is factually incorrect and suggests a callback invocation pattern (root click then Texas click) that does not exist here, which can mislead future test edits. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cab3ed9b4fa648bcb4fbbe1dfdf945b4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=cab3ed9b4fa648bcb4fbbe1dfdf945b4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <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/DrillDown/DrillDownBreadcrumb.test.tsx **Line:** 141:141 **Comment:** *Comment Mismatch: The assertion is correct, but the inline comment is factually wrong for this test case and can mislead future edits: this test never performs a root click, so saying the callback should only be called from a root click contradicts the actual setup. Update the comment to match the test’s behavior (no callback calls expected at all). 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. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=5c8e07b6ef05244dc1b3a5f4bf21cf03ced2e6245bca779e3dc996dad7e07316&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=5c8e07b6ef05244dc1b3a5f4bf21cf03ced2e6245bca779e3dc996dad7e07316&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]
