[test] Assert the edited dateTime cell value in the e2e edit test#22982
Conversation
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.
Deploy previewBundle size
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.
|
Update on the dateTime assertion. The original assertion was 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.
|
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
left a comment
There was a problem hiding this comment.
Thank you for a nice improvement! 🙏
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.
Found while reviewing the test suite with
e2e-skills/e2e-reviewer.