test(#11246): harden Reports Date Filter Sidebar tests (Bikram Sambat)#11257
Conversation
f1a896c to
41a8ea2
Compare
…sambat-support-in-cht' into fix/11246-reports-date-filter
sugat009
left a comment
There was a problem hiding this comment.
Thanks @megha1807! The two headline fixes are solid: the from-date now asserts the full 00:00:00.000 timestamp and maxDate checks the exact value, that closes what #11246 raised. Karma green.
Requesting a couple of test improvements before merge, both from CHT's test conventions:
- Test level: the maxDate-boundary, reopen-highlight, and scroll checks sit at the e2e layer, where they can only assert "something appeared." CHT's pyramid says push these down to unit / cht-form, where you can assert the exact value.
- Page objects: CHT's e2e style guide keeps element selectors and the actions on them in the page object, with only the
expects in the spec. These new checks reach into the DOM directly. Direction inline.
| }, { timeout: 5000, timeoutMsg: 'Nepali date picker not displayed' }); | ||
|
|
||
| // Future dates in the current month should be disabled (.disable) | ||
| const disableCells = await picker.$$('table tbody td.current-month-date.disable'); |
There was a problem hiding this comment.
suggestion: these locate elements inline in the spec (picker.$$('table tbody td.current-month-date.disable'), .active, $$('.nepali-date-picker')) using the plugin's structural/visual classes. Per CHT's e2e style guide, the selectors and the actions on them belong in the reports page object (reports.wdio.page.js), leaving only the expects here, and stable test-ids are preferred over structural CSS classes.
| const initialLocation = await picker.getLocation(); | ||
|
|
||
| // Simulate scroll event | ||
| await browser.execute('window.dispatchEvent(new Event("scroll"))'); |
There was a problem hiding this comment.
issue: this fires a scroll event without actually scrolling the page, so the picker never has a reason to move and the assertion passes regardless of the anchoring logic. To exercise re-anchoring, the page has to genuinely scroll, or better, test the reposition logic directly at the unit/cht-form level.
| // 5. Reopen Off-by-one verification | ||
| await $('#toDateFilter').click(); | ||
| // Verify that the active selected date is highlighted correctly | ||
| const activeCell = await picker.$('table tbody td.current-month-date.active'); |
There was a problem hiding this comment.
issue: this asserts only that an .active cell is displayed, not that the highlighted cell is the selected date, so a wrong (off-by-one) highlight still passes, which is exactly what this test is meant to catch. It should assert the highlighted cell is the selected date. That exact-value check is far easier as a unit / cht-form test than at e2e.
There was a problem hiding this comment.
Agreed! I have pushed this down to the unit test level as suggested.
I added a new unit test in bikram-sambat-datepicker.spec.ts (highlights the correct active day corresponding to the input value when reopened) that asserts the exact day text of the .active cell matches the input value when reopened, checking for any off-by-one errors.
The E2E test now simply acts as a high-level integration check utilizing the updated Page Object selectors.
| expect($('.nepali-date-picker-overlay')).to.have.lengthOf(0); | ||
| }); | ||
|
|
||
| it('should maintain independent picker elements for From and To components', () => { |
There was a problem hiding this comment.
suggestion: this confirms each component rendered its own hidden input, but not that the two pickers are actually independent. Add an assertion that interacting with one (e.g. selecting a date) leaves the other unchanged.
sugat009
left a comment
There was a problem hiding this comment.
Re-reviewed after the latest push, good progress: the picker query selectors are now proper page-object methods and the independence test reads well. Two notes inline, one blocking (the ngOnDestroy "multiple pickers" test).
| await reportsPage.openSidebarFilterDateAccordion(); | ||
|
|
||
| // 1. Max Date parity - verify future dates are disabled | ||
| await $('#fromDateFilter').click(); |
There was a problem hiding this comment.
suggestion (non-blocking): Nice work moving the picker query selectors into the page object. These interaction selectors are still inline though, $('#fromDateFilter') here and at :215, $('#toDateFilter') at :226, $('#date-filter-accordion mat-expansion-panel-header') at :217, and each already exists in reports.wdio.page.js (sidebarFilterSelectors.fromDate() :69, .toDate() :68, .dateAccordionHeader() :66). Since sidebarFilterSelectors is private (not exported), add small named action helpers in the page object (e.g. openBikFromDatePicker() clicking sidebarFilterSelectors.fromDate()) and call those, so no raw selector strings live in the spec and there's one place to update if the DOM changes.
| setFilter.restore(); | ||
| }); | ||
|
|
||
| it('ngOnDestroy should clean up multiple pickers and overlays completely', () => { |
There was a problem hiding this comment.
issue (blocking): Titled "clean up multiple pickers and overlays completely," but it destroys a single component and asserts only that one hidden input is gone (#test-wrapper-1 …, :402), then hand-removes the pickers and overlay itself (:405-408). Because it deletes the very elements it should verify, it can't fail even if ngOnDestroy left them behind. It's also a weaker duplicate of the existing ngOnDestroy should clean up specific picker container and overlay test (:285), which already asserts input + picker + overlay are all removed. Either drop it (the existing test covers single-component cleanup), or make it earn its name: destroy component A → assert A's input+picker gone while B's survive and the overlay remains, then destroy B → assert B's gone and the overlay is now removed. Don't hand-delete the elements under test before asserting.
c4f648e to
808ecf1
Compare
sugat009
left a comment
There was a problem hiding this comment.
One non-blocking convention note inline.
| await reportsPage.openSidebarFilterDateAccordion(); | ||
|
|
||
| // 1. Max Date parity - verify future dates are disabled | ||
| await $('#fromDateFilter').click(); |
There was a problem hiding this comment.
issue (non-blocking): These sidebar clicks are written as raw selectors here (:202, :215, :217, :226), but the same ones already exist in the page object, tests/page-objects/default/reports/reports.wdio.page.js: fromDate() (:69), toDate() (:68), dateAccordionHeader() (:66). CHT's e2e convention keeps selectors out of specs. Since sidebarFilterSelectors is private, the tidy fix is a small action helper in the page object (e.g. an "open From picker" click) that the spec calls. Non-blocking, and you already did this correctly for the picker query selectors, this is just finishing the same pattern for the click targets.
813b4e1 to
fcbde38
Compare
fcbde38 to
1198851
Compare
…t coverage and extract page object selectors
1198851 to
8c0af42
Compare
Description
Hardens test coverage for the Reports Date Filter Sidebar under Section B of issue #11246.
Changes:
maxDatepassed tosetupNepaliDatePickerrepresents today's date in Devanagari format (restricting future date selection).clear()resets the Nepali date filter state.ngOnDestroycompletely cleans up multiple picker instances and body-appended overlays.This is PR 2 of 2. PR 1 (#11255) addressed the Enketo Form Widget and Date Conversions (Sections A, C, D).
Issue Number
Fixes #11246
Code review checklist
can_view_old_navigationpermission to see the old design. Test it has appropriate design for RTL languages.