test(#11246): harden Bikram Sambat test coverage by megha1807 · Pull Request #11320 · medic/cht-core

megha1807 · 2026-08-04T12:04:34Z

Description

Harden and complete the test suite coverage for the Bikram Sambat calendar feature by addressing the remaining 7 test cases outlined in issue #11246.

Changes Made

Integration/E2E Tests
- A7 (Widget Cleanup): Verified that the standard date widget (.widget.date) is successfully destroyed when the Bikram Sambat widget is loaded.
- B6 (Viewport boundaries): Added integration test checking that the calendar popup is fully rendered inside the browser's viewport.
- B4 & B9 (Day Selection verification): Captured clicked day text and verified that:
  - Reopening the picker highlights the exact correct day.
  - The filter chip labels display the correct selected day.
- B11 (Clearing the filter): Simulates clicking the cancel (x) icon on the filter chip, verifying that it successfully resets the date filter inputs and restores the report list.
Unit Tests
- C6 (Genuine round-trip): Added a true round-trip test starting from a Bikram Sambat date, converting it to Gregorian via toGreg_text, and verifying it converts back to the original values using toBik.
- C5 (Timezone stability): Stubbed moment.fn.local to return a fixed offset (GMT-6) during the timezone conversion test to ensure offset shifts are tested deterministically across all environments, and cleaned up the redundant DST test case.

Verification Results

Verified all Karma unit tests pass successfully.
Verified ESLint checks pass with 0 errors.

Closes #11246

Code review checklist

UI/UX backwards compatible: Test it works for the new design (enabled by default). And test it works in the old design, enable can_view_old_navigation permission to see the old design. Test it has appropriate design for RTL languages.
Readable: Concise, well named, follows the style guide
Documented: Configuration and user documentation on cht-docs
Tested: Unit and/or e2e where appropriate
Internationalised: All user facing text
Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

sugat009

praise: good round. C5 is genuinely deterministic now, and deleting the DST case rather than trying to rescue it was the right call. A7 is a real guard, datepicker-extended is registered in webapp/src/js/enketo/widgets.js:7 and does create .widget.date, so removing the cleanup at bikram-sambat-datepicker.js:82 would fail that test. Returning the clicked cell's text from the page object is a neat way to make B4/B9 assert the actual day instead of "some day".

One blocking item inline, plus three smaller ones.

note: C6 no longer goes through the service (the rename reflects that), so it now sits in format-date.service.spec.ts as a pure bikram-sambat test. Defensible given how #11246 worded the item, just worth being a deliberate choice.

sugat009 · 2026-08-05T12:10:47Z

+      expect(gregStr).to.equal('2024-04-12');
+
+      // Convert back to BS using toBik and check it matches original values
+      const convertedBack = BikramSambat.toBik(moment(gregStr).toDate());


issue (blocking): this round-trip is timezone-fragile, the same class of bug the epic has been fixing. moment(gregStr).toDate() produces a local-midnight Date, which falls before the library's 12:30 UTC day boundary in far-eastern zones, so it decodes to the previous BS day. Measured: UTC and Asia/Kathmandu give 2080-12-30, but Pacific/Auckland and Pacific/Kiritimati both give 2080-12-29 and the test fails. The toGreg_text half above it is already stable because it never leaves string form.

sugat009 · 2026-08-05T12:10:47Z

+    // Verify both From and To input fields have selected date values and match selection (B9)
+    const fromDateLabel = await reportsPage.getFromDateValue();
+    const toDateLabel = await reportsPage.getToDateValue();
+    expect(fromDateLabel).to.include(fromDayText);


nitpick (non-blocking): fromDayText here is day 1, "१", and include matches substrings, so this also passes for "११", "२१" and "३१", i.e. it would stay green if the wrong day rendered. The To-side assertion below doesn't have this problem because it compares for equality.

sugat009 · 2026-08-05T12:10:47Z

    expect(await reportsPage.leftPanelSelectors.reportByUUID(visitDistrictHospital._id).isDisplayed()).to.be.true;

+    // 6. Clear Nepali date filter leg (B11)
+    const clearDateFilterChip = await $('#date-filter-accordion mat-expansion-panel-header .chip .fa-times');


issue (non-blocking): this reintroduces raw selectors into the spec (here and the .chip existence check below), which is the page-object convention you'd just finished cleaning up in #11257.

sugat009 · 2026-08-05T12:10:47Z

+    const picker = await $('.nepali-date-picker');
+    expect(await picker.isDisplayed()).to.be.true;
+
+    const viewportSize = await browser.getWindowSize();


nitpick (non-blocking): getWindowSize() is the outer browser window including chrome, while getLocation() is viewport-relative, so this bound is looser than intended and would still pass with the picker hanging below the visible area by roughly the toolbar height. window.innerWidth/innerHeight is the actual viewport.

…remove raw selectors, and adjust viewport logic

github-actions · 2026-08-06T05:03:51Z

All checks passed — thank you @megha1807! This PR is ready for review.

sugat009

Thanks @megha1807! The timezone fragility is sorted (round-trip and negative-offset tests are deterministic across zones now) and the assertions are tighter. LGTM.

Two tiny non-blocking notes for whenever: the round-trip test at format-date.service.spec.ts:519 now exercises the bikram-sambat library's inverse functions directly rather than going through service.date(), so it no longer covers the service's own year-boundary handling; and the clear-filter check at sidebar-filter.wdio-spec.js:268 uses .to.not.include on a single Devanagari digit, which can stay green even if the wrong day rendered. Neither blocks.

…overage and clear-filter assertions

sugat009

praise: the round-trip fix is better than what I asked for, C6 now asserts both directions and I verified it holds in UTC, Asia/Kathmandu, Pacific/Auckland, Pacific/Kiritimati and America/Anchorage. C5 is a genuine #11241 regression test with the right expected value, and deleting the DST case was correct since it could never have detected a DST bug. Selector extraction is clean, exporting sidebarFilterSelectors is purely additive across its 60+ importers, eslint is clean, and the PR is test-only.

One blocking item, plus one test that doesn't guard what it claims.

note on the checklist: #11246's B6 shouldn't be ticked off the back of this PR, see the inline comment on the viewport test. Everything else this PR set out to close (A7, B4, B9, B11, C5, C6) does hold up once B11 is corrected.

note: the approval on this PR predates the last two commits (approved 06:22, pushed 14:11), so the green tick is stale. Every other check is passing, including test-cht-form and Compile the app, and AndraBot is green here, so #11324 didn't bite this one.

sugat009 · 2026-08-07T09:34:07Z

+
+    // Verify labels are reset, the filter chip is gone, and the report list goes back to original length
+    expect(await reportsPage.getFromDateValue()).to.equal('बाट');
+    expect(await reportsPage.getToDateValue()).to.equal('सम्म');


issue (blocking): these two expected values are what CI is failing on, 4 attempts out of 4, with expected 'मिति' to equal 'बाट'. Both From and To are the same mm-date-filter component, and date-filter.component.html:10 renders {{ 'Any date' | translate }} whenever inputLabel is falsy, which after a clear it always is. messages-ne.properties:19 defines Any date = मिति, so both fields read मिति once cleared. बाट and सम्म exist in the Nepali bundle only inside two validation-error messages, never as field labels.

Worth knowing for the fix: once both expectations are the same string, these two lines can't tell you which field was cleared. The chip assertion on the next line is the one carrying the weight, since it's driven by the combined fromDateFilter + toDateFilter count.

sugat009 · 2026-08-07T09:34:07Z

+    }
+  });
+
+  it('asserts that the picker renders completely within the viewport', async () => {


issue (non-blocking, but B6 shouldn't be ticked): this can't fail for any realistic positioning regression. The enketo widget calls setupNepaliDatePicker without a position (bikram-sambat-datepicker.js:171), and showPickerContainer returns early when position !== 'anchored' (bikram-sambat-picker-shared.js:228), so handleReposition, the only code that keeps the picker inside the viewport, never runs in this harness. The picker is then centred purely by CSS (bikramsambat.less:130-135, position: fixed; top/left: 50%; translate(-50%,-50%)), and a centred element is inside the viewport by construction, so these four assertions reduce to "the picker is not larger than the window". Delete handleReposition entirely and this test stays green.

The behaviour #11246 asks for lives on the anchored path, which only the reports date filter uses (date-filter.component.ts:153), so covering it means exercising that picker, ideally after a scroll so the reposition branch actually runs.

nitpick: also worth noting getLocation() is page-relative while innerWidth/innerHeight are viewport dimensions, so the comparison only holds while the page doesn't scroll. isDisplayedInViewport() avoids mixing the two.

sugat009 · 2026-08-07T09:34:07Z

+    // Verify both From and To input fields have selected date values and match selection (B9)
+    const fromDateLabel = await reportsPage.getFromDateValue();
+    const toDateLabel = await reportsPage.getToDateValue();
+    expect(fromDateLabel.split(' ')[0]).to.equal(fromDayText);


nitpick (non-blocking): split(' ')[0] keeps the day and discards the month, so a regression that lands on the right day in the wrong month passes silently, which is precisely the #11252 family. The From leg is weaker again, since that helper always picks day १.

sugat009 · 2026-08-07T09:34:07Z

-    // Verify that the active selected date is highlighted correctly
+    // Verify that the active selected date is highlighted correctly and has the correct day text
    expect(await reportsPage.isNepaliDatePickerActiveCellDisplayed()).to.be.true;
+    expect(await reportsPage.getNepaliDatePickerActiveCellText()).to.equal(toDayText);


nitpick (non-blocking): the To helper picks the last enabled cell, which is today, and the picker already marks today active by default. So this assertion would still pass if the reopen-restore logic were removed; what actually makes it fail is an unrelated guard that strips .active when the input is empty. Reopening the From field instead would distinguish "restored the selection" from "defaulted to today".

sugat009 · 2026-08-07T09:34:07Z

-      expect(service.date(localDate)).to.equal(expectedText);
+      // Stub moment.fn.local to return a moment with a fixed -360 (GMT-6) offset
+      // to ensure a negative offset shift is exercised deterministically
+      const localStub = sinon.stub(moment.fn, 'local').callsFake(function(this: any) {


nitpick (non-blocking): stubbing moment.fn.local patches moment's shared prototype for what is only fixture construction; building the moment at the offset you want avoids touching global state. The test itself is sound and the expected value checks out.

test(medic#11246): harden Bikram Sambat test coverage

26cd7aa

sugat009 self-requested a review August 4, 2026 12:44

sugat009 requested changes Aug 5, 2026

View reviewed changes

test(medic#11246): resolve review feedback, fix timezone round-trip, …

b03bb98

…remove raw selectors, and adjust viewport logic

github-actions Bot added the Waiting for contributor label Aug 5, 2026

megha1807 requested a review from sugat009 August 5, 2026 15:02

sugat009 linked an issue Aug 6, 2026 that may be closed by this pull request

Bikram Sambat: harden feature test coverage (widget, reports filter, conversion, a11y/security) #11246

Open

33 tasks

github-actions Bot added Ready for review and removed Waiting for contributor labels Aug 6, 2026

sugat009 mentioned this pull request Aug 6, 2026

AndraBot: linked-issue check false-fails valid PRs on non-default branches (depends on GitHub's default-branch-only keyword linkage) #11324

Open

sugat009 approved these changes Aug 6, 2026

View reviewed changes

test(medic#11246): address non-blocking review items for round-trip c…

6a87bac

…overage and clear-filter assertions

sugat009 requested changes Aug 7, 2026

View reviewed changes

ken-talltree-io mentioned this pull request Aug 7, 2026

fix(#11324): recognise closing keywords on non-default-branch PRs #11332

Open

4 tasks

Uh oh!

Uh oh!

Conversation

megha1807 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes Made

Verification Results

Code review checklist

License

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

megha1807 commented Aug 4, 2026 •

edited

Loading