-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
[test] Cleanup Table tests TODOs #47656
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
base: master
Are you sure you want to change the base?
Conversation
Netlify deploy previewhttps://deploy-preview-47656--material-ui.netlify.app/ Bundle size report
|
How is it obselete? |
|
These TODOs are no longer actionable. The relevant behavior is already covered by existing tests and the original integration path has changed since the comments were added |
I don't think they are irrelevant. Why do you think so? The comments specifically suggest testing the integration via Testing only the context providers verifies internal wiring, but it doesn't exercise the real integration path. An integration-style test using From that perspective, I'd expect the TODO to be resolved by updating the tests to use |
|
If they are indeed actionable, we should create an issue to track it rather than rely on the |
|
@Ocheretovich @mj12albert I add a Table integration test which covers the context logic. However, most of it is covered in |
Removes obsolete TODO comments from table component tests without changing test behavior