Skip to content

DataGrid - stabilize tests - #34642

Open
Tucchhaa wants to merge 5 commits into
DevExpress:mainfrom
Tucchhaa:stabilize_tests_26_2
Open

DataGrid - stabilize tests#34642
Tucchhaa wants to merge 5 commits into
DevExpress:mainfrom
Tucchhaa:stabilize_tests_26_2

Conversation

@Tucchhaa

@Tucchhaa Tucchhaa commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Tucchhaa Tucchhaa self-assigned this Aug 4, 2026
Copilot AI review requested due to automatic review settings August 4, 2026 11:43
@Tucchhaa
Tucchhaa requested a review from a team as a code owner August 4, 2026 11:43
@Tucchhaa Tucchhaa added the 26_2 label Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies DataGrid TestCafe E2E tests in an attempt to improve stability around hover rendering and React header template rendering checks.

Changes:

  • Reworked the “row hover with fixed columns” sticky-columns test to hover a specific cell rather than the whole row.
  • Added explicit readiness/content assertions before taking screenshots in the Select All + headerCellTemplate (React) test.
  • Introduced repeated (looped) execution patterns for the affected tests (and a test.only in one file), which currently makes the suite heavier and risks altering what runs in CI.

Reviewed changes

Copilot reviewed 2 out of 102 changed files in this pull request and generated 2 comments.

File Description
e2e/testcafe-devextreme/tests/dataGrid/sticky/common/stickyColumns.ts Changes hover target for the fixed-columns row-hover screenshot test; currently introduces repeated execution and test.only.
e2e/testcafe-devextreme/tests/dataGrid/common/selection.ts Adds readiness assertions for the React header template scenario; currently runs the same screenshot test many times with varying screenshot names.
Suppressed comments (1)

e2e/testcafe-devextreme/tests/dataGrid/common/selection.ts:61

  • The screenshot name is currently parameterized with i, which forces maintaining multiple etalons for the same scenario and makes the test harder to maintain. If the test is stable, prefer a single canonical screenshot name.

Comment thread e2e/testcafe-devextreme/tests/dataGrid/sticky/common/stickyColumns.ts Outdated
Comment thread e2e/testcafe-devextreme/tests/dataGrid/common/selection.ts Outdated
Copilot AI review requested due to automatic review settings August 4, 2026 12:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 104 changed files in this pull request and generated no new comments.

Suppressed comments (6)

