Skip to content

test(#11246): harden Reports Date Filter Sidebar tests (Bikram Sambat) #11257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
megha1807 wants to merge 4 commits into
base: 10707-dmp-2026-enhance-bikram-sambat-support-in-cht
Choose a base branch
from
Open

test(#11246): harden Reports Date Filter Sidebar tests (Bikram Sambat) #11257

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41a8ea2
test(11246): harden Reports Date Filter Sidebar tests (Bikram Sambat)
megha1807 Jul 17, 2026
86832b5
Merge remote-tracking branch 'upstream/10707-dmp-2026-enhance-bikram-…
megha1807 Jul 17, 2026
8c0af42
test(11246): address reviewer feedback on reports date filter E2E/uni…
megha1807 Jul 21, 2026
429175a
trigger ci
megha1807 Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions tests/e2e/default/reports/sidebar-filter.wdio-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,57 @@ describe('Reports Sidebar Filter', () => {

await reportsPage.openSidebarFilter();
await reportsPage.openSidebarFilterDateAccordion();

// 1. Max Date parity - verify future dates are disabled
await $('#fromDateFilter').click();

@sugat009 sugat009 Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sugat009 sugat009 Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

let picker;
await browser.waitUntil(async () => {
const pickers = await $$('.nepali-date-picker');
for (const p of pickers) {
if (await p.isDisplayed()) {
picker = p;
return true;
}
}
return false;
}, { 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');

@sugat009 sugat009 Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

expect(disableCells.length).to.be.greaterThan(0);

// 2. Escape Dismissal - press Escape and verify picker is dismissed
await browser.keys(['Escape']);
expect(await picker.isDisplayed()).to.be.false;

// 3. Viewport positioning & scroll alignment
await $('#fromDateFilter').click();
expect(await picker.isDisplayed()).to.be.true;
const initialLocation = await picker.getLocation();

// Simulate scroll event
await browser.execute('window.dispatchEvent(new Event("scroll"))');

@sugat009 sugat009 Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const newLocation = await picker.getLocation();
expect(newLocation.y).to.equal(initialLocation.y); // Should remain anchored correctly

// Dismiss by clicking outside (e.g. the accordion header)
const accordionHeader = await $('#date-filter-accordion mat-expansion-panel-header');
await accordionHeader.click();
expect(await picker.isDisplayed()).to.be.false;

// 4. Select From and To dates
await reportsPage.setSidebarFilterBikFromDate();
await reportsPage.setSidebarFilterBikToDate();

// 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');

@sugat009 sugat009 Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@megha1807 megha1807 Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(await activeCell.isDisplayed()).to.be.true;

// Dismiss it
await browser.keys(['Escape']);

await commonPage.waitForPageLoaded();

expect(await reportsPage.leftPanelSelectors.allReports().length).to.equal(2);
Expand Down
84 changes: 82 additions & 2 deletions webapp/tests/karma/ts/components/filters/date-filter.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Selectors } from '@mm-selectors/index';
import { ResponsiveService } from '@mm-services/responsive.service';
import { LanguageService } from '@mm-services/language.service';
import { FormatDateService } from '@mm-services/format-date.service';
import { toBik_dev } from 'bikram-sambat';

describe('Date Filter Component', () => {
let component: DateFilterComponent;
Expand Down Expand Up @@ -195,7 +196,8 @@ describe('Date Filter Component', () => {
expect(nepaliDatePickerStub.callCount).to.equal(1);

const maxDateVal = nepaliDatePickerStub.args[0][0].maxDate;
expect(maxDateVal).to.match(/^[०-९-]+$/);
const expectedMaxDate = toBik_dev(moment().clone().locale('en').format('YYYY-MM-DD'));
expect(maxDateVal).to.equal(expectedMaxDate);

const hiddenInput = $('#bikram-sambat-test-wrapper .nepali-datepicker-input');
expect(hiddenInput).to.have.lengthOf(1);
Expand Down Expand Up @@ -254,7 +256,7 @@ describe('Date Filter Component', () => {

expect(setFilter.callCount).to.equal(1);
const appliedDate = setFilter.args[0][0].date.from;
expect(moment(appliedDate).format('YYYY-MM-DD')).to.equal('2024-07-24');
expect(moment(appliedDate).format('YYYY-MM-DD HH:mm:ss.SSS')).to.equal('2024-07-24 00:00:00.000');
setFilter.restore();
});

Expand Down Expand Up @@ -300,6 +302,84 @@ describe('Date Filter Component', () => {
expect($('.nepali-date-picker-overlay')).to.have.lengthOf(0);
});

it('should maintain independent picker elements for From and To components', () => {

@sugat009 sugat009 Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Create From component
const fixtureFrom = TestBed.createComponent(DateFilterComponent);
const compFrom = fixtureFrom.componentInstance;
compFrom.isStartDate = true;
compFrom.fieldId = 'from-test-field';

const htmlFrom = `<div id="from-test-wrapper"><input id="from-test-field" /></div>`;
document.body.insertAdjacentHTML('afterbegin', htmlFrom);
fixtureFrom.detectChanges();

// Create To component
const fixtureTo = TestBed.createComponent(DateFilterComponent);
const compTo = fixtureTo.componentInstance;
compTo.isStartDate = false;
compTo.fieldId = 'to-test-field';

const htmlTo = `<div id="to-test-wrapper"><input id="to-test-field" /></div>`;
document.body.insertAdjacentHTML('afterbegin', htmlTo);
fixtureTo.detectChanges();

// Verify each appended its own hidden input
expect($('#from-test-wrapper .nepali-datepicker-input')).to.have.lengthOf(1);
expect($('#to-test-wrapper .nepali-datepicker-input')).to.have.lengthOf(1);

// Clean up DOM
compFrom.ngOnDestroy();
compTo.ngOnDestroy();
$('#from-test-wrapper').remove();
$('#to-test-wrapper').remove();
});

it('clearing/resetting the date filter should set the date to undefined', () => {
component.ngAfterViewInit();
const setFilter = sinon.stub(GlobalActions.prototype, 'setFilter');
component.dateRange = { from: moment('2024-07-24').valueOf(), to: undefined };

component.clear();

expect(setFilter.callCount).to.equal(1);
expect(setFilter.args[0]).to.deep.equal([{ date: undefined }]);
setFilter.restore();
});

it('ngOnDestroy should clean up multiple pickers and overlays completely', () => {

@sugat009 sugat009 Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Simulate two components having been initialized
const html1 = `<div id="test-wrapper-1"><input id="field-1" />` +
`<input type="text" class="nepali-datepicker-input" /></div>`;
const html2 = `<div id="test-wrapper-2"><input id="field-2" />` +
`<input type="text" class="nepali-datepicker-input" /></div>`;
document.body.insertAdjacentHTML('afterbegin', html1);
document.body.insertAdjacentHTML('afterbegin', html2);

// Create pickers and overlays
$('<div class="nepali-date-picker"></div>').appendTo('body');
$('<div class="nepali-date-picker"></div>').appendTo('body');
$('<div class="nepali-date-picker-overlay"></div>').appendTo('body');

// Bind picker data to the hidden inputs
const hiddenInput1 = $('#test-wrapper-1 .nepali-datepicker-input');
const hiddenInput2 = $('#test-wrapper-2 .nepali-datepicker-input');
hiddenInput1.data('picker', $('.nepali-date-picker').eq(0));
hiddenInput2.data('picker', $('.nepali-date-picker').eq(1));

// Instantiate a component and call ngOnDestroy
component.fieldId = 'field-1';
component.ngOnDestroy();

// Verify that at least the first component's elements are removed
expect($('#test-wrapper-1 .nepali-datepicker-input')).to.have.lengthOf(0);

// Clean up others manually
$('#test-wrapper-1').remove();
$('#test-wrapper-2').remove();
$('.nepali-date-picker').remove();
$('.nepali-date-picker-overlay').remove();
});

it('setLabel should use formatDateService.dayMonth', () => {
component.isStartDate = true;
component.setLabel({ from: moment('2024-07-24').valueOf(), to: undefined });
Expand Down
Loading