-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
[core] Setup e2e tests #1443
[core] Setup e2e tests #1443
Conversation
is it running in the CI? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it running in the CI?
No
columns: [{ field: 'brand', width: 100 }], | ||
}; | ||
|
||
export default function BaseGridFocus() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that the file structure is flying. Material-UI X while host more components. I think that a prefix with the name of the component and a flat structure after would work a lot better. So 👍 for test/e2e/fixtures/DataGrid/KeyboardNavigationFocus.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was not convinced about the naming. I'll update it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Should I add a new section in the |
Yes, we need a new CI step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Side notes for the future:
- We might be able to share some of the visual regression and end-to-end set up with the main repo. So improvements that @eps1lon bring in the core repo can benefit the whole codebase. But maybe it's too early
- The test assertion seems to be a weak spot.
expect(await page.evaluate(()
adds quite some boilerplate. - We could have Storybook render all the content inside
test/e2e/fixtures/
. These new tests are already rendered by the end-to-end infra but maybe having them inside Storybook could serve use cases as well. Not sure, it could also slow the storybook even more.
This PR aims to provide the ability to test real browser interaction based on this pr mui/material-ui#25405
I've added a simple test case to illustrate that the setup is working.