Test failures in v5.0.1

[ddolcimascolo]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Current behavior 😯

We have 70 tests failing when upgrading from v5.0.0-beta.4 to v5.0.0-beta.5.

This is a loooooot higher than the usual failures that we expect (because you're still in beta). Most (probably all, but I need to dig deeper) failures are related to columns being invisible while they were in beta4, columns being absent from the DOM while there were present in beta4, or other similar assertions....

Can you advise on what might have changed in beta5 causing so many failures? I'm OK to fix these on my side, but I'd apreciate guidance on where to start :)

Thx a lot !

David

Expected behavior 🤔

No failures?

Steps to reproduce 🕹

I've no reproduction, but I can try to setup one if that's required. Basically we've lots of tests doing stuff like expect(column).toBeVisible() or expect(column).toBeInTheDocument() that are now failing.

Context 🔦

No response

Your environment 🌎

`npx @mui/envinfo`

System:
    OS: Linux 5.11 Linux Mint 20.2 (Uma)
  Binaries:
    Node: 14.15.1 - ~/.nvm/versions/node/v14.15.1/bin/node
    Yarn: Not Found
    npm: 6.14.8 - ~/.nvm/versions/node/v14.15.1/bin/npm
  Browsers:
    Chrome: 95.0.4638.54
    Firefox: 93.0

Order ID 💳 (optional)

No response

[m4theushw]

The virtualization engine was completely rewritten, I suppose that is main reason for your tests to fail. Without knowing how is your test environment we can't provide clear guidance to make them to pass. Are you using jsdom or running in a real browser (e.g. Chrome)? Do they pass using disableVirtualization?

[ddolcimascolo]

Hi, thx for the quick feedback.

Tests are run in jsdom, and using disableVirtualization helps, only 24 failures left. So yeah, that seems to be it.

You mean rewritten as in "you'd have to throw in a few waitFor() to find the columns"?

[ddolcimascolo]

About our test setup, a typical test case using a DataGrid does

• Mock stuff for jsdom, like getBoundingClientRect to let the grid believe it has space to draw the columns
• Render our component
• Assert the contents of column headers, rows and cells using various selectors, but mostly getByRole(grid | columnheader | row | cell)

[m4theushw]

You mean rewritten as in "you'd have to throw in a few waitFor() to find the columns"?

The waitFor won't work, because the column is never rendered. Waiting a few ms should not fix the tests.

Mock stuff for jsdom, like getBoundingClientRect to let the grid believe it has space to draw the columns

Probably there's the problem. The virtualization doesn't relay on getBoundingClientRect anymore, but on clientWidth / clientHeight instead. Could you try mocking these other properties too?

Also, if you managed to share a test case (e.g. in a GitHub Gist or repo) I could take a better look to see what's going on.

[ddolcimascolo]

Thx, I'll try that first and if it doesn't fix it I'll setup a real case in a repo

[ddolcimascolo]

@m4theushw Thx for your help, that was it for 90% of the failures. The remaining failures were due to the columns being recycled in the DOM, and not removed. I suppose these are side-effects of the changes you mention above.

For the record, in case someone else get similar failures in the future, in jsdom:

Object.defineProperties(Element.prototype, {
  clientWidth: { get: () => /* your width here */ },
  clientHeight: { get: () => /* your height here */ }
});

[flaviendelangle]

If we have other feedbacks of users having a hard time testing their Grid with the virtualization, it could be interesting to add a paragraph in the doc with those kind of tips.

[ddolcimascolo]

Hi @flaviendelangle @m4theushw

v5.0.1 brings back unexpected failures, though this time you released v5 so breaking things is clearly not expected in minor releases, let alone patch ones.

As usual, I'm in to refactor my tests but I'd like some guidance on what might be causing the regressions, as @m4theushw did last time. The failures apparently are related to some rows not being rendered, while they were in 5.0.0. Anything related to size or whatever again?

Thx

David

[m4theushw]

It could be related to #3007.

Which tests are you doing that are failing with v5.0.1?

When testing with jsdom, these kind of errors are expected since it doesn't have the capability to infer the size of an element. We only test layout stuff with a real browser to avoid having to mock the dimensions.

[ddolcimascolo]

