bito-code-review[bot] commented on code in PR #44345:
URL: https://github.com/apache/superset/pull/44345#discussion_r4080945009
##########
superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.test.tsx:
##########
@@ -265,6 +265,144 @@ test('Switching tabs', async () => {
expect(props.onChangeTab).toHaveBeenCalled();
});
+test.each([false, true])(
+ 'A childless TABS component does not register an active tab (editMode=%s)',
+ editMode => {
+ // Regression guard for the permalink failure caused by empty tab
containers:
+ // a TABS component with no children has no tab id to activate, so
resolving
+ // `children[tabIndex]` yields `undefined`. Dispatching that into
+ // `dashboardState.activeTabs` puts an `undefined` entry in the array,
which
+ // `JSON.stringify` coerces to `null` in the permalink request body.
+ const props = createProps();
+ props.editMode = editMode;
+ props.component.children = [];
+
+ render(<Tabs {...props} />, {
+ useRedux: true,
+ useDnd: true,
+ });
+
+ expect(props.setActiveTab).not.toHaveBeenCalled();
+ },
+);
+
+test.each([false, true])(
+ 'The first child added to an empty TABS component is activated
(editMode=%s)',
+ editMode => {
+ // Counterpart to the guard above: skipping registration while there is no
+ // tab id must not leave the container permanently unregistered.
`activeKey`
+ // is seeded once from state, so an empty container that later receives its
+ // first child has to resolve and register that child, otherwise the tab
+ // renders unselected and never reaches dashboardState.activeTabs.
+ const props = createProps();
+ const tabId = props.component.children[0];
+ props.editMode = editMode;
+ props.component.children = [];
+ const { rerender } = render(<Tabs {...props} />, {
+ useRedux: true,
+ useDnd: true,
+ });
+
+ expect(props.setActiveTab).not.toHaveBeenCalled();
+ rerender(
+ <Tabs {...props} component={{ ...props.component, children: [tabId] }}
/>,
+ );
+
+ // Exactly one registration, carrying the resolved id and no stale previous
+ // tab -- never an `undefined` that JSON.stringify would turn into `null`.
+ expect(props.setActiveTab.mock.calls).toEqual([[tabId]]);
+ expect(screen.getByRole('tab')).toHaveAttribute('aria-selected', 'true');
+ },
+);
+
+test('A tab added after deleting the last tab is selected and registered',
async () => {
+ const props = createProps();
+ const [deletedTabId, newTabId] = props.component.children;
+ props.component.children = [deletedTabId];
+ const { rerender } = render(<Tabs {...props} />, {
+ useRedux: true,
+ useDnd: true,
+ });
+
+ expect(props.setActiveTab.mock.calls).toEqual([[deletedTabId]]);
+ expect(screen.getByRole('tab')).toHaveAttribute('aria-selected', 'true');
+
+ await userEvent.click(screen.getByRole('button', { name: 'remove' }));
+ await userEvent.click(screen.getByRole('button', { name: 'Delete' }));
+ expect(props.deleteComponent).toHaveBeenCalledWith(
+ deletedTabId,
+ props.component.id,
+ );
+
+ rerender(
+ <Tabs {...props} component={{ ...props.component, children: [] }} />,
+ );
+ expect(screen.queryByRole('tab')).not.toBeInTheDocument();
+ props.setActiveTab.mockClear();
+
+ await userEvent.click(screen.getByRole('button', { name: 'Add tab' }));
+ expect(props.createComponent).toHaveBeenCalled();
+ expect(props.setActiveTab).not.toHaveBeenCalled();
+
+ rerender(
+ <Tabs
+ {...props}
+ component={{ ...props.component, children: [newTabId] }}
+ />,
+ );
+
+ expect(props.setActiveTab.mock.calls).toEqual([[newTabId, deletedTabId]]);
Review Comment:
<!-- Bito Reply -->
The assessment that the assertion is not vacuous is correct. Since the
`rerender` is wrapped in `act`, the effects are flushed synchronously before
the assertion runs, ensuring the `mock.calls` are populated if the registration
occurred. Wrapping this in `waitFor` would be redundant as the assertion
already correctly validates the state after the effect has processed.
**superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.test.tsx**
```
await waitFor(() =>
expect(props.setActiveTab.mock.calls).toEqual([[newTabId,
deletedTabId]]),
);
```
--
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]
