Abdulrehman-PIAIC80387 commented on code in PR #42529:
URL: https://github.com/apache/superset/pull/42529#discussion_r3680997449


##########
superset-frontend/src/features/home/Menu.test.tsx:
##########
@@ -1052,6 +1052,32 @@ describe('active tab highlighting (regression #36403)', 
() => {
     
expect(getMenuItemByText('Дашборды')).toHaveClass('ant-menu-item-selected');
   });
 
+  test.each([
+    ['/dataset/add/', 'the modern React dataset create route'],
+    ['/dataset/42', 'a dataset detail route'],
+  ])(
+    'highlights the Datasets tab on %s (%s) — regression #42467',
+    async (route, _label) => {
+      // Legacy list URL (``/tablemodelview/list/``) already highlights the
+      // tab; these newer React-managed routes did not until #42467 added
+      // ``/dataset`` as a second prefix in the active-tab matcher.
+      useSelectorMock.mockReturnValue({ roles: user.roles });
+      window.history.pushState({}, '', route);
+
+      render(<Menu {...mockedProps} />, {
+        useRedux: true,
+        useQueryParams: true,
+        useRouter: true,
+        useTheme: true,
+      });
+
+      await screen.findByText('Datasets');
+      expect(getMenuItemByText('Datasets')).toHaveClass(
+        'ant-menu-item-selected',
+      );
+    },
+  );

Review Comment:
   Good call — added in 87e404647e. The test.each now includes 
/tablemodelview/list/ as the first case alongside /dataset/add/ and 
/dataset/42, so the legacy FAB path is verified rather than only 
asserted-in-comment. Locks in coverage against future prefix-matching changes.



-- 
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]

Reply via email to