Skip to content

Conversation

@Ocheretovich
Copy link

Removes obsolete TODO comments from table component tests without changing test behavior

@mui-bot
Copy link

mui-bot commented Jan 19, 2026

Netlify deploy preview

https://deploy-preview-47656--material-ui.netlify.app/

Bundle size report

Bundle Parsed size Gzip size
@mui/material 0B(0.00%) 0B(0.00%)
@mui/lab 0B(0.00%) 0B(0.00%)
@mui/system 0B(0.00%) 0B(0.00%)
@mui/utils 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 62c79b8

@ZeeshanTamboli
Copy link
Member

Removes obsolete TODO comments from table component tests without changing test behavior

How is it obselete?

@Ocheretovich
Copy link
Author

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

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Jan 20, 2026

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 TableCell rather than asserting against TableContext / Tablelvl2Context directly. That still feels relevant to me, because TableCell is the primary consumer of these context values and represents how the Table APIs are actually used by developers.

Testing only the context providers verifies internal wiring, but it doesn't exercise the real integration path. An integration-style test using TableCell would better validate that the context values are correctly interpreted and applied in practice.

From that perspective, I'd expect the TODO to be resolved by updating the tests to use TableCell, and then removing the comment. Simply deleting the TODO seems to drop that intent without addressing it.

@ZeeshanTamboli ZeeshanTamboli added test scope: table Changes related to the table. labels Jan 20, 2026
@ZeeshanTamboli ZeeshanTamboli changed the title test: cleanup table test TODO comments [table] Cleanup table test TODO comments Jan 20, 2026
@mj12albert
Copy link
Member

If they are indeed actionable, we should create an issue to track it rather than rely on the TODO

@ZeeshanTamboli
Copy link
Member

@Ocheretovich @mj12albert I add a Table integration test which covers the context logic. However, most of it is covered in packages/mui-material/test/integration/TableCell.test.js. But it is better to have a test involving the various Table components as well. This resolves the TODOs. @mj12albert Could you review?

@ZeeshanTamboli ZeeshanTamboli changed the title [table] Cleanup table test TODO comments [table] Cleanup table test TODOs Jan 21, 2026
@ZeeshanTamboli ZeeshanTamboli changed the title [table] Cleanup table test TODOs [test] Cleanup Table tests TODOs Jan 21, 2026
@ZeeshanTamboli ZeeshanTamboli added the internal Behind-the-scenes enhancement. Formerly called “core”. label Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Behind-the-scenes enhancement. Formerly called “core”. scope: table Changes related to the table. test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants