From 87a05a5b2e40a67492e004b7851e0b62b33cca85 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 10 Dec 2019 15:08:59 -0500 Subject: [PATCH] Testing Improvements and Refactoring --- .../InventoryGroup/InventoryGroup.jsx | 44 +++--- .../InventoryGroup/InventoryGroup.test.jsx | 16 +- .../InventoryGroupAdd/InventoryGroupAdd.jsx | 6 +- .../InventoryGroupAdd.test.jsx | 25 ++- .../InventoryGroupDetail.jsx | 144 +++++++++--------- .../InventoryGroupDetail.test.jsx | 4 +- .../InventoryGroupEdit/InventoryGroupEdit.jsx | 11 +- .../InventroyGroupEdit.test.jsx | 25 ++- .../InventoryGroupForm.test.jsx | 1 - .../InventoryGroups/InventoryGroups.jsx | 53 +++---- .../InventoryGroups/InventoryGroups.test.jsx | 10 +- 11 files changed, 173 insertions(+), 166 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.jsx index f5fbada3fe..b205182c04 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.jsx @@ -14,8 +14,8 @@ import InventoryGroupDetail from '../InventoryGroupDetail/InventoryGroupDetail'; function InventoryGroups({ i18n, match, setBreadcrumb, inventory, history }) { const [inventoryGroup, setInventoryGroup] = useState(null); - const [hasContentLoading, setContentLoading] = useState(true); - const [hasContentError, setHasContentError] = useState(false); + const [hasContentLoading, setHasContentLoading] = useState(true); + const [contentError, setHasContentError] = useState(null); useEffect(() => { const loadData = async () => { @@ -26,12 +26,18 @@ function InventoryGroups({ i18n, match, setBreadcrumb, inventory, history }) { } catch (err) { setHasContentError(err); } finally { - setContentLoading(false); + setHasContentLoading(false); } }; loadData(); - }, [match.params.groupId, setBreadcrumb, inventory]); + }, [ + history.location.pathname, + match.params.groupId, + inventory, + setBreadcrumb, + ]); + const tabsArray = [ { name: i18n._(t`Return to Groups`), @@ -46,7 +52,7 @@ function InventoryGroups({ i18n, match, setBreadcrumb, inventory, history }) { id: 0, }, { - name: i18n._(t`RelatedGroups`), + name: i18n._(t`Related Groups`), link: `/inventories/inventory/${inventory.id}/groups/${inventoryGroup && inventoryGroup.id}/nested_groups`, id: 1, @@ -58,26 +64,28 @@ function InventoryGroups({ i18n, match, setBreadcrumb, inventory, history }) { id: 2, }, ]; - if (hasContentError) { - return ; + if (contentError) { + return ; } if (hasContentLoading) { return ; } - let cardHeader = hasContentLoading ? null : ( - - - - - ); + + let cardHeader = null; if ( - !history.location.pathname.includes('groups/') || - history.location.pathname.endsWith('edit') + history.location.pathname.includes('groups/') && + !history.location.pathname.endsWith('edit') ) { - cardHeader = null; + cardHeader = ( + + + + + ); } + return ( <> {cardHeader} diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.test.jsx index afec079a6d..4c1b50c95b 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroup/InventoryGroup.test.jsx @@ -45,26 +45,20 @@ describe('', () => { } ); }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); }); afterEach(() => { wrapper.unmount(); }); test('renders successfully', async () => { - await act(async () => { - waitForElement(wrapper, 'ContentLoading', el => el.length === 0); - }); expect(wrapper.length).toBe(1); - expect(wrapper.find('button[aria-label="Return to Groups"]').length).toBe( - 1 - ); }); - test('expect Return to Groups tab to exist', async () => { - await act(async () => { - waitForElement(wrapper, 'ContentLoading', el => el.length === 0); - }); - expect(wrapper.length).toBe(1); + test('expect all tabs to exist, including Return to Groups', async () => { expect(wrapper.find('button[aria-label="Return to Groups"]').length).toBe( 1 ); + expect(wrapper.find('button[aria-label="Details"]').length).toBe(1); + expect(wrapper.find('button[aria-label="Related Groups"]').length).toBe(1); + expect(wrapper.find('button[aria-label="Hosts"]').length).toBe(1); }); }); diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupAdd/InventoryGroupAdd.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupAdd/InventoryGroupAdd.jsx index 248309b005..720e9d4b3c 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupAdd/InventoryGroupAdd.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupAdd/InventoryGroupAdd.jsx @@ -7,8 +7,10 @@ import { Card } from '@patternfly/react-core'; import InventoryGroupForm from '../InventoryGroupForm/InventoryGroupForm'; function InventoryGroupsAdd({ history, inventory, setBreadcrumb }) { - useEffect(() => setBreadcrumb(inventory), [inventory, setBreadcrumb]); const [error, setError] = useState(null); + + useEffect(() => setBreadcrumb(inventory), [inventory, setBreadcrumb]); + const handleSubmit = async values => { values.inventory = inventory.id; try { @@ -18,9 +20,11 @@ function InventoryGroupsAdd({ history, inventory, setBreadcrumb }) { setError(err); } }; + const handleCancel = () => { history.push(`/inventories/inventory/${inventory.id}/groups`); }; + return ( ', () => { let history; beforeEach(async () => { history = createMemoryHistory({ - initialEntries: ['/inventories/inventory/1/groups'], + initialEntries: ['/inventories/inventory/1/groups/add'], }); await act(async () => { wrapper = mountWithContexts( - {}} - inventory={{ inventory: { id: 1 } }} + ( + {}} inventory={{ id: 1 }} /> + )} />, { context: { - router: { - history, - }, + router: { history, route: { location: history.location } }, }, } ); @@ -38,17 +40,12 @@ describe('', () => { expect(wrapper.length).toBe(1); }); test('cancel should navigate user to Inventory Groups List', async () => { - await act(async () => { - waitForElement(wrapper, 'isLoading', el => el.length === 0); - }); + wrapper.find('button[aria-label="Cancel"]').simulate('click'); expect(history.location.pathname).toEqual( '/inventories/inventory/1/groups' ); }); test('handleSubmit should call api', async () => { - await act(async () => { - waitForElement(wrapper, 'isLoading', el => el.length === 0); - }); await act(async () => { wrapper.find('InventoryGroupForm').prop('handleSubmit')({ name: 'Bar', diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.jsx index 7244385471..7b383e986f 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.jsx @@ -29,6 +29,9 @@ const ActionButtonWrapper = styled.div` } `; function InventoryGroupDetail({ i18n, history, match, inventoryGroup }) { + const { + summary_fields: { created_by, modified_by }, + } = inventoryGroup; const [error, setError] = useState(false); const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); @@ -41,52 +44,7 @@ function InventoryGroupDetail({ i18n, history, match, inventoryGroup }) { setError(err); } }; - if (error) { - return ( - setError(false)} - > - {i18n._(t`Failed to delete group ${inventoryGroup.name}.`)} - - - ); - } - if (isDeleteModalOpen) { - return ( - setIsDeleteModalOpen(false)} - actions={[ - , - , - ]} - > - {i18n._(t`Are you sure you want to delete:`)} -
- {inventoryGroup.name} -
-
- ); - } + return ( @@ -104,32 +62,32 @@ function InventoryGroupDetail({ i18n, history, match, inventoryGroup }) { label={i18n._(t`Variables`)} /> - - {i18n._(t`${formatDateString(inventoryGroup.created)} by`)}{' '} - - {inventoryGroup.summary_fields.created_by.username} - - - } - /> - - {i18n._(t`${formatDateString(inventoryGroup.modified)} by`)}{' '} - - {inventoryGroup.summary_fields.modified_by.username} - - - } - /> + {created_by && created_by.username && ( + + {i18n._(t`${formatDateString(inventoryGroup.created)} by`)}{' '} + + {created_by.username} + + + } + /> + )} + {modified_by && modified_by.username && ( + + {i18n._(t`${formatDateString(inventoryGroup.modified)} by`)}{' '} + + {modified_by.username} + + + } + /> + )} , + , + ]} + > + {i18n._(t`Are you sure you want to delete:`)} +
+ {inventoryGroup.name} +
+ + )} + {error && ( + setError(false)} + > + {i18n._(t`Failed to delete group ${inventoryGroup.name}.`)} + + + )}
); } diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.test.jsx index 8e82fea394..99a017ce32 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupDetail/InventoryGroupDetail.test.jsx @@ -33,7 +33,7 @@ describe('', () => { beforeEach(async () => { await act(async () => { history = createMemoryHistory({ - initialEntries: ['/inventories/inventory/1/groups/1/edit'], + initialEntries: ['/inventories/inventory/1/groups/1/details'], }); wrapper = mountWithContexts( ', () => { expect(GroupsAPI.destroy).toBeCalledWith(1); }); test('should navigate user to edit form on edit button click', async () => { - wrapper.find('button[aria-label="Edit"]').prop('onClick'); + wrapper.find('button[aria-label="Edit"]').simulate('click'); expect(history.location.pathname).toEqual( '/inventories/inventory/1/groups/1/edit' ); diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupEdit/InventoryGroupEdit.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupEdit/InventoryGroupEdit.jsx index 42d2fcde2a..6ff0e58c58 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupEdit/InventoryGroupEdit.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupEdit/InventoryGroupEdit.jsx @@ -11,19 +11,20 @@ function InventoryGroupEdit({ history, inventoryGroup, inventory, match }) { const handleSubmit = async values => { try { await GroupsAPI.update(match.params.groupId, values); + history.push( + `/inventories/inventory/${inventory.id}/groups/${inventoryGroup.id}` + ); } catch (err) { setError(err); - } finally { - history.push( - `/inventories/inventory/${inventory.id}/groups/${inventoryGroup.id}/details` - ); } }; + const handleCancel = () => { history.push( - `/inventories/inventory/${inventory.id}/groups/${inventoryGroup.id}/details` + `/inventories/inventory/${inventory.id}/groups/${inventoryGroup.id}` ); }; + return ( ', () => { let history; beforeEach(async () => { history = createMemoryHistory({ - initialEntries: ['/inventories/1/groups'], + initialEntries: ['/inventories/inventory/1/groups/2/edit'], }); await act(async () => { wrapper = mountWithContexts( - ( + {}} + inventory={{ id: 1 }} + inventoryGroup={{ id: 2 }} + /> + )} />, { context: { @@ -35,6 +42,7 @@ describe('', () => { match: { params: { groupId: 13 }, }, + location: history.location, }, }, }, @@ -49,11 +57,12 @@ describe('', () => { expect(wrapper.length).toBe(1); }); test('cancel should navigate user to Inventory Groups List', async () => { - await waitForElement(wrapper, 'isLoading', el => el.length === 0); - expect(history.location.pathname).toEqual('/inventories/1/groups'); + wrapper.find('button[aria-label="Cancel"]').simulate('click'); + expect(history.location.pathname).toEqual( + '/inventories/inventory/1/groups/2' + ); }); test('handleSubmit should call api', async () => { - await waitForElement(wrapper, 'isLoading', el => el.length === 0); wrapper.find('InventoryGroupForm').prop('handleSubmit')({ name: 'Bar', description: 'Ansible', diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupForm/InventoryGroupForm.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupForm/InventoryGroupForm.test.jsx index 2c3484e1b6..ebf459f76f 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupForm/InventoryGroupForm.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroupForm/InventoryGroupForm.test.jsx @@ -26,7 +26,6 @@ describe('', () => { expect(wrapper.length).toBe(1); }); test('should render values for the fields that have them', () => { - expect(wrapper.length).toBe(1); expect(wrapper.find("FormGroup[label='Name']").length).toBe(1); expect(wrapper.find("FormGroup[label='Description']").length).toBe(1); expect(wrapper.find("VariablesField[label='Variables']").length).toBe(1); diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.jsx index 0e3d82a8b0..e35e2dedb7 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.jsx @@ -11,37 +11,32 @@ import InventoryGroupsList from './InventoryGroupsList'; function InventoryGroups({ setBreadcrumb, inventory, location, match }) { return ( - {[ - { - return ; - }} - />, - { - return ( - - ); - }} - />, - ( - { + return ( + - )} - />, - ]} + ); + }} + /> + ( + + )} + /> + { + return ; + }} + /> ); } diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.test.jsx index 2b5a7340c0..8c60d8bfbd 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroups/InventoryGroups.test.jsx @@ -4,7 +4,7 @@ import { Route } from 'react-router-dom'; import { createMemoryHistory } from 'history'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import { InventoriesAPI, GroupsAPI } from '@api'; -import InventoryGroups from './InventoryGroups'; +import InventoryGroupsList from './InventoryGroupsList'; jest.mock('@api'); @@ -50,7 +50,7 @@ const mockGroups = [ }, ]; -describe('', () => { +describe('', () => { let wrapper; beforeEach(async () => { @@ -75,7 +75,7 @@ describe('', () => { wrapper = mountWithContexts( } + component={() => } />, { context: { @@ -88,7 +88,7 @@ describe('', () => { }); test('initially renders successfully', () => { - expect(wrapper.find('InventoryGroups').length).toBe(1); + expect(wrapper.find('InventoryGroupsList').length).toBe(1); }); test('should fetch groups from api and render them in the list', async () => { @@ -147,7 +147,7 @@ describe('', () => { Promise.reject(new Error()) ); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); await waitForElement(wrapper, 'ContentError', el => el.length === 1); });