Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CommandBar (v8): convert tests to use testing-library #22247

Merged
merged 2 commits into from
Mar 31, 2022
Merged

CommandBar (v8): convert tests to use testing-library #22247

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d0053b
convert to testing library
TristanWatanabe Mar 30, 2022
dcd935f
use jest timers
TristanWatanabe Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 71 additions & 105 deletions packages/react/src/components/CommandBar/CommandBar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import * as React from 'react';
import * as renderer from 'react-test-renderer';

import { render, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { CommandBar } from './CommandBar';
import { mount } from 'enzyme';
import { isConformant } from '../../common/isConformant';
import { resetIds } from '../../Utilities';
import type { IContextualMenuItem } from '../../ContextualMenu';
Expand All @@ -12,45 +11,30 @@ describe('CommandBar', () => {
resetIds();
});

afterEach(() => {
for (let i = 0; i < document.body.children.length; i++) {
if (document.body.children[i].tagName === 'DIV') {
document.body.removeChild(document.body.children[i]);
i--;
}
}
});

it('renders commands correctly', () => {
expect(
renderer
.create(
<CommandBar
items={[
{ key: '1', text: 'asdf' },
{ key: '2', text: 'asdf' },
]}
className={'TestClassName'}
/>,
)
.toJSON(),
).toMatchSnapshot();
const { container } = render(
<CommandBar
items={[
{ key: '1', text: 'asdf' },
{ key: '2', text: 'asdf' },
]}
className={'TestClassName'}
/>,
);
expect(container).toMatchSnapshot();
});

it('renders empty farItems correctly', () => {
expect(
renderer
.create(
<CommandBar
items={[
{ key: '1', text: 'asdf' },
{ key: '2', text: 'asdf' },
]}
farItems={[]}
/>,
)
.toJSON(),
).toMatchSnapshot();
const { container } = render(
<CommandBar
items={[
{ key: '1', text: 'asdf' },
{ key: '2', text: 'asdf' },
]}
farItems={[]}
/>,
);
expect(container).toMatchSnapshot();
});

isConformant({
Expand All @@ -62,7 +46,7 @@ describe('CommandBar', () => {
});

it('opens a menu with IContextualMenuItem.subMenuProps.items property', () => {
const commandBar = mount(
const { getByRole } = render(
<CommandBar
items={[
{
Expand All @@ -83,13 +67,9 @@ describe('CommandBar', () => {
/>,
);

const menuItem = commandBar.find('.MenuItem button');

expect(menuItem.length).toEqual(1);

menuItem.simulate('click');

expect(document.querySelector('.SubMenuClass')).toBeTruthy();
const menuItem = getByRole('menuitem', { hidden: true });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another case that testing-library deems as "inaccessible" for seemingly no reason 😢

Copy link
Member

@ecraig12345 ecraig12345 Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was bothering me enough that I started looking at the testing-library code, and I figured it out...it's the measurement/overflow logic in some of our components, including CommandBar and Breadcrumb. I don't know the details of how measurement works, but the general idea is that it initially renders stuff hidden and/or off-screen to get the size, then re-renders with appropriate size and overflow items. getByRole is running against that initial measurement render, where the elements are in fact hidden.

How I figured this out was a little over-complicated, and in retrospect I probably should have tried running the test and stepping through the code first. 😆

What I actually did: I started by looking in @testing-library/dom's code for the implementation of the role query and searching for hidden. That led to the isInaccessible implementation. I wasn't sure exactly what would cause issues there, so I made a codepen that renders a basic Breadcrumb (since it has the same problem and is a little simpler) and runs isInaccessible against the list items after render.

Turns out getComputedStyle(element).visibility was returning 'hidden'. This seemed weird (since I could see the breadcrumb on the page) but then I remembered that Breadcrumb and CommandBar have measurement with initial hidden rendering. So I tried running isInaccessible on the items again after a 500ms timeout and it returns false (is accessible) as expected.
https://codepen.io/ecraig12345/pen/RwxZwdN?editors=0011

Anyway...the possible solutions for the tests are:

  • keep { hidden: true } and add comments that it's due to the measurement logic
  • use fake timers and run timers before doing getByRole on items

Definitely any tests that involve overflow should use fake timers, but in other cases it's up to you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow - the initial hidden rendering of some of our components is quite the gotcha in this case😆. Thanks a lot for looking into this!

userEvent.click(menuItem);
expect(getByRole('menu')).toBeTruthy();
});

it('passes event and item to button onClick callbacks', () => {
Expand All @@ -107,59 +87,51 @@ describe('CommandBar', () => {
},
};

const commandBar = mount(<CommandBar items={[itemData]} />);

const menuItem = commandBar.find('.MenuItem button');

menuItem.simulate('click');
const { getByRole } = render(<CommandBar items={[itemData]} />);

const menuItem = getByRole('menuitem', { hidden: true });
userEvent.click(menuItem);
expect(testValue).toEqual(itemData);
});

it('keeps menu open after update if item is still present', () => {
const commandBar = mount(
<CommandBar
items={[
{
text: 'TestText 1',
key: 'TestKey1',
subMenuProps: {
items: [
{
text: 'SubmenuText 1',
key: 'SubmenuKey1',
className: 'SubMenuClass',
},
],
const items = [
{
text: 'TestText 1',
key: 'TestKey1',
subMenuProps: {
items: [
{
text: 'SubmenuText 1',
key: 'SubmenuKey1',
className: 'SubMenuClass',
},
},
]}
/>,
);

const menuItem = commandBar.find('button');
],
},
},
];
const { getByRole, rerender } = render(<CommandBar items={items} />);

menuItem.simulate('click');
const menuItem = getByRole('menuitem', { hidden: true });
userEvent.click(menuItem);

// Make sure the menu is open before the re-render
expect(document.querySelector('.SubMenuClass')).toBeTruthy();
expect(getByRole('menu')).toBeTruthy();

// Update the props, and re-render
commandBar.setProps({
items: commandBar.props().items.concat([
{
name: 'Test Key 2',
key: 'TestKey2',
},
]),
items.push({
text: 'Test Key 2',
key: 'TestKey2',
subMenuProps: { items: [{ text: 'SubmenuText 2', key: 'SubmenuKey2', className: 'SubmenuClass' }] },
});
rerender(<CommandBar items={items} />);

// Make sure the menu is still open after the re-render
expect(document.querySelector('.SubMenuClass')).toBeTruthy();
expect(getByRole('menu')).toBeTruthy();
});

it('closes menu after update if item is not longer present', () => {
const commandBar = mount(
const { getByRole, queryByRole, rerender } = render(
<CommandBar
items={[
{
Expand All @@ -179,20 +151,17 @@ describe('CommandBar', () => {
/>,
);

const menuItem = commandBar.find('button');

menuItem.simulate('click');
const menuItem = getByRole('menuitem', { hidden: true });
userEvent.click(menuItem);

// Make sure the menu is open before the re-render
expect(document.querySelector('.SubMenuClass')).toBeTruthy();
expect(getByRole('menu')).toBeTruthy();

// Update the props, and re-render
commandBar.setProps({
items: [],
});
rerender(<CommandBar items={[]} />);

// Make sure the menu is still open after the re-render
expect(document.querySelector('.SubMenuClass')).toBeFalsy();
expect(queryByRole('menu')).toBeNull();
});

it('passes overflowButton menuProps to the menu, and prepend menuProps.items to top of overflow', () => {
Expand All @@ -210,7 +179,7 @@ describe('CommandBar', () => {
},
];

const commandBar = mount(
const { getByRole, getAllByRole } = render(
<CommandBar
overflowButtonProps={{
menuProps: {
Expand All @@ -223,16 +192,15 @@ describe('CommandBar', () => {
/>,
);

const overflowMenuButton = commandBar.find('.ms-CommandBar-overflowButton');
const overflowMenuButton = getAllByRole('menuitem', { hidden: true })[1];
userEvent.click(overflowMenuButton);

overflowMenuButton.hostNodes().simulate('click');

const overfowItems = document.querySelectorAll('.ms-ContextualMenu-item');
const overfowItems = within(getByRole('menu')).getAllByRole('menuitem');

expect(overfowItems).toHaveLength(2);
expect(overfowItems[0].textContent).toEqual('Text3');
expect(overfowItems[1].textContent).toEqual('Text2');
expect(document.querySelectorAll('.customMenuClass')).toHaveLength(1);
expect(getByRole('menu').querySelectorAll('.customMenuClass')!.length).toEqual(1);
});

it('updates menu after update if item is still present', () => {
Expand All @@ -252,21 +220,19 @@ describe('CommandBar', () => {
},
];

const commandBar = mount(<CommandBar items={items('SubMenuClass')} />);
const { getByRole, rerender } = render(<CommandBar items={items('SubMenuClass')} />);

const menuItem = commandBar.find('button');
const menuItem = getByRole('menuitem', { hidden: true });

menuItem.simulate('click');
userEvent.click(menuItem);

// Make sure the menu is open before the re-render
expect(document.querySelector('.SubMenuClass')).toBeTruthy();
expect(getByRole('menu')).toBeTruthy();

// Re-render
commandBar.setProps({
items: items('SubMenuClassUpdate'),
});
// Update the props, and re-render
rerender(<CommandBar items={items('SubMenuClassUpdate')} />);

// Make sure the menu is still open after the re-render
expect(document.querySelector('.SubMenuClassUpdate')).toBeTruthy();
expect(getByRole('menu').querySelector('.SubMenuClassUpdate')).toBeTruthy();
});
});
Loading