mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[data grid] Testing with react testing library

Open benjamin-schneider opened this issue 3 years ago • 46 comments

With the component @material-ui/data-grid I am unable to get the rows rendered in a jest / react-testing-library environment. I use jest and material-ui since years, and this is the first time I do not manage to find any solution.

  • [x] The issue is present in the latest release.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

I want to test a fully renderer datagrid. To test if a cell in a row il well formatted, to test row clicks, etc. Unfortunately, I don't manage to render the rows. In debug mode, I get :

<div
  class="rendering-zone"
  style="max-height: 36px; width: 900px; pointer-events: unset; transform: translate3d(-0px, -NaNpx, 0);"
/>

I saw this line in the internal tests : https://github.com/mui-org/material-ui-x/blob/master/packages/grid/x-grid/src/tests/rows.XGrid.test.tsx#L43 Maybe it it actually not possible to achieve testing rows :(

Expected Behavior 🤔

I would like datagrid to fully render in a jest/jestdom env.

Steps to Reproduce 🕹

Here is a very minimal repro to isolate :

import { DataGrid } from '@material-ui/data-grid';
import { render } from '@testing-library/react';
const Repro = () => {
  return (
    <DataGrid
      columns={
        [
          {
            field: 'field',
            headerName: 'col',
            flex: 1,
          },
        ]
      }
      rows={[
        {
          id: 'row1',
          field: 'row',
        },
      ]}
    />
  );
};

const createRender = () => {
  return render(<Repro />);
};

describe('Datagrid problem repro', () => {
  it('should render col', () => {
    const { getByText } = createRender();
    expect(getByText('col')).toBeInTheDocument();
  });
  it('should render row', () => {
    const { getByText } = createRender();
    expect(getByText('row')).toBeInTheDocument();
  });
});

First test (cols or headings) passes, testing rows fails

Your Environment 🌎

Ubuntu 20.04 / jest

Progress on the solution

https://github.com/mui-org/material-ui-x/issues/1151#issuecomment-817209168

benjamin-schneider avatar Mar 03 '21 10:03 benjamin-schneider

@benjamin-schneider Did you find a solution?

I have looked at this problem a few months ago when I started to share the testing stack of the main repository with this one. I have stopped looking after failing to make the AutoSizer return my fake data in jsdom. So I ended up adding:

https://github.com/mui-org/material-ui-x/blob/59ef9dc874c49ae9be1f25b03339aef0026db884/packages/grid/data-grid/src/tests/components.DataGrid.test.tsx#L30-L33

