microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.39k stars 2.72k forks source link

[Feature]: Enable testing of fluentui components #25180

Closed morsh closed 9 months ago

morsh commented 1 year ago

Library

React Components / v9 (@fluentui/react-components)

Describe the feature that you would like added

I want to be able to easily test fluetui component, like so:

i.e.:

Component:

const SomeComponent = ({ isUp }: { isUp: boolean }) => (
  <IconButton iconProps={{ iconName: isUp ? 'Up' : 'Down' }} />
);

Test:

const { container } = render(<SomeComponent isUp={true} />);
expect(iconDriver(container).getIconName()).toEqual('Up');

Without that, I need to either check the value of data-icon-name (which might change in the future), match a snapshot (which would check a lot more then just this attribute + wouldn't be different from checking the attribute directly) or"

import * as fluentui from '@fluentui/react';
// ...
const iconButtonSpy = jest.spyOn(fluentui, 'IconButton');
const { container } = render(<SomeComponent isUp={true} />);
expect(iconButtonSpy).toHaveBeenCalledWith(expect.objectContaining({ iconName: 'Up' }));

This approach is less scaleable then the one with the driver, more complex, depends on implementation and would change all the dependent tests when the API changes (in big projects with multiple types of components, this could result in hundreds of changes when updating the version of fluentui).

Have you discussed this feature with our team

No response

Additional context

We are currently using @fluentui/react in Viva Sales | Conversation Intelligence and thinking about a big scale approach to testing usages of fluentui components in our projects. We would also like to decouple the test logic from the implementation, and we're seeing that we'll eventually need to provide a driver for testing any fluentui component we'll use.

Validations

tudorpopams commented 1 year ago

Hello @morsh , thank you for the feedback! @Hotell do we have any recommendations on this scenario?

morsh commented 1 year ago

FYI @Hotell, I have a lot of experience in building testing capabilities for UI libraries, I'd really be happy to talk

msft-fluent-ui-bot commented 1 year ago

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fluent UI!

morsh commented 1 year ago

@Hotell, @tudorpopams not sure what's the required feedback

morsh commented 1 year ago

@khmakoto @tudorpopams @Hotell pinging since I'm not sure this is monitored 😅

msft-fluent-ui-bot commented 1 year ago

Gentle ping that this issue needs attention.

Hotell commented 1 year ago

Hey! I'm not really sure I understand what you're asking for, but in general it looks like you wanna test implementation details which by definition creates brittle tests and should be avoided ( especially in application environment ).

could you provide more context/background on this ? ty

msft-fluent-ui-bot commented 1 year ago

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fluent UI!

morsh commented 1 year ago

@Hotell - on the contrary, I want to test functionality, not implementation, this is exactly the point - currently I'm required to know the implementation for testing.

If I have a button, and I want to test it like so:

const AppButton = ({ onClick }) => <Button data-testid="app-button" onClick={onClick}>Some Text</Button>;

// I can only write the following test knowing that the top most element is `button`

it('should click the button', () => {
  const onClickSpy = jest.fn();
  render(<AppButton onClick={onClickSpy} />
  fireEvent.click(screen.getByTestId("app-button"));
  expect(onClickSpy).toHaveBeenCalled();
});

If you changed the implementation to have button wrapped by a div, or button implemented as span with a link, my tests would break.

This becomes a lot more complex with components like Dropdown (if I want to have a onChange listener) or with DataGrid if I wanted to have a test where I select items and want a test to test the received items.

The latter would look like this:

it('should select items', () => {
  const selectedItems = null;
  render(<SomeComponentWithDataGrid someSelectableData={items} onSelectItems{selected => (selectedItems = selected)} />
  fireEvent.click(container.baseElement.querySelectorAll('input[type="checkbox"]')[0]);
  fireEvent.click(container.baseElement.querySelectorAll('input[type="checkbox"]')[2]);
  expect(selectedItems).toBe([items[0], items[2]]);
});

If we had a test driver exported with each component, we wouldn't need to know implementation details:

it('should click the button', () => {
  const onClickSpy = jest.fn();
  render(<AppButton onClick={onClickSpy} />
  buttonDriver.click();
  expect(onClickSpy).toHaveBeenCalled();
});
it('should select items', () => {
  const selectedItems = null;
  render(<SomeComponentWithDataGrid someSelectableData={items} onSelectItems{selected => (selectedItems = selected)} />
  dataGridDriver.getRow(0).clickOnSelectCheckbox();
  dataGridDriver.getRow(1).clickOnSelectCheckbox();
  expect(selectedItems).toBe([items[0], items[2]]);
});
morsh commented 1 year ago

I also think this would be clearer on a f2f call

microsoft-github-policy-service[bot] commented 9 months ago

Because this issue has not had activity for over 180 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

microsoft-github-policy-service[bot] commented 9 months ago

Because this issue has not had activity for over 180 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.