[GitHub] [superset] michael-s-molina commented on a change in pull request #16483: refactor: Changes the DatabaseSelector and TableSelector to use the new Select component

[GitBox <gi...@apache.org>]

michael-s-molina commented on a change in pull request #16483:
URL: https://github.com/apache/superset/pull/16483#discussion_r700267681



##########
File path: superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx
##########
@@ -153,50 +153,62 @@ test('Refresh should work', async () => {
 
   render(<DatabaseSelector {...props} />);
 
+  const select = screen.getByRole('combobox', {
+    name: 'Select a schema',
+  });
+
+  userEvent.click(select);
+
   await waitFor(() => {
-    expect(SupersetClientGet).toBeCalledTimes(2);
-    expect(props.getDbList).toBeCalledTimes(1);
+    expect(SupersetClientGet).toBeCalledTimes(1);
+    expect(props.getDbList).toBeCalledTimes(0);
     expect(props.getTableList).toBeCalledTimes(0);
     expect(props.handleError).toBeCalledTimes(0);
     expect(props.onDbChange).toBeCalledTimes(0);
     expect(props.onSchemaChange).toBeCalledTimes(0);
-    expect(props.onSchemasLoad).toBeCalledTimes(1);
+    expect(props.onSchemasLoad).toBeCalledTimes(0);
     expect(props.onUpdate).toBeCalledTimes(0);
   });
 
-  userEvent.click(screen.getByRole('button'));
+  userEvent.click(screen.getByRole('button', { name: 'refresh' }));
 
   await waitFor(() => {
-    expect(SupersetClientGet).toBeCalledTimes(3);
-    expect(props.getDbList).toBeCalledTimes(1);
+    expect(SupersetClientGet).toBeCalledTimes(2);
+    expect(props.getDbList).toBeCalledTimes(0);
     expect(props.getTableList).toBeCalledTimes(0);
     expect(props.handleError).toBeCalledTimes(0);
-    expect(props.onDbChange).toBeCalledTimes(1);
-    expect(props.onSchemaChange).toBeCalledTimes(1);
+    expect(props.onDbChange).toBeCalledTimes(0);
+    expect(props.onSchemaChange).toBeCalledTimes(0);
     expect(props.onSchemasLoad).toBeCalledTimes(2);
-    expect(props.onUpdate).toBeCalledTimes(1);
+    expect(props.onUpdate).toBeCalledTimes(0);
   });
 });
 
 test('Should database select display options', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />);
-  const selector = await screen.findByText('Database:');
-  expect(selector).toBeInTheDocument();
-  expect(selector.parentElement).toHaveTextContent(
-    'Database:postgresql examples',
-  );
+  const select = screen.getByRole('combobox', {
+    name: 'Select a database',
+  });
+  expect(select).toBeInTheDocument();
+  userEvent.click(select);
+  expect(
+    await screen.findByRole('option', { name: 'postgresql: test' }),

Review comment:
       Great suggestion. I created two new tests to catch the previous regression:
   - Sends the correct db when changing the database
   - Sends the correct schema when changing the schema




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org