Scheduler - Fix unstable tests - QUnit - integration.dstAppointments.tests.js#32643
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes unstable QUnit tests for DST (Daylight Saving Time) appointments in the Scheduler component by unskipping previously skipped test modules and updating them with proper test patterns to ensure stability.
Changes:
- Unskipped two test modules by changing
QUnit.skiptoQUnit.module - Added
legacyForm: trueconfiguration to tests that verify appointment form behavior - Implemented fake timer pattern for appointment clicks to prevent timing issues
- Removed several tests that checked appointment rendering positions (likely unstable)
- Updated assertions to expect appointment timezone times instead of converted scheduler timezone times (matching legacyForm behavior)
- Removed unused
timeZoneUtilsimport - Removed one complete test about recurrence exception deletion behavior
Comments suppressed due to low confidence (1)
packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/integration.dstAppointments.tests.js:360
- The assertion message 'Appointment was deleted' is misleading. This test verifies that a recurrence exception is not rendered after timezone adjustment, not that an appointment was deleted. The test doesn't perform any delete action. Consider changing the message to something like 'exception is not rendered' or 'correct number of events after timezone change' to accurately reflect what the test is verifying.
assert.equal(scheduler.appointments.getAppointmentCount(), 0, 'Appointment was deleted');
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/integration.dstAppointments.tests.js:35
- Stubbing
Date.prototype.getTimezoneOffsetin the module hook is a very broad change that can impact unrelated Date logic inside the Scheduler, Sinon fake timers, and other helpers. In this codebase, timezone-dependent tests typically stubtimeZoneUtils.getClientTimezoneOffset(and/or othertimeZoneUtilshelpers) instead of modifyingDate.prototypedirectly (e.g.testing/tests/DevExpress.ui.widgets.scheduler/timezones.tests.jsstubsgetClientTimezoneOffset). Consider replacing theDate.prototypestub with atimeZoneUtils.getClientTimezoneOffsetstub (or limiting the stub to the specific helper under test) to reduce side effects and keep the approach consistent with existing tests.
this.tzStub = sinon.stub(timeZoneUtils, 'getDiffBetweenClientTimezoneOffsets').returns(0);
// Stub getTimezoneOffset to return 0 (UTC) to avoid CI timezone affecting Date operations
this.timezoneOffsetStub = sinon.stub(Date.prototype, 'getTimezoneOffset').returns(0);
},
packages/devextreme/testing/tests/DevExpress.ui.widgets.scheduler/integration.dstAppointments.tests.js:382
- The final assertion message is misleading here: the test verifies that no occurrences are rendered due to
recurrenceException, but the message says "Appointment was deleted". Updating the assertion text to describe the expected behavior (no appointment rendered because the occurrence is excluded) will make failures easier to diagnose.
assert.equal(scheduler.appointments.getAppointmentCount(), 0, 'Appointment was deleted');
…ility and maintainability
ad65879 to
b31995a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
packages/devextreme/js/__internal/scheduler/tests/integration.dstAppointments.test.ts:215
- This test name says it checks appointment width, but it only asserts that at least one appointment exists (no width/geometry assertion). Consider asserting width via the appointment model geometry (or rename the test to reflect the weaker expectation) to avoid losing coverage compared to the original intent.
// NOTE: Timezone-sensitive test, use US/Pacific for proper testing
it('Appointment which started in DST and ended in STD time should have right width, timeline view', async () => {
const startDate = new Date(2018, 10, 4, 1);
const endDate = new Date(2018, 10, 4, 3);
const currentDate = new Date(2018, 10, 4);
packages/devextreme/js/__internal/scheduler/tests/integration.dstAppointments.test.ts:309
- The test currently uses
jest.runAllTimers()and then switches back to real timers mid-test, and it queries the delete button via a globaldocument.querySelector. For stability/consistency with other scheduler tests, prefer advancing only the needed tooltip delay (e.g.,advanceTimersByTime) and locate the delete button relative to the tooltip element (e.g., viaPOM.getTooltipAppointment()), restoring timers in afinallyif you switch modes.
jest.useFakeTimers();
const appointment = POM.getAppointment('Recruiting students');
appointment.element?.click();
jest.runAllTimers();
jest.useRealTimers();
const tooltipDeleteButton = document.querySelector<HTMLElement>(
'.dx-tooltip-appointment-item-delete-button',
);
expect(tooltipDeleteButton).not.toBeNull();
(tooltipDeleteButton as HTMLElement).click();
packages/devextreme/js/__internal/scheduler/tests/integration.dstAppointments.test.ts:26
- The test title says it validates tooltip/popup behavior, but the test only asserts the appointment display date and never opens the tooltip or the edit popup. Either rename the test to match what’s asserted, or add explicit assertions that the tooltip and popup display the expected times.
it('Any recurrence appt part should have correct tooltip and popup if recurrence starts in STD and ends in DST in custom timezone, appointment timezone is set (T804886)', async () => {
// NOTE: The daylight saving changed in Montreal on 10.03.2019 and in Paris on 31.03.2019
packages/devextreme/js/__internal/scheduler/tests/integration.dstAppointments.test.ts:64
- The test name mentions tooltip/popup validation, but the body only checks the appointment’s display date and doesn’t interact with tooltip or popup UI. Please rename the test or add the missing UI assertions to keep the intent accurate.
it('Recurrence appt part at the time of DST should have correct tooltip and popup if recurrence starts in STD and ends in DST in custom timezone, appointment timezone is set (T804886)', async () => {
// NOTE: The daylight saving changed in Montreal on 10.03.2019 and in Paris on 31.03.2019
packages/devextreme/js/__internal/scheduler/tests/integration.dstAppointments.test.ts:97
- The test title claims tooltip/popup verification, but the assertions only cover the appointment’s display date and no tooltip/popup is opened. Rename the test or add assertions for tooltip/popup content to match the stated scenario.
it('Recurrence appt part at the time of DST-end should have correct tooltip and popup if recurrence starts in DST and ends in STD in custom timezone, appointment timezone is set (T804886)', async () => {
// NOTE: The daylight saving changed backward in Montreal on 03.11.2019 and
// in Paris on 27.10.2019
packages/devextreme/js/__internal/scheduler/tests/integration.dstAppointments.test.ts:191
- The test title says it verifies correct dates and position, but it only asserts the appointment text and display date and doesn’t check geometry/position. Either update the name or add assertions using the appointment model’s geometry (e.g., translate/width/height) to actually cover positioning.
it('Second recurring appointment which started in STD and ended in DST time should have correct start & end dates & position', async () => {
const startDate = new Date(1520748000000);
const endDate = new Date(1520751600000);
No description provided.