[test] Scope the tests to just Material UI components #35219

siriwatknp · 2022-11-21T10:23:21Z

The current conformance test suite makes it hard to test slots prop on Joy UI components #34997 because it is used in Material UI tests.

This PR moves the slots to components so that it reflects only Material UI components. This will make the transition to slots prop easier for Material UI because, in v6, the slots prop will behave the same as Joy UI.

Changes

move slots key to components key to test Material UI components (in [Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components #34997, Joy UI will use slots key). The test logics remain the same.

// mui-material/src/Alert/Alert.test.js
describeConformance({
   ...
-   slots: { root: { ... } } 
+   components: { root: { ... } }
})

The future

#34997

Joy UI will run the slots prop tests which are separate from the current Material UI tests, allow them to coexist in the same describeConformance.

// describeConformance.js
function testSlotsProp() {
  ...test Joy UI and Material UI v6 (in the future)
}

function testSlotPropsProp() {
  ...test Joy UI and Material UI v6 (in the future)
}

// mui-joy/src/Alert/Alert.test.tsx
describeConformance({
   ...
+   slots: { root: { ... } } 
})

v6

Material UI: add slots key to conformance which runs the same tests as Joy UI. components tests remain the same.

describeConformance({
   ...
+   slots: { root: { ... } } 
})

v7

Material UI: remove components tests because it is removed from the prop.

describeConformance({
   ...
-   components: { root: { ... } } 
})

I have followed (at least) the PR section of the contributing guide.

This reverts commit ee3091f.

…slots-slotprops

mui-bot · 2022-11-21T10:26:05Z

	Messages
📖	Netlify deploy preview: https://deploy-preview-35219--material-ui.netlify.app/

No bundle size changes

Generated by 🚫 dangerJS against 0faa8b3

siriwatknp · 2022-11-21T11:03:23Z

test/utils/describeConformance.js

+      });
+    } else {
+      // eslint-disable-next-line react/prop-types
+      const CustomComponent = React.forwardRef(({ className }, ref) => (


Below are the same tests from #34873

…/use-components

siriwatknp · 2022-11-23T10:13:45Z

@michaldudak Could you take a quick look at this? The change might be hard to review but it is all about moving the tests to Material UI scope so that I can add similar tests for Joy UI.

The idea is to have a smooth transition for Material UI tests in v6, v7.

hbjORbj · 2022-11-23T12:04:27Z

I agree with the idea completely. Does this PR change slots back to components in conformance test suite for all material components?

hbjORbj · 2022-11-23T14:47:40Z

test/utils/describeConformance.tsx

+
+    if (!components) {
+      // if `components` are not defined, do not run tests that depend on them
+      filteredTests = filteredTests.filter((testKey) => !['componentsPropsProp'].includes(testKey));


What about componentsProp?

Those tests are in function testMaterialUIComponentsPropsProp().

I guess we will need another big refactoring but that can be done in a separate PR.

hbjORbj · 2022-11-23T15:21:38Z

test/utils/describeConformance.tsx

@@ -41,6 +41,7 @@ export interface InputConformanceOptions {
    mount: (node: React.ReactNode) => ReactWrapper,
  ) => (node: React.ReactNode) => ReactWrapper;
  slots?: Record<string, SlotTestingOptions>;
+  components?: Record<string, SlotTestingOptions>;


leave a comment explaining the current usage of components and slots?
e.g.,
slots are for Material UI v6 and Joy UI
components are for Material UI v6; Expected to be removed from v7

Good point.

hbjORbj

Left a few comments.

test/utils/describeConformance.tsx

Signed-off-by: Siriwat K <siriwatkunaporn@gmail.com>

michaldudak · 2022-11-24T08:08:15Z

This will make the transition to slots prop easier for Material UI because, in v6, the slots prop will behave the same as Joy UI.

What's the difference between Material UI v5 and Joy in terms of the slots prop currently?

siriwatknp · 2022-11-25T03:53:37Z

This will make the transition to slots prop easier for Material UI because, in v6, the slots prop will behave the same as Joy UI.

What's the difference between Material UI v5 and Joy in terms of the slots prop currently?

For slots prop, the name of the slot is in camel case for Joy UI, but Material UI uses PascalCase.

For slotProps prop:

Joy UI supports callback (ownerState) => ... whereas Material UI does not.
Joy UI supports slotProps: { $slot: { component: '...' }} whereas Material UI does not.

With the above conditions, mixing both packages in the same test suite would create a lot of if-else which is not good for maintainability and transition for Material UI.

michaldudak · 2022-11-25T09:03:59Z

The idea for the slots/slotProps is to be as similar between products as possible, even today.

For slots prop, the name of the slot is in camel case for Joy UI, but Material UI uses PascalCase.

That's not true. Material UI components use camelCase as well:

https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Alert/Alert.d.ts#L114-L116
https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Backdrop/Backdrop.d.ts#L73-L75
etc.

Joy UI supports callback (ownerState) => ... whereas Material UI does not.
Joy UI supports slotProps: { $slot: { component: '...' }} whereas Material UI does not.

This isn't something that needs v6 to be implemented.
We can add a separate test that verifies if the callback work and add them to the skip array in components that don't have it implemented yet. This would allow us to easily spot which Material UI components do not conform with the standard yet and what needs to be done to make them conformant (it would be finer-grained control).

siriwatknp · 2022-11-25T09:17:59Z

That's not true. Material UI components use camelCase as well:

My bad, I meant Material UI components prop.

This isn't something that needs v6 to be implemented.
We can add a separate test that verifies if the callback work and add them to the skip array in components that don't have it implemented yet. This would allow us to easily spot which Material UI components do not conform with the standard yet and what needs to be done to make them conformant (it would be finer-grained control).

Okay, I got your point. Will update the PR but won't include implementation to support callback, is this align with you?

michaldudak · 2022-11-25T10:54:29Z

I'd say we should leave the slots prop in conformance tests as they were previously, add a test that verifies the slotProps as callbacks (and skip them in Material UI components), and perhaps add a flag testLegacyComponentsProp that would determine if the tests should take into consideration the older components/componentsProps prop (this could be false by default and only set in Material UI tests).

This reverts commit 3a6b30a.

…/use-components

siriwatknp · 2022-11-28T11:16:58Z

I'd say we should leave the slots prop in conformance tests as they were previously, add a test that verifies the slotProps as callbacks (and skip them in Material UI components), and perhaps add a flag testLegacyComponentsProp that would determine if the tests should take into consideration the older components/componentsProps prop (this could be false by default and only set in Material UI tests).

Updated!

michaldudak · 2022-11-28T13:12:00Z

packages/mui-material/src/Autocomplete/Autocomplete.test.js

+        'componentsProp',
+        'slotsProp',
+        'reactTestRenderer',
+        'slotPropsCallback',


Would you mind adding a comment to explain why this is skipped (for all instances)? // not supported yet should be enough.

Sure, thanks for asking.

michaldudak · 2022-11-28T13:12:52Z

test/utils/describeConformance.tsx

+  });
+}
+
+function testSlotPropsCallback(element: React.ReactElement, getOptions: () => ConformanceOptions) {


siriwatknp added 10 commits November 18, 2022 17:23

use getBy for better DX

ee3091f

move Material UI components test

ae5629c

fix Alert

52df009

update conformance

397c96d

Revert "use getBy for better DX"

842cf9b

This reverts commit ee3091f.

update Autocomplete

fc8a694

fix AvatarGroup

3fa5754

rename to components

3a6b30a

Merge branch 'master' of https://github.com/mui/material-ui into joy/…

a0771dd

…slots-slotprops

add comment

e14e4fb

siriwatknp added 5 commits November 21, 2022 17:36

fix misused variables

bd95b34

remove outdated tests

1005da0

add comment

97713b2

fix pagination tests

5585d42

mark components and slots as optional

ecbdb3d

siriwatknp commented Nov 21, 2022

View reviewed changes

siriwatknp requested review from michaldudak, hbjORbj and mnajdova November 21, 2022 11:04

zannager added the test label Nov 21, 2022

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 22, 2022

siriwatknp added 2 commits November 23, 2022 15:45

Merge branch 'master' of https://github.com/mui/material-ui into test…

f0a43d9

…/use-components

fix lint

200986d

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 23, 2022

hbjORbj reviewed Nov 23, 2022

View reviewed changes

siriwatknp commented Nov 24, 2022

View reviewed changes

test/utils/describeConformance.tsx Outdated Show resolved Hide resolved

Update test/utils/describeConformance.tsx

d9203c7

Signed-off-by: Siriwat K <siriwatkunaporn@gmail.com>

update comment

28a891e

siriwatknp added 4 commits November 28, 2022 17:16

Revert "rename to components"

fcce5c6

This reverts commit 3a6b30a.

Merge branch 'master' of https://github.com/mui/material-ui into test…

98e54ab

…/use-components

revert

2f2feb9

fix tests

e2aba9a

michaldudak reviewed Nov 28, 2022

View reviewed changes

add comment

0faa8b3

siriwatknp requested a review from michaldudak November 28, 2022 14:37

michaldudak approved these changes Nov 28, 2022

View reviewed changes

siriwatknp merged commit 0c7a5cb into mui:master Nov 29, 2022

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022

[test] Scope the tests to just Material UI components (mui#35219)

384e0eb

feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022

[test] Scope the tests to just Material UI components (mui#35219)

f77a1df

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[test] Scope the tests to just Material UI components #35219

[test] Scope the tests to just Material UI components #35219

siriwatknp commented Nov 21, 2022 •

edited

mui-bot commented Nov 21, 2022 •

edited

siriwatknp Nov 21, 2022

siriwatknp commented Nov 23, 2022

hbjORbj commented Nov 23, 2022

hbjORbj Nov 23, 2022

siriwatknp Nov 24, 2022

siriwatknp Nov 24, 2022

hbjORbj Nov 23, 2022

siriwatknp Nov 24, 2022

hbjORbj left a comment

michaldudak commented Nov 24, 2022

siriwatknp commented Nov 25, 2022 •

edited

michaldudak commented Nov 25, 2022

siriwatknp commented Nov 25, 2022

michaldudak commented Nov 25, 2022

siriwatknp commented Nov 28, 2022

michaldudak Nov 28, 2022

siriwatknp Nov 28, 2022

michaldudak Nov 28, 2022

[test] Scope the tests to just Material UI components #35219

[test] Scope the tests to just Material UI components #35219

Conversation

siriwatknp commented Nov 21, 2022 • edited

Changes

The future

v6

v7

mui-bot commented Nov 21, 2022 • edited

Choose a reason for hiding this comment

siriwatknp commented Nov 23, 2022

hbjORbj commented Nov 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hbjORbj left a comment

Choose a reason for hiding this comment

michaldudak commented Nov 24, 2022

siriwatknp commented Nov 25, 2022 • edited

michaldudak commented Nov 25, 2022

siriwatknp commented Nov 25, 2022

michaldudak commented Nov 25, 2022

siriwatknp commented Nov 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp commented Nov 21, 2022 •

edited

mui-bot commented Nov 21, 2022 •

edited

siriwatknp commented Nov 25, 2022 •

edited