Scheduler - Qunit tests - remove watchers of private methods#32994
Scheduler - Qunit tests - remove watchers of private methods#32994aleksei-semikozov wants to merge 2 commits intoDevExpress:26_1from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates DevExtreme Scheduler QUnit tests to avoid spying on (and tightly coupling to) private/internal methods, replacing those assertions with checks based on public behavior and DOM outcomes.
Changes:
- Replaced a spy on
setRemoteFilterIfNeededwith acurrentDate-based assertion in the date navigator integration test. - Reworked resource-change coverage to validate the appointment popup form contains the expected resource editor rather than spying on form-creation internals.
- Replaced resize call-order spies with a DOM measurement asserting appointment layout recalculates after resize; updated a dataSource-loading test to assert via store
loadcalls.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/integration.dateNavigator.tests.js | Removes spying on a private scheduler method and replaces it with a public currentDate outcome check after navigation. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/common.options.tests.js | Replaces internal spies with public popup-form/editor checks; adjusts dataSource-load assertions to use CustomStore.load counts. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/common.events.tests.js | Replaces internal call-order spies with a public DOM-based assertion that appointment sizing changes after resize. |
| QUnit.test('Tasks should be rerendered after click on next/prev button', async function(assert) { | ||
| await this.createInstance({ currentDate: new Date(2015, 1, 24) }); | ||
| const initialDate = new Date(2015, 1, 24); | ||
| await this.createInstance({ currentDate: initialDate }); | ||
|
|
||
| const spy = sinon.spy(this.instance, 'setRemoteFilterIfNeeded'); | ||
| $(this.instance.$element()).find('.dx-scheduler-navigator-previous').trigger('dxclick'); |
There was a problem hiding this comment.
This test no longer verifies the behavior implied by its name (“Tasks should be rerendered…”). It now only checks that currentDate becomes earlier after clicking Previous, which is already covered by the existing “Click on the 'previous' button should update currentDate” tests above and doesn’t assert anything about task/appointment rerendering. Consider either renaming the test to match the new assertion or asserting a public rerendering outcome (e.g., a DOM change in rendered appointments/tasks after navigation).
| scheduler.instance.showAppointmentPopup({ | ||
| startDate: new Date(2017, 11, 18, 10), | ||
| endDate: new Date(2017, 11, 18, 11), | ||
| }, true); | ||
|
|
There was a problem hiding this comment.
This test’s new assertion is satisfied even if the appointment popup form is created lazily only when showAppointmentPopup is called, so it may not actually validate that the form is recreated in response to the resources option change. To keep the original intent without spying on internals, create/show the popup first (so a form instance exists), then change resources, then verify via a public signal that the form was rebuilt/updated (e.g., the resource editor appears/updates after the change).
| }); | ||
| await waitForAsync(() => count === 2); | ||
| await waitForAsync(() => loadCount === 2); | ||
| await waitAsync(200); |
There was a problem hiding this comment.
The extra waitAsync(200) introduces an arbitrary time-based delay that slows the suite and can still be flaky on slow/fast environments. Prefer a deterministic approach (e.g., sinon fake timers with a controlled tick, or a helper that waits until loadCount stays unchanged for a short period) so the test asserts “no additional loads happen” without fixed sleeps.
| await waitAsync(200); |
No description provided.