Skip to content

[test] Assert the edited dateTime cell value in the e2e edit test#22982

Merged
LukasTy merged 4 commits into
mui:masterfrom
voidmatcha:fix/e2e-datetime-edit-assertion
Jun 29, 2026
Merged

[test] Assert the edited dateTime cell value in the e2e edit test#22982
LukasTy merged 4 commits into
mui:masterfrom
voidmatcha:fix/e2e-datetime-edit-assertion

Conversation

@voidmatcha

Copy link
Copy Markdown
Contributor

Summary

The "should edit date cells" e2e test edits the lastConnection dateTime grid cell, presses Enter,
then checks the result with:

expect(page.getByText('1/31/2025, 4:05:00 PM')).not.to.equal(null);

page.getByText returns a Locator, which is never null, so this assertion passes whether or not the
dateTime edit applied. It is the only check of the dateTime edit, so a regression that broke
dateTime editing would not fail this test.

Changelog

Read the rendered cell text and assert it equals the edited value, wrapped in the file's existing
waitFor helper. This mirrors the date column and the initial dateTime assertions already in the
same test, so the dateTime edit is now actually verified. No production code changes; test/e2e only.

The e2e suite was not run locally (it requires building the full monorepo and browsers); the change
uses only assertion patterns already present in the same test.

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

Found while reviewing the test suite with e2e-skills/e2e-reviewer.

The final check in the edit date cells e2e test asserted
page.getByText('1/31/2025, 4:05:00 PM') is not null. getByText returns a
Locator, which is never null, so the assertion passed whether or not the
dateTime edit applied, leaving the dateTime edit effectively unverified.

Read the rendered cell text and assert it equals the edited value, matching
the date column and initial dateTime assertions already in the same test.
@code-infra-dashboard

code-infra-dashboard Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploy preview

https://deploy-preview-22982--material-ui-x.netlify.app/

Bundle size

Bundle Parsed size Gzip size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium 0B(0.00%) 0B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)
@mui/x-license 0B(0.00%) 0B(0.00%)

Details of bundle changes


Check out the code infra dashboard for more information about this PR.

Reading the cell textContent could observe an empty cell during the
post-edit re-render. Wait for the edited value via getByText instead,
which retries until the value appears and fails if it never does.
The lastConnection edit commits as 1/31/2025, 4:05:00 AM. Typing the
trailing "p" does not reach the meridiem section, so the cell keeps the
AM it started with. Assert that value so the check verifies the edit
instead of waiting for text that never renders.
@voidmatcha

voidmatcha commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Update on the dateTime assertion.

The original assertion was expect(page.getByText('1/31/2025, 4:05:00 PM')).not.to.equal(null). A Playwright locator is never null, so it always passed and never checked the edit.

The edit cell for the dateTime column is a native datetime-local input, whose value is normalized to the ISO format regardless of how it is displayed. Typing display text into it is locale and browser dependent: running the test locally, the chromium path committed 4:05 AM and the firefox path committed an empty cell, so no single typed sequence produced a consistent value.

The edit now fills the input with the ISO value it normalizes to:

await dateTimeInput.fill('2025-01-31T16:05');

With that, chromium and firefox both commit 1/31/2025, 4:05:00 PM, which is the value this test always intended. The assertion now waits for that text, so it fails if the edit does not commit, and the browser specific typing branch is removed. Verified locally on chromium and firefox.

The dateTime edit cell is a native datetime-local input, whose value is
normalized to the ISO format regardless of how it is displayed. Typing
display text into it was locale and browser dependent: the chromium path
committed 4:05 AM and the firefox path committed an empty cell. Set the
value with fill using the ISO string, then wait for the committed cell
text. This drops the browser specific branch and makes the edit
actually verifiable.
@voidmatcha

Copy link
Copy Markdown
Contributor Author

Pushed a revised fix and updated the note above.

Short version: the dateTime edit cell is a native datetime-local input, so typing display text was unreliable across browsers (chromium committed AM, firefox committed an empty cell). Filling the input with the ISO value makes chromium and firefox both commit 1/31/2025, 4:05:00 PM, and the assertion now waits for that value.

@LukasTy LukasTy added test scope: data grid Changes related to the data grid. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. labels Jun 29, 2026

@LukasTy LukasTy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for a nice improvement! 🙏

@LukasTy
LukasTy merged commit 88164ea into mui:master Jun 29, 2026
25 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: data grid Changes related to the data grid. test type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants