Skip to content

Comments

Scheduler - Fix unstable tests - QUnit - integration.dstAppointments.tests.js#32643

Open
aleksei-semikozov wants to merge 3 commits intoDevExpress:26_1from
aleksei-semikozov:fix/3261-scheduler-unstable-tests-dst_26_1
Open

Scheduler - Fix unstable tests - QUnit - integration.dstAppointments.tests.js#32643
aleksei-semikozov wants to merge 3 commits intoDevExpress:26_1from
aleksei-semikozov:fix/3261-scheduler-unstable-tests-dst_26_1

Conversation

@aleksei-semikozov
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings February 19, 2026 17:34
@aleksei-semikozov aleksei-semikozov requested a review from a team as a code owner February 19, 2026 17:34
Copy link
Contributor

Copilot AI left a comment

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 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.skip to QUnit.module
  • Added legacyForm: true configuration 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 timeZoneUtils import
  • 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');

@aleksei-semikozov aleksei-semikozov self-assigned this Feb 19, 2026
Copilot AI review requested due to automatic review settings February 19, 2026 22:49
Copy link
Contributor

Copilot AI left a comment

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 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.getTimezoneOffset in 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 stub timeZoneUtils.getClientTimezoneOffset (and/or other timeZoneUtils helpers) instead of modifying Date.prototype directly (e.g. testing/tests/DevExpress.ui.widgets.scheduler/timezones.tests.js stubs getClientTimezoneOffset). Consider replacing the Date.prototype stub with a timeZoneUtils.getClientTimezoneOffset stub (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');

@aleksei-semikozov aleksei-semikozov marked this pull request as draft February 19, 2026 23:50
@aleksei-semikozov aleksei-semikozov force-pushed the fix/3261-scheduler-unstable-tests-dst_26_1 branch from ad65879 to b31995a Compare February 20, 2026 13:34
@aleksei-semikozov aleksei-semikozov marked this pull request as ready for review February 20, 2026 13:37
Copilot AI review requested due to automatic review settings February 20, 2026 13:37
Copy link
Contributor

Copilot AI left a comment

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 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 global document.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., via POM.getTooltipAppointment()), restoring timers in a finally if 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);

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.

1 participant