Yeah, and I know I should change the failing tests. I'll dig more tomorrow morning and get back to you. I'll also check #3007, thanks for the hint

[flaviendelangle]

Yeah much likely #3007
I would be very interested to see actual failing tests to have more context about which types of tests users create and have struggle maintaining when we rework our internals.

[ddolcimascolo]

Hi @flaviendelangle @m4theushw

Actually this time tests are right, we do have one grid that does not render anymore. It's a simple one with static rows and columns. We do have other grids that render fine.

One thing I noticed is that now we have this warning in the console

react_devtools_backend.js:2540 MUI: useResizeContainer - The parent of the grid has an empty width.
You need to make sure the container has an intrinsic width.
The grid displays with a width of 0px.

Side note: we do have a license for your product, we're a french company

Thx,
David

[flaviendelangle]

This warning is fired when to grid does not have a height.
As explained on this doc page, by default the grid takes the size of its parent
But in JSDOM this does not work. So we advise you to use the autoHeight for testing.

I see you found a workaround #2945 (comment) and I suppose that this workaround does not work anymore since you have the error again.
The reason may be because I don't call clientHeight / clientWidth anymore but rely purely on our GridAutoSizer.

If you are passing props to your Grid directly in your test, you could bypass the issue by publishing a resize event with the dimensions.
Something like:

let apiRef: GridApiRef

describe('', () => {
  it('', () => {
    const Wrapper = (props) => {
      apiRef = useGridApiRef();
      return <DataGridPro {...props} apiRef={apiRef} />
    }

    render(<Wrapper rows={rows} />)
    apiRef.current.publishEvents('resize', { height: 300, width: 300 });
  });
});

But depending on how your tests are structured, it can be tricky to access the apiRef
There is probably also ways to trick our GridAutoSizer to fire the onResize callback but I did not try it.

If you can give me a repo with a small reproduction case I can spend some time finding a solution (and documenting it properly for everyone).

[ddolcimascolo]

Thx for your reply, but I was unclear sorry. It's the actual production app that suffers the regression... We do have one table that don't render in the app anymore when upgrading the lib

[ddolcimascolo]

I'm currently trying to repro in a repo, or a codesandbox. I'll keep you posted

[ddolcimascolo]

So the width warning is something else, I know how to fix that. For a reason the component wrapping the grid renders a first time without a width, I detected that with a resize observer. We do use React Suspense a lots to load translations, etc. so it might be related.

Using a simple hack like widthIsNotZero && <DataGrid ... /> makes the warning disappear and removes a "flickering" due to the rerender of the grid when parent resizes. So it's a "bug" on our side and can be fixed.

However, in 5.0.1 I still have the table that does not render. I'll try to reproduce this issue and get back to you

[ddolcimascolo]

I moved on a bit, but still can't reproduce outside my app. My findings are

• The non rendered table uses autoHeight and is under a React <Suspense /> but before a component that actually suspend
• The grid parent is a MUI <Grid item xs />
• The table uses a static list of columns and a static list of rows. autoHeight is only used to make the parent grow and adapt to the number of rows, when we change the static list of rows
• The warning about width or height happens when the grid renders while the UI suspends because of another component in the tree. I can confirm this is unrelated to the actual issue of the table not rendering. Removing <Suspense /> or waiting before rendering the grid removes the warning but the table still does not render
• Setting a height to the parent of the grid makes it render fine

I have 6 remaining failing tests that are all failures because in 5.0.1 the data grid does not render the same number of rows. All data Grids under test use autoHeight. I can probably fix them but I still don't understand why the number of rendered rows is not the same as in 5.0.0...

Cheers,
David

[ddolcimascolo]

OK @flaviendelangle @m4theushw I finally managed to reproduce, and it was dumber than expected. As often you may say!

Closing this one in favor of #3233 that I just created

[ddolcimascolo]

For the record, the failing tests were due to missing mocks of offsetHeight / offsetWidth that were not needed before 5.0.1. If someone comes by, here's what we do

  Object.defineProperties(HTMLElement.prototype, {
    offsetWidth: { get: () => /* Your width here */ },
    offsetHeight: { get: () => /* Your height here */ }
  });

[m4theushw]

Could you share all of the properties you had to mock to be able to unit test with jsdom? We want to add a page to help to set up a testing environment in #1151.