I'm reopening because it's important. Developers should be able to tests their grid without having to rely on an actual browser as we do (it's not very common for UI components tests inside web applications). As far as I have pushed the exploration, I can think of two paths:

  1. We keep looking into what needs to be mocked in the tests in order for them to render without an empty cell. This might requires some not really pretty code (verbose and brittle).
  2. We add special logic in the source to behave nicely in the test environment. It's something we have already done in the past for the core components.

If you are using jest, you could probably mock the component as it's done in https://github.com/bvaughn/react-window/issues/454#issuecomment-646031139.

A bit of caution. I'm not completely sure that it's something that possible. jsdom might be missing some APIs in order for the grid to work well. This is not a simple button component.

oliviertassinari avatar Mar 08 '21 14:03 oliviertassinari

try to wrap your <DataGrid /> around a div that has a prefixed height and width as the grid calculates what to render according to the size of its parent container

<div style={{ width: 300, height: 300 }}>
  <DataGrid .../>
</div>,

dtassone avatar Mar 08 '21 16:03 dtassone

@dtassone jsdom has no layout capabilities, the width/height would have no effects.

oliviertassinari avatar Mar 08 '21 16:03 oliviertassinari

@oliviertassinari Hi, thanks for your anwser. I did not find a solution, I just closed to avoid polluting your issues as I was thinking I was maybe wrong. I did not manage to mock the AutoSize component (with jest) correctly, it never enters in the mock function :

jest.mock('@material-ui/data-grid', () => {
  const original = jest.requireActual('@material-ui/data-grid');
  return {
    ...original,
    AutoSizer: ({ children }: AutoSizerProps) => {
      console.log('mocked AutoSizer');
      return children({
        height: 500,
        width: 500,
      });
    },
  };
});

This was just a five minutes try... I will try to investigate further as soon as I will find some time to understand the internals. Thanks a lot for all your work on the best React UI library.

benjamin-schneider avatar Mar 09 '21 14:03 benjamin-schneider

@benjamin-schneider I have also spent 5 minutes looking at this yesterday. After solving the autosize issue, I was facing a .scrollTo missing API in jsdom 🙈. I have stopped there because I had to look at more pressing matters.

I have added the waiting for users upvotes tag. It will help evaluate if this is a frequent requirement. Fixing jsdom might be challenging and ever more as we add complexity in the internals of the component.

oliviertassinari avatar Mar 09 '21 14:03 oliviertassinari

@oliviertassinari How about we add 2 props to force the height and width of AutoSizer https://github.com/bvaughn/react-virtualized/issues/493#issuecomment-361029657

dtassone avatar Mar 09 '21 17:03 dtassone

@dtassone Yeah, maybe we could do this for test only purposes. It could solve one part of the issue.

I think that we need to understand all the implications of supporting the rendering inside jsdom before making changes. There might be even bigger issues.

I also think that we can wait for more upvotes before dedicating time to it. It's unclear if developers want this. Intuitively, I would assume that many want it, but it seems costly, so if we can live without it, that's even better 🙃.

oliviertassinari avatar Mar 09 '21 17:03 oliviertassinari

just chiming in to say I'm having a bear of a time trying to test for row content in @material-ui/core/DataGrid. Tried everything in the past 48 hours & can't get the rows to even render within ReactTestingLibrary

UPDATE: finally got stuff to render. needed the "autoheight" param. for some reason it renders in the browser but not in the tests. needs autoheight. go figure

DavidHanover avatar Apr 01 '21 17:04 DavidHanover

@DavidHanover The autoHeight approach sounds interesting. It might actually be good enough in many of the cases. It basically disables the row virtualization.

oliviertassinari avatar Apr 01 '21 18:04 oliviertassinari

Since you're wondering, we're also feeling this pain. We've been using Xgrid for about 4 mos, and felt almost immediate pain since we're a TDD shop. Found a workaround with alpha18 by setting a fixed width on the Grid's parent tied to a theme variable being set in test env only, but that stopped working in alpha24. BTW, we're testing with jest + RTL.

gap777 avatar Apr 09 '21 00:04 gap777

@gap777 What aspects are you testing on the data grid? Are you testing simple interactions, like what rows are displayed or more advanced things?

#1361 is one step toward the resolution of this issue. With it, developers can test that the rows prop renders the correct value.

oliviertassinari avatar Apr 10 '21 21:04 oliviertassinari

Making sure the right columns are displayed was our first round of tests where we ran into issues. Making sure the data is run through the correct formatting rules (colDef valueFormatter, etc) Making sure the react component using the table updated it with the correct data Making sure the click actions do the right thing

On Apr 10, 2021, at 5:34 PM, Olivier Tassinari @.***> wrote:

@gap777 https://github.com/gap777 What aspects are you testing on the data grid? Are you testing simple interactions, like what rows are displayed or more?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mui-org/material-ui-x/issues/1151#issuecomment-817205241, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRR26RNT5PXSFZKQPGBQEDTIC76LANCNFSM4YQ7LFFQ.

gap777 avatar Apr 10 '21 21:04 gap777

@gap777 Ok great. This is covered by #1361.

For closing the issue. I would propose the following requirements:

  • [x] #1361 is merged. We go from 2 tests out of our test suite of 132 to 47 running in jsdom.
  • [x] A follow-up was done to migrate as many tests as possible in jsdom. We won't be able to get 100% but the more the merrier.
  • [ ] We document how to test the data grid, maybe in https://material-ui.com/components/data-grid/virtualization/

On the approach, I could find this interesting comment. https://github.com/ag-grid/ag-grid/issues/4165#issuecomment-786916614. They have a prop to disable the virtualization. I guess we will know more about the challenges with our grid as me make progress on 2.

oliviertassinari avatar Apr 10 '21 22:04 oliviertassinari

Since #1361, we now run 69% of our tests inside jsdom (and 100% inside real browsers).

oliviertassinari avatar Apr 26 '21 18:04 oliviertassinari

Very good job with the coverage improvements, thanks ! Finally, with forced autoHeight like suggested, we can render every Datagrids in our codebase without having to touching the source component, which was mandatory for us. If it can be helpful, here is small snippet :

import type { DataGridProps } from '@material-ui/data-grid';
jest.mock('@material-ui/data-grid', () => {
  const { DataGrid } = jest.requireActual('@material-ui/data-grid');
  return {
    ...jest.requireActual('@material-ui/data-grid'),
    DataGrid: (props: DataGridProps) => {
      return (
        <DataGrid
          {...props}
          autoHeight
        />
      );
    },
  };
});

benjamin-schneider avatar Apr 27 '21 07:04 benjamin-schneider

This is still causing me major issues :( Even if I look for row in my test It only slightly increase test coverage as opposed to the large increase when using a failing test that looks for the specific text I expect to be found in a cell after an API call is placed. Hopping to see a further resolution of this issue come down the pipe.

Eyegee-Eleven avatar May 26 '21 02:05 Eyegee-Eleven

@Eyegee-Eleven do you have an example of a case you can't test? (please provide a codesandbox).

oliviertassinari avatar May 26 '21 08:05 oliviertassinari

Hi, just curious if any progress has been made on this issue? We currently have to disable about a third of our test suite, which is very far from ideal.

squidsoup avatar Jul 15 '21 00:07 squidsoup

To summarize where we are today for clarity:

  1. Internally, we have 248 tests running with jsdom, 286 tests running in real browsers.
  2. We need to allow developers to disable virtualization completely. The trick with autoHeight is limited as it only covers the rows virtualization, not the columns virtualization. #1781.
  3. Once we have added such an API, we need to document it.

I have added the "important" label to signal that it's one of the most upvoted issues. I'm not saying that we should work on it now as our priority should likely be to add the missing features, and maybe only after to polish, starting from this one. Or actually, to take this into account in #1933. Developers can always run their tests into a real browser, not jsdom.

@squidsoup Could you provide an example of tests that you can't write? Is it about using the columns or something else?

oliviertassinari avatar Jul 15 '21 11:07 oliviertassinari

Is this issue the reason why this test fails?

    it('Renders all columns', async () => {
        render(
            <DataGridPro
                columns={[
                    {
                        field: 'field1',
                    },
                    {
                        field: 'field2',
                    },
                    {
                        field: 'field3',
                    },
                    {
                        field: 'field4',
                    },
                    {
                        field: 'field5',
                    },
                    {
                        field: 'field6',
                    },
                ]}
                rows={[]}
            />,
        );

       // Fails, only three columns render
        expect(within(screen.getAllByRole('row')[0]).getAllByRole('columnheader')).toHaveLength(6);
    });

In the debugger the window has a width of 1024, so idk why this would be such a problem.

JPTeasdale avatar Sep 15 '21 18:09 JPTeasdale

Is this issue the reason why this test fails?

    it('Renders all columns', async () => {
        render(
            <DataGridPro
                columns={[
                    {
                        field: 'field1',
                    },
                    {
                        field: 'field2',
                    },
                    {
                        field: 'field3',
                    },
                    {
                        field: 'field4',
                    },
                    {
                        field: 'field5',
                    },
                    {
                        field: 'field6',
                    },
                ]}
                rows={[]}
            />,
        );

       // Fails, only three columns render
        expect(within(screen.getAllByRole('row')[0]).getAllByRole('columnheader')).toHaveLength(6);
    });

In the debugger the window has a width of 1024, so idk why this would be such a problem.

It seems like this. Try to add 'autoHeight' props to your DataGridPro component. If problem will gone, then we can confidently say that this is the same problem

Kosurij avatar Sep 20 '21 11:09 Kosurij

@JPTeasdale We now have a disableVirtualization prop which you can use to allow tests to see all columns. It's available in the last beta release. There's also the columnBuffer prop which might also be usable if virtualization cannot be disabled.

m4theushw avatar Sep 20 '21 16:09 m4theushw

I assume the only thing that we miss now on this issue is documentation, and a good chunk of it was done in https://mui.com/components/data-grid/virtualization/#disable-virtualization. Is there something else we could do to close the issue?

oliviertassinari avatar Sep 20 '21 17:09 oliviertassinari

disableVirtualization

Is there any tips on how to transform the tested DataGrid into one with disableVirtualization prop only for testing?

bsides avatar Dec 15 '21 13:12 bsides

:+1: I am running into the same issue. TestingLibrary+JSDom. Experimenting around, I found that columnBuffer does the trick.

jakub-bao avatar Jan 20 '22 21:01 jakub-bao

For CRA based src/setupTests.tsx:

import { DataGridProps } from '@mui/x-data-grid';

jest.mock('@mui/x-data-grid', () => {
  const { DataGrid } = jest.requireActual('@mui/x-data-grid');
  return {
    ...jest.requireActual('@mui/x-data-grid'),
    DataGrid: (props: DataGridProps) => {
      return <DataGrid {...props} disableVirtualization />;
    },
  };
});

ModPhoenix avatar Apr 25 '22 09:04 ModPhoenix

I ran into the same problem today in a project. @ModPhoenix 's solution works perfectly!

caedes avatar May 17 '22 13:05 caedes

I'm using the following:

render(<DataGrid rows={items} columns={columns} columnBuffer={columns.length} disableVirtualization />);

but my test is still not showing all of the columns.

On the grid it says it has a aria-colcount="5"

Rafael805 avatar Jul 11 '22 15:07 Rafael805

@oliviertassinari @m4theushw @dtassone @samuelsycamore

I'm encountering a similar issue but in my case using the Data Grid Pro, I need to change the aria-label of the checkbox, so that it is unique, we need to include the name in the accessible name if the checkbox.

The issue we're having is passing the name as a prop to baseCheckbox how do we get the name of each row and pass it as a prop?

Or is there another recommended or better approach to this, we want to find the input by the role checkbox and then its name, we're using Cypress and Testing library.

Thanks!

              componentsProps={{
                baseCheckbox: { name: /* I want to pass the name here, how can I do it? */ }
              }}
              ```

tigerabrodi avatar Aug 09 '22 09:08 tigerabrodi

I need to change the aria-label of the checkbox, so that it is unique

@narutosstudent Why?

oliviertassinari avatar Aug 09 '22 12:08 oliviertassinari