Scheduler - Migrate popup state and editor tests to isolated environment#33670
Scheduler - Migrate popup state and editor tests to isolated environment#33670aleksei-semikozov wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates a subset of Scheduler AppointmentPopup state/editor behaviors out of the Scheduler integration suite into an isolated AppointmentPopup test environment, aiming to make these scenarios faster and less dependent on full Scheduler setup.
Changes:
- Added isolated tests for
allDayswitch behavior and start/end editor synchronization inappointment_popup.test.ts. - Removed corresponding integration tests from
appointment_popup.integration.test.tsas part of the migration. - Updated the isolated popup test factory to use
createTimeZoneCalculator(with configurabletimeZone) and refactored popup-show DOM querying.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts | Adds isolated AppointmentPopup tests for allDay toggling and start/end date-time editor interactions. |
| packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.integration.test.ts | Removes several integration tests that were migrated (and some that currently have no replacement). |
| packages/devextreme/js/__internal/scheduler/tests/mock/create_appointment_popup.ts | Refactors isolated popup creation: uses createTimeZoneCalculator, adds optional timeZone, and encapsulates show+DOM lookup. |
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.integration.test.ts:1518
- Several firstDayOfWeek regression checks were removed (explicit option application to weekday buttons and to the start/end/recurrence calendars, including changing firstDayOfWeek at runtime). Repo search shows no equivalent tests remain, so coverage is reduced to only the localization-default case below. Please re-add coverage for the explicit firstDayOfWeek option and dynamic option changes, since these scenarios differ from the localization fallback.
describe('firstDayOfWeek', () => {
beforeEach(() => {
jest.spyOn(dateLocalization, 'firstDayOfWeekIndex').mockReturnValue(3);
});
afterEach(() => {
jest.restoreAllMocks();
});
it('should pass value from localization firstDayOfWeek to calendars when option is not set', async () => {
const { POM, scheduler } = await createScheduler({
...getDefaultConfig(),
firstDayOfWeek: undefined,
});
scheduler.showAppointmentPopup(commonAppointment);
const startDateEditor = POM.popup.dxForm.getEditor('startDateEditor');
expect(startDateEditor).toBeDefined();
expect(startDateEditor?.option('calendarOptions.firstDayOfWeek')).toBe(3);
});
| jest.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('should apply firstDayOfWeek to week day buttons', async () => { |
There was a problem hiding this comment.
why tests for firstDayOfWeek were removed?
There was a problem hiding this comment.
Pure pass-through, no logic — form.ts assigns to calendarOptions.firstDayOfWeek:
L233 / L607 / L637. Propagation now covered by this isolated test.
There was a problem hiding this comment.
firstDayOfWeek is also used for weekDay buttons (link), please add a check for week day buttons too in the test that you added
There was a problem hiding this comment.
Added in 44aa4ec42e. Test now also asserts recurrenceWeekDayButtons.textContent === 'MTWTFSS' to cover the getRecurrenceWeekDayItems ordering path.
| }, | ||
| firstDayOfWeek: 1, | ||
| }); | ||
| POM.selectRepeatValue('weekly'); |
There was a problem hiding this comment.
This test only opens popup once with firstDayOfWeek: 1. Old tests also changed the option and opened popup again to check dynamic update. Is that case tested somewhere now?
There was a problem hiding this comment.
On option('firstDayOfWeek') change scheduler doesn't patch the popup — it fully recreates form+popup via createAppointmentPopupForm(). The new instance reads the value via the path this PR's test already covers — nothing popup-specific left to verify.
| it('should propagate firstDayOfWeek to editor calendars and recurrence week day buttons', async () => { | ||
| const { POM } = await createAppointmentPopup({ | ||
| appointmentData: { | ||
| text: 'common-app', | ||
| startDate: new Date(2017, 4, 9, 9, 30), | ||
| endDate: new Date(2017, 4, 9, 11), | ||
| }, | ||
| firstDayOfWeek: 1, | ||
| }); |
No description provided.