e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1792

  • test.only(...) will cause TestCafe to run only this test and skip the rest of the suite, which is almost never intended in committed code.
    e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1821
  • Leftover debugging comment (// fails here) should be removed before merging to keep the test suite clean and avoid implying a known failing assertion.
    e2e/testcafe-devextreme/tests/dataGrid/sticky/common/stickyColumns.ts:92
  • Defining the same visual test 50 times via a for-loop will multiply E2E runtime and screenshots work by 50x. If this was intended for local flakiness investigation, it shouldn’t be committed as-is; prefer a single test and use the runner’s retry/quarantine mechanisms instead (runner.ts treats meta.unstable as quarantined).
    e2e/testcafe-devextreme/tests/dataGrid/common/selection.ts:48
  • This previously ran as a single quarantined/unstable visual test; the new for-loop defines 50 separate tests, which will significantly increase suite runtime and still won’t provide a stable pass/fail signal. Consider keeping this as one test and relying on meta.unstable (runner.ts treats it as quarantined) and/or retryAttempts instead of cloning tests.
    e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/startEditing.functional.ts:127
  • The new for-loop defines this same keyboard-navigation test 100 times, which will dramatically increase CI time and load without changing coverage. This looks like local flakiness debugging code and should be reduced back to a single test (use retries/quarantine if needed).
    e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1791
  • This loop defines 100 copies of the same test, which will heavily slow down E2E runs and create redundant work. If you need repeated runs for flakiness investigation, prefer runner retries/quarantine instead of duplicating tests.

This issue also appears in the following locations of the same file:

  • line 1792
  • line 1821

Copilot AI review requested due to automatic review settings August 4, 2026 12:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 104 changed files in this pull request and generated no new comments.

Suppressed comments (5)

e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1821

  • Leftover debugging comment (// fails here) should be removed before merging; it doesn’t provide actionable context and will go stale as the test changes.
    e2e/testcafe-devextreme/tests/dataGrid/sticky/common/stickyColumns.ts:96
  • Wrapping this screenshot test in a 50-iteration loop multiplies runtime and adds 50 near-duplicate test cases. The e2e runner already supports rerunning failed tests (e2e/testcafe-devextreme/runner.ts:395-430), so this repetition should not be committed as a stabilization mechanism.

Reduce this to a single test case (or rely on the runner retries/quarantine mechanisms) to avoid bloating CI time and reporting.
e2e/testcafe-devextreme/tests/dataGrid/common/selection.ts:52

  • This adds 50 duplicate TestCafe tests, which will significantly increase the DataGrid e2e runtime and clutter the test report. If the goal is flake mitigation, prefer the existing retry/quarantine mechanisms in the runner instead of multiplying test definitions.

Please keep a single test case here.
e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/startEditing.functional.ts:128

  • Repeating this functional test 100 times will heavily inflate suite runtime without improving determinism (each run still has to pass). Please keep this as a single test and use the runner’s retry/quarantine mechanisms if flakiness remains.
    e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1794
  • This introduces 100 duplicate tests per edit mode, which will drastically increase execution time and make failures harder to triage. The existing retry/quarantine mechanisms are a better fit for stabilizing flakes.

Please keep this as a single test case per edit mode.

This issue also appears on line 1821 of the same file.

Copilot AI review requested due to automatic review settings August 4, 2026 13:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 105 changed files in this pull request and generated no new comments.

Suppressed comments (5)

e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1821

  • Leftover debug comment // fails here should not be committed; it adds noise without actionable information. If this assertion is flaky, stabilize it with an assertion timeout or a deterministic wait instead.
    e2e/testcafe-devextreme/tests/dataGrid/common/selection.ts:51
  • This change repeats the same screenshot assertion 50 times by generating 50 distinct tests and 50 screenshot names. That will substantially slow the suite and makes the test harder to triage. It also drops the previous test.meta({ unstable: true }), which is how the e2e runner quarantines flaky tests (see e2e/testcafe-devextreme/runner.ts). Prefer a single test with an explicit wait for the async header template (as you already added) and keep it quarantined (unstable meta) if it is still flaky, rather than duplicating the test.
    e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/keyboardNavigation.functional.ts:1794
  • The for (let i = 0; i < 100; i++) loop will create 100 extra tests per editMode value. Given this file is already parameterized by edit mode, this can balloon to hundreds of tests, significantly extending e2e runtime and increasing memory/state-leak risk under disablePageReloads patterns elsewhere. Prefer stabilizing the single test via proper waits/timeouts (e.g., t.expect(...).ok({ timeout: ... })) and/or relying on the runner’s retry/quarantine features rather than duplicating the test.

This issue also appears on line 1821 of the same file.
e2e/testcafe-devextreme/tests/dataGrid/common/keyboardNavigation/startEditing.functional.ts:139

  • Wrapping this scenario in for (let i = 0; i < 100; i++) creates 100 identical tests, each recreating the widget in a .before hook. This will drastically increase runtime and can hide the underlying flakiness rather than fixing it. Prefer a single test and stabilize it (e.g., by using assertion timeouts on focus/editor readiness) and/or rely on the existing e2e runner retry/quarantine mechanisms instead of test duplication.
    e2e/testcafe-devextreme/tests/dataGrid/sticky/common/stickyColumns.ts:96
  • The new for (let i = 0; i < 50; i++) loop turns one visual test into 50 separate TestCafe tests (each running its own .before(createWidget(...))). This will significantly increase suite runtime and can amplify state leakage because the fixture uses disablePageReloads. It also removes the previous unstable: true meta, which the repo’s e2e runner uses to quarantine flaky tests (see e2e/testcafe-devextreme/runner.ts logic that treats testMeta.unstable as quarantined). Prefer keeping a single test and stabilizing it via deterministic waits/timeouts (or leaving it quarantined) rather than multiplying test cases.

Copilot AI review requested due to automatic review settings August 4, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

}));

test(`${editMode} mode - Cells in a modified row should be updated on Tab (T898356)`, async (t) => {
test(`${i}: ${editMode} mode - Cells in a modified row should be updated on Tab (T898356)`, async (t) => {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants