From 98daefd7bae4610b3d6941b72771412a201e0ce1 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Wed, 10 May 2023 10:07:19 +1200 Subject: [PATCH] MNT Apply changes requested from peer review (#1058) --- .../tests/AbstractAction-test.js | 2 +- .../tests/ArchiveAction-test.js | 4 +- .../tests/DuplicateAction-test.js | 4 +- .../tests/PublishAction-test.js | 4 +- .../ElementActions/tests/SaveAction-test.js | 6 +-- .../tests/UnpublishAction-test.js | 6 +-- .../ElementEditor/tests/Content-test.js | 20 ++++----- .../ElementEditor/tests/Element-test.js | 19 ++++---- .../tests/ElementActions-test.js | 4 +- .../ElementEditor/tests/ElementEditor-test.js | 4 +- .../ElementEditor/tests/ElementList-test.js | 10 ++--- .../ElementEditor/tests/Header-test.js | 44 ++++++++++--------- .../ElementEditor/tests/Summary-test.js | 9 ++-- .../tests/HistoricElementView-test.js | 4 +- 14 files changed, 72 insertions(+), 68 deletions(-) diff --git a/client/src/components/ElementActions/tests/AbstractAction-test.js b/client/src/components/ElementActions/tests/AbstractAction-test.js index d337d330..293541c0 100644 --- a/client/src/components/ElementActions/tests/AbstractAction-test.js +++ b/client/src/components/ElementActions/tests/AbstractAction-test.js @@ -18,7 +18,7 @@ function makeProps(obj = {}) { test('AbstractAction renders a DropdownItem', () => { const { container } = render(); - expect(container.querySelector('.dropdown-item')).not.toBeNull(); + expect(container.querySelectorAll('.dropdown-item')).toHaveLength(1); }); test('AbstractAction includes the title text', () => { diff --git a/client/src/components/ElementActions/tests/ArchiveAction-test.js b/client/src/components/ElementActions/tests/ArchiveAction-test.js index 5ed5d153..c883dcd5 100644 --- a/client/src/components/ElementActions/tests/ArchiveAction-test.js +++ b/client/src/components/ElementActions/tests/ArchiveAction-test.js @@ -27,7 +27,7 @@ function makeProps(obj = {}) { }; } -test('ArchiveAction renders the wrapped component', () => { +test('ArchiveAction renders the title and class', () => { const { container } = render(); expect(container.querySelector('button.element-editor__actions-archive').textContent).toBe('Archive'); }); @@ -97,5 +97,5 @@ test('ArchiveAction renders a button even when block is broken', () => { } })} />); - expect(container.querySelector('button.element-editor__actions-archive')).not.toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-archive')).toHaveLength(1); }); diff --git a/client/src/components/ElementActions/tests/DuplicateAction-test.js b/client/src/components/ElementActions/tests/DuplicateAction-test.js index ab9c3da3..f4b6f6b1 100644 --- a/client/src/components/ElementActions/tests/DuplicateAction-test.js +++ b/client/src/components/ElementActions/tests/DuplicateAction-test.js @@ -29,7 +29,7 @@ function makeProps(obj = {}) { test('DuplicateAction renders a button', () => { const { container } = render(); - expect(container.querySelector('button.element-editor__actions-duplicate')).not.toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-duplicate')).toHaveLength(1); }); test('DuplicateAction is disabled when user doesn\'t have correct permissions', () => { @@ -52,5 +52,5 @@ test('DuplicateAction does not render a button when block is broken', () => { })} /> ); - expect(container.querySelector('button.element-editor__actions-duplicate')).toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-duplicate')).toHaveLength(0); }); diff --git a/client/src/components/ElementActions/tests/PublishAction-test.js b/client/src/components/ElementActions/tests/PublishAction-test.js index 9476bb50..3c39f86b 100644 --- a/client/src/components/ElementActions/tests/PublishAction-test.js +++ b/client/src/components/ElementActions/tests/PublishAction-test.js @@ -36,7 +36,7 @@ function makeProps(obj = {}) { }; } -test('PublishAction renders the wrapped component', () => { +test('PublishAction renders the title and class', () => { const { container } = render(); expect(container.querySelector('button.element-editor__actions-publish').textContent).toBe('Publish'); }); @@ -89,5 +89,5 @@ test('PublishAction does not render a button when block is broken', () => { })} /> ); - expect(container.querySelector('button.element-editor__actions-publish')).toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-publish')).toHaveLength(0); }); diff --git a/client/src/components/ElementActions/tests/SaveAction-test.js b/client/src/components/ElementActions/tests/SaveAction-test.js index d9119328..bab15830 100644 --- a/client/src/components/ElementActions/tests/SaveAction-test.js +++ b/client/src/components/ElementActions/tests/SaveAction-test.js @@ -34,7 +34,7 @@ test('SaveAction renders a button when block is expandable', () => { const { container } = render( ); - expect(container.querySelector('button.element-editor__actions-save')).not.toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-save')).toHaveLength(1); }); test('SaveAction does not render a button when block is not expandable', () => { @@ -44,7 +44,7 @@ test('SaveAction does not render a button when block is not expandable', () => { })} /> ); - expect(container.querySelector('button.element-editor__actions-save')).toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-save')).toHaveLength(0); }); test('SaveAction does not render a button when block is broken', () => { @@ -56,5 +56,5 @@ test('SaveAction does not render a button when block is broken', () => { })} /> ); - expect(container.querySelector('button.element-editor__actions-save')).toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-save')).toHaveLength(0); }); diff --git a/client/src/components/ElementActions/tests/UnpublishAction-test.js b/client/src/components/ElementActions/tests/UnpublishAction-test.js index ae28948a..5c695e58 100644 --- a/client/src/components/ElementActions/tests/UnpublishAction-test.js +++ b/client/src/components/ElementActions/tests/UnpublishAction-test.js @@ -29,7 +29,7 @@ function makeProps(obj = {}) { const WrappedComponent = (props) =>
{props.children}
; const ActionComponent = UnpublishAction(WrappedComponent); -test('UnpublishAction renders the wrapped component', () => { +test('UnpublishAction renders the title and class', () => { const { container } = render( ); @@ -45,7 +45,7 @@ test('UnpublishAction returns null when is not published', () => { })} /> ); - expect(container.querySelector('button')).toBeNull(); + expect(container.querySelectorAll('button')).toHaveLength(0); }); test('UnpublishAction calls the unpublish mutation', () => { @@ -84,5 +84,5 @@ test('UnpublishAction does not render a button when block is broken', () => { })} /> ); - expect(container.querySelector('button.element-editor__actions-unpublish')).toBeNull(); + expect(container.querySelectorAll('button.element-editor__actions-unpublish')).toHaveLength(0); }); diff --git a/client/src/components/ElementEditor/tests/Content-test.js b/client/src/components/ElementEditor/tests/Content-test.js index f7761433..f2fbf075 100644 --- a/client/src/components/ElementEditor/tests/Content-test.js +++ b/client/src/components/ElementEditor/tests/Content-test.js @@ -19,9 +19,9 @@ function makeProps(obj = {}) { test('Content should render the Summary component if the preview is not expanded', () => { const { container } = render(); - expect(container.querySelector('.element-editor-content')).not.toBeNull(); - expect(container.querySelector('.test-inline-edit-form')).toBeNull(); - expect(container.querySelector('.test-summary')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor-content')).toHaveLength(1); + expect(container.querySelectorAll('.test-inline-edit-form')).toHaveLength(0); + expect(container.querySelectorAll('.test-summary')).toHaveLength(1); }); test('Content should render the InlineEditForm component if the preview is expanded', () => { @@ -31,12 +31,12 @@ test('Content should render the InlineEditForm component if the preview is expan })} /> ); - expect(container.querySelector('.element-editor-content')).not.toBeNull(); - expect(container.querySelector('.test-inline-edit-form')).not.toBeNull(); - expect(container.querySelector('.test-summary')).toBeNull(); + expect(container.querySelectorAll('.element-editor-content')).toHaveLength(1); + expect(container.querySelectorAll('.test-inline-edit-form')).toHaveLength(1); + expect(container.querySelectorAll('.test-summary')).toHaveLength(0); }); -test('Content returns a div when no content or image is provided', () => { +test('Content renders even when no content or image is provided', () => { const { container } = render( { })} /> ); - expect(container.querySelector('.element-editor-content')).not.toBeNull(); - expect(container.querySelector('.test-inline-edit-form')).not.toBeNull(); - expect(container.querySelector('.test-summary')).toBeNull(); + expect(container.querySelectorAll('.element-editor-content')).toHaveLength(1); + expect(container.querySelectorAll('.test-inline-edit-form')).toHaveLength(1); + expect(container.querySelectorAll('.test-summary')).toHaveLength(0); }); diff --git a/client/src/components/ElementEditor/tests/Element-test.js b/client/src/components/ElementEditor/tests/Element-test.js index e25d82c2..73188d39 100644 --- a/client/src/components/ElementEditor/tests/Element-test.js +++ b/client/src/components/ElementEditor/tests/Element-test.js @@ -41,11 +41,11 @@ function makeProps(obj = {}) { test('Element should render the HeaderComponent and the ContentComponent', () => { const { container } = render(); - expect(container.querySelector('.element-editor__element .test-header')).not.toBeNull(); - expect(container.querySelector('.element-editor__element .test-content')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor__element .test-header')).toHaveLength(1); + expect(container.querySelectorAll('.element-editor__element .test-content')).toHaveLength(1); }); -test('Element should render null if no ID is given', () => { +test('Element should not render at all if no ID is given', () => { const { container } = render( { })} /> ); - expect(container.querySelector('.element-editor__element .test-header')).toBeNull(); - expect(container.querySelector('.element-editor__element .test-content')).toBeNull(); + expect(container.querySelectorAll('.element-editor__element')).toHaveLength(0); }); test('Element should render even if the element is broken', () => { @@ -68,8 +67,8 @@ test('Element should render even if the element is broken', () => { })} /> ); - expect(container.querySelector('.element-editor__element .test-header')).not.toBeNull(); - expect(container.querySelector('.element-editor__element .test-content')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor__element .test-header')).toHaveLength(1); + expect(container.querySelectorAll('.element-editor__element .test-content')).toHaveLength(1); }); test('Element getVersionedStateClassName() should identify draft elements', () => { @@ -82,7 +81,7 @@ test('Element getVersionedStateClassName() should identify draft elements', () = })} /> ); - expect(container.querySelector('.element-editor__element--draft')).not.toBeNull(); + expect(container.querySelector('.element-editor__element').classList.contains('element-editor__element--draft')).toBe(true); }); test('Element getVersionedStateClassName() should identify modified elements', () => { @@ -96,7 +95,7 @@ test('Element getVersionedStateClassName() should identify modified elements', ( })} /> ); - expect(container.querySelector('.element-editor__element--modified')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor__element--modified')).toHaveLength(1); }); test('Element getVersionedStateClassName() should identify published elements', () => { @@ -110,5 +109,5 @@ test('Element getVersionedStateClassName() should identify published elements', })} /> ); - expect(container.querySelector('.element-editor__element--published')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor__element--published')).toHaveLength(1); }); diff --git a/client/src/components/ElementEditor/tests/ElementActions-test.js b/client/src/components/ElementEditor/tests/ElementActions-test.js index 49ebe624..b36a1a62 100644 --- a/client/src/components/ElementEditor/tests/ElementActions-test.js +++ b/client/src/components/ElementEditor/tests/ElementActions-test.js @@ -31,7 +31,7 @@ test('ElementActions should map input tabs into an array of buttons', () => { expect(actions[1].textContent).toEqual('Settings'); expect(actions[2].textContent).toEqual('History'); // No drop down separator should exist when there are no non-CMS actions - expect(container.querySelector('.dropdown-divider')).toBeNull(); + expect(container.querySelectorAll('.dropdown-divider')).toHaveLength(0); }); @@ -46,7 +46,7 @@ test('ElementActions should render a divider when CMS tab actions and default ac expect(actions[0].textContent).toEqual('Content'); expect(actions[1].textContent).toEqual('Settings'); expect(actions[2].textContent).toEqual('History'); - expect(container.querySelector('.dropdown-divider')).not.toBeNull(); + expect(container.querySelectorAll('.dropdown-divider')).toHaveLength(1); expect(actions[3].textContent).toEqual('some button'); }); diff --git a/client/src/components/ElementEditor/tests/ElementEditor-test.js b/client/src/components/ElementEditor/tests/ElementEditor-test.js index f6d3a7fa..832fe234 100644 --- a/client/src/components/ElementEditor/tests/ElementEditor-test.js +++ b/client/src/components/ElementEditor/tests/ElementEditor-test.js @@ -55,8 +55,8 @@ function makeProps(obj = {}) { test('ElementEditor should render ElementList and Toolbar', () => { const { container } = render(); - expect(container.querySelector('.test-list')).not.toBeNull(); - expect(container.querySelector('[data-testid="test-toolbar"]')).not.toBeNull(); + expect(container.querySelectorAll('.test-list')).toHaveLength(1); + expect(container.querySelectorAll('[data-testid="test-toolbar"]')).toHaveLength(1); }); test('ElementEditor should filter all element types by those allowed for this editor', () => { diff --git a/client/src/components/ElementEditor/tests/ElementList-test.js b/client/src/components/ElementEditor/tests/ElementList-test.js index 6baf63ce..b312dc99 100644 --- a/client/src/components/ElementEditor/tests/ElementList-test.js +++ b/client/src/components/ElementEditor/tests/ElementList-test.js @@ -56,7 +56,7 @@ function makeProps(obj = {}) { test('ElementList renders elements when blocks are provided as props', () => { const { container } = render(); expect(container.querySelectorAll('.test-element')).toHaveLength(2); - expect(container.querySelector('.test-loading')).toBeNull(); + expect(container.querySelectorAll('.test-loading')).toHaveLength(0); }); test('ElementList renders a loading component', () => { @@ -68,8 +68,8 @@ test('ElementList renders a loading component', () => { })} /> ); - expect(container.querySelector('.test-element')).toBeNull(); - expect(container.querySelector('.test-loading')).not.toBeNull(); + expect(container.querySelectorAll('.test-element')).toHaveLength(0); + expect(container.querySelectorAll('.test-loading')).toHaveLength(1); }); test('ElementList renders a placeholder message when no elements are provided as props', () => { @@ -81,8 +81,8 @@ test('ElementList renders a placeholder message when no elements are provided as })} /> ); - expect(container.querySelector('.test-element')).toBeNull(); - expect(container.querySelector('.test-loading')).toBeNull(); + expect(container.querySelectorAll('.test-element')).toHaveLength(0); + expect(container.querySelectorAll('.test-loading')).toHaveLength(0); const placeholder = container.querySelector('.elemental-editor-list--empty'); expect(placeholder.textContent).toBe('Add blocks to place your content'); }); diff --git a/client/src/components/ElementEditor/tests/Header-test.js b/client/src/components/ElementEditor/tests/Header-test.js index 2764930d..6b2171b2 100644 --- a/client/src/components/ElementEditor/tests/Header-test.js +++ b/client/src/components/ElementEditor/tests/Header-test.js @@ -49,7 +49,7 @@ function makeProps(obj = {}) { test('Header should render the icon', () => { const { container } = render(
); - expect(container.querySelector('i.font-icon-block-file')).not.toBeNull(); + expect(container.querySelectorAll('i.font-icon-block-file')).toHaveLength(1); }); test('Header should render the title', () => { @@ -65,7 +65,7 @@ test('Header should render the title', () => { expect(container.querySelector('.element-editor-header__title').textContent).toBe('Sample File Block'); }); -test('Header should render the title for broken elements', () => { +test('Header should override the title for broken elements', () => { const { container } = render(
{ /> ); fireEvent.mouseOver(container.querySelector('#element-icon-13.font-icon-block-file')); - const tooltip = await screen.findByText('File'); - expect(tooltip.getAttribute('role')).toBe('tooltip'); + const tooltip = await screen.findByRole('tooltip', {}, { timeout: 500, onTimeout: () => null }); + expect(tooltip.textContent).toBe('File'); }); test('Header should not contain a Tooltip for a broken element', async () => { @@ -108,11 +108,12 @@ test('Header should not contain a Tooltip for a broken element', async () => { /> ); fireEvent.mouseOver(container.querySelector('#element-icon-13')); - // wait for 500 milliseconds for the tooltip to appear (which is should not) - // https://testing-library.com/docs/dom-testing-library/api-async/#waitfor - await waitFor(() => null, { timeout: 500 }); - // use "queryBy" because it does not throw an error on fail - https://testing-library.com/docs/queries/about - expect(screen.queryByText('File')).toBeNull(); + // Normally we would use "queryByRole" here because it does not throw an error on fail - but + // in this case that provides a false negative result (confirmed by trying a timeout + // with queryByRole in the "Header should not contain a Tooltip for a broken element" test + // which causes that test to fail) + const tooltip = await screen.findByRole('tooltip', {}, { timeout: 500, onTimeout: () => null }); + expect(tooltip).toBeNull(); }); test('Header should render a right caret button when not expandable', () => { @@ -120,7 +121,8 @@ test('Header should render a right caret button when not expandable', () => { expandable: false })} />); - expect(container.querySelector('.element-editor-header__expand.font-icon-right-open-big')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor-header__expand')).toHaveLength(1); + expect(container.querySelector('.element-editor-header__expand').classList.contains('font-icon-right-open-big')).toBe(true); }); test('Header should render a down caret button when not expanded', () => { @@ -129,7 +131,8 @@ test('Header should render a down caret button when not expanded', () => { previewExpanded: false })} />); - expect(container.querySelector('.element-editor-header__expand.font-icon-down-open-big')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor-header__expand')).toHaveLength(1); + expect(container.querySelector('.element-editor-header__expand').classList.contains('font-icon-down-open-big')).toBe(true); }); test('Header should render an up caret button when expanded', () => { @@ -138,7 +141,8 @@ test('Header should render an up caret button when expanded', () => { previewExpanded: true })} />); - expect(container.querySelector('.element-editor-header__expand.font-icon-up-open-big')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor-header__expand')).toHaveLength(1); + expect(container.querySelector('.element-editor-header__expand').classList.contains('font-icon-up-open-big')).toBe(true); }); test('Header should not render a caret button for a broken element', () => { @@ -150,7 +154,7 @@ test('Header should not render a caret button for a broken element', () => { } })} />); - expect(container.querySelector('.element-editor-header__expand')).toBeNull(); + expect(container.querySelectorAll('.element-editor-header__expand')).toHaveLength(0); }); test('Header should render an ElementActions component when the element is expandable', () => { @@ -158,7 +162,7 @@ test('Header should render an ElementActions component when the element is expan expandable: true, })} />); - expect(container.querySelector('.test-element-actions')).not.toBeNull(); + expect(container.querySelectorAll('.test-element-actions')).toHaveLength(1); }); test('Header should render an ElementActions component when the element is not expandable', () => { @@ -166,7 +170,7 @@ test('Header should render an ElementActions component when the element is not e expandable: false, })} />); - expect(container.querySelector('.test-element-actions')).not.toBeNull(); + expect(container.querySelectorAll('.test-element-actions')).toHaveLength(1); }); test('Header should render an ElementActions component even when the element is broken', () => { @@ -177,7 +181,7 @@ test('Header should render an ElementActions component even when the element is } })} />); - expect(container.querySelector('.test-element-actions')).not.toBeNull(); + expect(container.querySelectorAll('.test-element-actions')).toHaveLength(1); }); test('Header should render a versioned state message when the element is not published', () => { @@ -189,7 +193,7 @@ test('Header should render a versioned state message when the element is not pub } })} />); - expect(container.querySelector('.element-editor-header__version-state--draft').getAttribute('title')).toContain('not been published'); + expect(container.querySelector('.element-editor-header__version-state.element-editor-header__version-state--draft').getAttribute('title')).toContain('not been published'); }); test('Header should render a versioned state message when the element is modified', () => { @@ -201,10 +205,10 @@ test('Header should render a versioned state message when the element is modifie } })} />); - expect(container.querySelector('.element-editor-header__version-state--modified').getAttribute('title')).toContain('has unpublished changes'); + expect(container.querySelector('.element-editor-header__version-state.element-editor-header__version-state--modified').getAttribute('title')).toContain('has unpublished changes'); }); -test('Header should render a versioned state message when the element is published', () => { +test('Header should not render a versioned state message when the element is published', () => { const { container } = render(
); - expect(container.querySelector('.element-editor-header__version-state')).toBeNull(); + expect(container.querySelectorAll('.element-editor-header__version-state')).toHaveLength(0); }); diff --git a/client/src/components/ElementEditor/tests/Summary-test.js b/client/src/components/ElementEditor/tests/Summary-test.js index 6135245f..8b1659d3 100644 --- a/client/src/components/ElementEditor/tests/Summary-test.js +++ b/client/src/components/ElementEditor/tests/Summary-test.js @@ -16,7 +16,8 @@ function makeProps(obj = {}) { test('Summary should render an image if the fileUrl prop is provided', () => { const { container } = render(); - expect(container.querySelector('.element-editor-summary__thumbnail-image')).not.toBeNull(); + expect(container.querySelectorAll('img.element-editor-summary__thumbnail-image')).toHaveLength(1); + expect(container.querySelector('img.element-editor-summary__thumbnail-image').getAttribute('src')).toBe('/ss4/assets/Uploads/c70617f2e4/sample__FillWzEwMCwxMDBd.jpeg'); }); test('Summary should not render an image if the fileUrl prop is not provided', () => { @@ -26,12 +27,12 @@ test('Summary should not render an image if the fileUrl prop is not provided', ( })} /> ); - expect(container.querySelector('.element-editor-summary__thumbnail-image')).toBeNull(); + expect(container.querySelectorAll('img.element-editor-summary__thumbnail-image')).toHaveLength(0); }); test('Summary should render a content summary if the content is provided', () => { const { container } = render(); - expect(container.querySelector('.element-editor-summary__content')).not.toBeNull(); + expect(container.querySelectorAll('.element-editor-summary__content')).toHaveLength(1); }); test('Summary should not render a content summary if the content prop is not provided', () => { @@ -41,5 +42,5 @@ test('Summary should not render a content summary if the content prop is not pro })} /> ); - expect(container.querySelector('.element-editor-summary__content')).toBeNull(); + expect(container.querySelectorAll('.element-editor-summary__content')).toHaveLength(0); }); diff --git a/client/src/components/HistoricElementView/tests/HistoricElementView-test.js b/client/src/components/HistoricElementView/tests/HistoricElementView-test.js index f82fc006..3de62c4b 100644 --- a/client/src/components/HistoricElementView/tests/HistoricElementView-test.js +++ b/client/src/components/HistoricElementView/tests/HistoricElementView-test.js @@ -31,9 +31,9 @@ class FieldGroupStub extends React.Component { const HistoricElementView = ElementalAreaHistoryFactory(FieldGroupStub); -test('HistoricElementView render ', () => { +test('HistoricElementView renders', () => { const { container } = render(); - expect(container.querySelector('.elemental-area__element--historic-inner.ok')).not.toBeNull(); + expect(container.querySelectorAll('.elemental-area__element--historic-inner.ok')).toHaveLength(1); expect(container.querySelector('.elemental-preview__detail h3').textContent).toContain('Pretend Element'); expect(container.querySelector('.elemental-preview__detail small').textContent).toBe('Stub'); expect(container.querySelector('.elemental-preview__link').href).toBe('http://localhost:8080/');