DataGrid: fix skipped qunit tests #32525
Conversation
There was a problem hiding this comment.
Pull request overview
This PR targets the DevExtreme QUnit test suite, aiming to address previously skipped DataGrid (and some TreeList) tests by enabling a subset of them, adjusting expectations, and removing several skipped tests entirely.
Changes:
- Re-enabled/updated several previously skipped DataGrid tests (selection/editing/reordering/fixed columns) with updated arrangements and assertions.
- Added a new DataGrid integration test to validate selection restoration from state storing.
- Removed multiple
QUnit.skip(...)tests across DataGrid and TreeList suites.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/testing/tests/DevExpress.ui.widgets.treeList/dataController.tests.js | Removes a previously skipped TreeList remote-operations filtering test. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/virtualScrolling.tests.js | Removes a previously skipped virtual scrolling controller test. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/selection.tests.js | Removes several skipped selection tests; re-enables one with a changed configuration (checkbox selection). |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/selection.integration.tests.js | Adds an integration test asserting selection restoration from state storing. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/editing.tests.js | Re-enables skipped editing tests and removes a skipped virtual scrolling + inserted-row positioning test. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/editing.integration.tests.js | Re-enables a skipped popup editRowKey initialization test with updated timing/assertions. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/columnsResizingReorderingModule.tests.js | Re-enables one test with updated expectation; removes two skipped column chooser separator movement tests. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/columnFixing.integration.tests.js | Re-enables a fixed-columns + column hiding test with updated assertions/selectors. |
Comments suppressed due to low confidence (5)
packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/selection.tests.js:555
- The skipped test around restoring selection from user state (stateStoringController.restoreSelectedItemKeys / selectedItemKeys) was removed, and there is no equivalent unit-level coverage for that controller API. If this scenario is still supported, consider re-enabling/fixing the test or adding a dedicated integration test that exercises the same restoreSelectedItemKeys path (not just stateStoring.selectedRowKeys).
QUnit.test('Set isSelected items', function(assert) {
this.applyOptions({
selection: { mode: 'multiple' }
});
packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/selection.tests.js:1883
- A skipped shift-selection scenario (selectAll -> ctrl toggle -> shift selection) was removed without a replacement elsewhere in the suite. If this behavior is still expected to work, consider re-enabling/fixing the test or adding a new one that validates the same interaction sequence.
QUnit.test('changeRowSelection with shift key. Change shift selection from down to down', function(assert) {
this.applyOptions({
selection: { mode: 'multiple' }
});
packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/virtualScrolling.tests.js:190
- The previously skipped "Load when dataSource pageIndex > 0" test was deleted rather than fixed/re-enabled, and no replacement exists in the test suite. This removes coverage for virtual scrolling controller behavior when starting from a non-zero pageIndex; consider restoring the test with correct expectations instead of deleting it.
QUnit.test('setContentItemSizes. No items', function(assert) {
this.scrollController.viewportSize(12);
this.scrollController.setContentItemSizes([]);
const virtualContentSize = this.scrollController.getVirtualContentSize();
packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/editing.tests.js:16699
- The skipped test "Change position of the inserted row when virtual scrolling" was removed and no equivalent test exists elsewhere. This drops coverage for inserted-row positioning across virtual scroll page changes; consider re-enabling/fixing the test or replacing it with a more stable assertion so regressions in this area are still detected.
// T258714
QUnit.test('Edit row after the virtual scrolling when there is inserted row', function(assert) {
// arrange
const testElement = $('#container');
packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/editing.integration.tests.js:6566
- In this enabled test, the grid init config no longer specifies
loadingTimeout: null(commonly used elsewhere in this file to avoid async timing). Depending on the default loadingTimeout,this.clock.tick(10)may be insufficient and make the test flaky; consider restoringloadingTimeout: null(or ticking the configured timeout) for deterministic assertions.
const dataGrid = $('#dataGrid').dxDataGrid({
dataSource: [{ id: 1 }, { id: 2 }],
keyExpr: 'id',
editing: {
allowUpdating: true,
| QUnit.test('Initial load when dataSource has filter and filterMode is withAncestors (default)', function(assert) { | ||
| // arrange, act | ||
| const loadingArgs = []; |
There was a problem hiding this comment.
A skipped test case was removed (filterMode = 'standard' with an initial dataSource filter), and there is no equivalent test elsewhere in the suite. This reduces coverage for remote-operations filtering in the TreeList data controller; consider re-enabling the test (fixing it if needed) or adding a new passing test that asserts the expected loadOptions for standard filter mode.
| QUnit.test('changeRowSelection. Multiple. Several calls on different rows with selection column visible', function(assert) { | ||
| this.applyOptions({ | ||
| selection: { mode: 'multiple' } | ||
| selection: { mode: 'multiple', showCheckBoxesMode: 'always' } | ||
| }); |
There was a problem hiding this comment.
This test was changed to require showCheckBoxesMode: 'always', which narrows coverage to the checkbox-selection UI path. If changeItemSelection is expected to work the same way when the selection column is not visible (default configuration), consider keeping an additional test for the default (no checkboxes) configuration so regressions there are still caught.
| QUnit.test('selection should be restored from state storing if defined', function(assert) { | ||
| // act | ||
| const dataGrid = createDataGrid({ | ||
| loadingTimeout: null, | ||
| dataSource: { |
There was a problem hiding this comment.
This newly added test duplicates the same createDataGrid + stateStoring customLoad setup used by the preceding stateStoring-related test(s). Consider merging the assertions or extracting a small helper for the repeated configuration to reduce future maintenance cost when state storing behavior changes.
| const getVisibleHeaderCells = () => { | ||
| const headerRow = instance.$element().find('.dx-header-row').first(); | ||
|
|
||
| // act | ||
| assert.equal($cells.length, 3, 'columns count'); | ||
| assert.equal($cells.eq(0).text(), 'First Name', 'First is \'firstName\' column'); | ||
| assert.equal($cells.eq(1).text(), 'Age', 'Second is \'firstName\' column'); | ||
| return $(headerRow.find('td.dx-datagrid-action:not(.dx-datagrid-hidden-column)')); | ||
| }; |
There was a problem hiding this comment.
The selector for visible header cells (td.dx-datagrid-action:not(.dx-datagrid-hidden-column)) is inconsistent with the pattern used in other column-hiding/adaptivity tests, which select .dx-header-row td:not(.dx-datagrid-hidden-column) (e.g., adaptiveColumns.integration.tests.js:110,125). Using dx-datagrid-action risks excluding regular header cells and coupling the test to internal CSS classes; consider aligning the selector with the established pattern.
| QUnit.test('Reset opacity for target element', function(assert) { | ||
| // arrange | ||
| const testElement = $('#container'); |
There was a problem hiding this comment.
Two skipped tests covering block-separator movement in the column chooser (dragging up/down) were removed, and there are no replacements elsewhere. If this behavior is still supported, consider restoring/fixing these tests or adding new ones to keep coverage for column-chooser reordering edge cases.
|
|
||
| // act | ||
| $('#container').width(800); | ||
| instance.updateDimensions(); |
There was a problem hiding this comment.
This test performs assertions immediately after updateDimensions() without advancing the fake clock. Similar column-hiding/adaptivity integration tests tick the clock after initialization and after dimension updates to let adaptivity/layout settle; adding this.clock.tick(...) here would reduce timing-related flakiness.
| instance.updateDimensions(); | |
| instance.updateDimensions(); | |
| this.clock.tick(10); |
1 participant
No description provided.