MOSIP-44758 : Automated Ekyc provider services screen testcases#1689
Open
rachanaspsoratur wants to merge 5 commits intomosip:developfrom
Open
MOSIP-44758 : Automated Ekyc provider services screen testcases#1689rachanaspsoratur wants to merge 5 commits intomosip:developfrom
rachanaspsoratur wants to merge 5 commits intomosip:developfrom
Conversation
WalkthroughAdds page locators and interaction/visibility methods for the eKYC Terms & Conditions screen, new Cucumber step definitions exercising the T&C flows (checkbox, cancel→warning popup, stay/discontinue, proceed), and a new feature scenario that drives the full T&C journey. (30 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Cucumber as Cucumber Runner
participant StepDefs as StepDefinitions
participant PageObj as EkycPage
participant Browser as Browser/DOM
Cucumber->>StepDefs: Run "Verify eKyc policy terms and conditions screen"
StepDefs->>PageObj: isTermsAndConditionHeaderDisplayed()
PageObj->>Browser: locate header element
Browser-->>PageObj: header visible
PageObj-->>StepDefs: true
StepDefs->>PageObj: isTermsAndConditionCheckboxNotSelected()
PageObj->>Browser: read checkbox.checked
Browser-->>PageObj: false
PageObj-->>StepDefs: unchecked
StepDefs->>PageObj: clickOnTermsAndConditionCheckBox()
PageObj->>Browser: click checkbox
Browser-->>PageObj: checked
StepDefs->>PageObj: isTermsProceedButtonEnabled()
PageObj->>Browser: read proceed.enabled
Browser-->>PageObj: true
PageObj-->>StepDefs: enabled
StepDefs->>PageObj: click proceed
PageObj->>Browser: navigate to next screen (Video Preview / Relying Party)
Browser-->>StepDefs: navigation complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
ui-test/src/main/java/pages/EkycPage.java (2)
119-120: Duplicatestay-buttonlocator declaration.
eKycTermsAndConditionsStayButtonuses the sameid = "stay-button"locator that's already defined at line 59 forekycWarningPopupStayButton. Consider reusing the existing element or consolidating to avoid maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/pages/EkycPage.java` around lines 119 - 120, The duplicate locator id "stay-button" is declared twice as eKycTermsAndConditionsStayButton and ekycWarningPopupStayButton; remove the redundant WebElement (eKycTermsAndConditionsStayButton) and update all usages to reference the existing ekycWarningPopupStayButton (or consolidate both names into a single, clearly named field in EkycPage), ensuring any methods referencing eKycTermsAndConditionsStayButton are changed to use the chosen field so the locator is maintained in one place.
107-108: Fragile XPath using exact class match.This XPath relies on an exact class string match (
scrollable-div tnc-content flex text-justify sm:py-0 sm:ps-0). If the UI framework adds/removes/reorders Tailwind classes, this locator will break. Consider using acontains(@Class, 'scrollable-div')approach or request a dedicatedidattribute from the UI team.♻️ Suggested approach
-@FindBy(xpath = "//div[`@class`='scrollable-div tnc-content flex text-justify sm:py-0 sm:ps-0']") +@FindBy(xpath = "//div[contains(`@class`,'scrollable-div') and contains(`@class`,'tnc-content')]") WebElement ekycTermsAndConditionsContentScrollBar;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/pages/EkycPage.java` around lines 107 - 108, The field ekycTermsAndConditionsContentScrollBar uses a fragile XPath that matches the entire class attribute; change the locator to be resilient by using a partial class match (e.g., contains(`@class`,'scrollable-div') or contains(`@class`,'tnc-content')) or switch to a stable CSS selector that targets the same element; alternatively, ask the UI team to add a dedicated id and update the `@FindBy` on ekycTermsAndConditionsContentScrollBar to use that id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/pages/EkycPage.java`:
- Around line 347-349: The method isEkyTermsAndConditionScreenVisible contains a
typo ("Eky"→"Ekyc") and duplicates the existing
isEkycTermsAndConditionsScreenVisible method; remove this duplicate method and
update any call sites to use isEkycTermsAndConditionsScreenVisible instead,
making sure they reference the same element identifier
ekycTermsAndConditionsHeader so visibility checks remain unchanged.
- Around line 304-308: Rename the misnamed method
isTermaAndConditionContentScrollBarEnabled to
isTermsAndConditionContentScrollBarVisible (or similar) and change its
implementation to use a visibility/scroll check instead of isButtonEnabled;
specifically, replace the call to
isButtonEnabled(ekycTermsAndConditionsContentScrollBar, ...) with either
isElementVisible(ekycTermsAndConditionsContentScrollBar, "message") or a DOM/CSS
check (e.g., compare scrollHeight vs clientHeight or inspect overflow CSS) to
detect a visible scrollbar, and update any call sites to the new method name.
In `@ui-test/src/main/java/stepdefinitions/EkycStepDefinition.java`:
- Around line 350-354: The assertion in userVerifyProceedButtonIsEnabled uses
Assert.assertTrue(ekycPage.isTermsProceedButtonEnabled()) but the failure
message is inverted; update the message to reflect the actual expectation (e.g.,
"Proceed button is not enabled after selecting the checkbox in terms and
conditions screen") so that Assert.assertTrue and
ekycPage.isTermsProceedButtonEnabled() produce a correct, non-contradictory
failure message.
- Around line 126-130: The test currently asserts
ekycPage.isEkycProcessStepsScreenLabelDisplayed() in
userVerifyWarningPopupDisappeared(), which does not confirm the warning popup
was dismissed; update the step to assert the warning popup element is not
visible instead (e.g., use or add a method on ekycPage such as
isWarningPopupDisplayed() or isWarningPopupNotDisplayed()) and assert that it
returns false (or true for "not displayed") with a clear failure message like
"Warning popup is still displayed after clicking stay button"; leave the
TNC/tnc-header check out of this step or keep it in a separate verification step
if needed.
- Around line 283-287: Rename the test method
userVerifyTermsAndCoditionContentScrollBarIsDisabled to
userVerifyTermsAndConditionContentScrollBarIsEnabled (fixing "Codition" ->
"Condition" and reflecting the actual expectation), and ensure the assertion
message and referenced helper are consistent: keep using
ekycPage.isTermaAndConditionContentScrollBarEnabled (or rename that helper to
isTermsAndConditionContentScrollBarEnabled if you also want to fix "Terma") and
change the Assert.assertTrue message to state that the scrollbar is enabled
rather than enabled/disabled mismatch.
---
Nitpick comments:
In `@ui-test/src/main/java/pages/EkycPage.java`:
- Around line 119-120: The duplicate locator id "stay-button" is declared twice
as eKycTermsAndConditionsStayButton and ekycWarningPopupStayButton; remove the
redundant WebElement (eKycTermsAndConditionsStayButton) and update all usages to
reference the existing ekycWarningPopupStayButton (or consolidate both names
into a single, clearly named field in EkycPage), ensuring any methods
referencing eKycTermsAndConditionsStayButton are changed to use the chosen field
so the locator is maintained in one place.
- Around line 107-108: The field ekycTermsAndConditionsContentScrollBar uses a
fragile XPath that matches the entire class attribute; change the locator to be
resilient by using a partial class match (e.g.,
contains(`@class`,'scrollable-div') or contains(`@class`,'tnc-content')) or switch
to a stable CSS selector that targets the same element; alternatively, ask the
UI team to add a dedicated id and update the `@FindBy` on
ekycTermsAndConditionsContentScrollBar to use that id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d6f96ff6-2309-4605-aace-d54f80947bf9
📒 Files selected for processing (4)
ui-test/src/main/java/pages/EkycPage.javaui-test/src/main/java/stepdefinitions/EkycStepDefinition.javaui-test/src/main/resources/featurefiles/ConsentPage.featureui-test/src/main/resources/featurefiles/EkycPage.feature
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ui-test/src/main/java/pages/EkycPage.java (1)
347-349:⚠️ Potential issue | 🟠 MajorMake the terms-screen visibility helper unique to the terms screen and drop the duplicate wrapper.
This reintroduces a second helper for the same check, and both wrappers are keyed off
tnc-header, which is already reused byekycProcessStepsScreenLabelat Lines 17-18. That means the redirect assertions can go green while the test is still on the process-steps screen. Please switch the shared terms-screen helper to a terms-only element liketnc-sub-header/tnc-content, then update the call site to reuse that single API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/pages/EkycPage.java` around lines 347 - 349, The isEkycTermsAndConditionScreenVisible wrapper duplicates an existing check keyed off the shared locator ekycTermsAndConditionsHeader (which collides with ekycProcessStepsScreenLabel), so replace the duplicate with a singular terms-only locator (e.g., tncSubHeader or tncContent) and update the helper accordingly: add a unique element field (tncSubHeader/tncContent), change isEkycTermsAndConditionScreenVisible to call isElementVisible(tncSubHeader or tncContent, "...terms screen is visible"), remove the old duplicate wrapper, and update all call sites to use this single API (referencing ekycTermsAndConditionsHeader, ekycProcessStepsScreenLabel, and isEkycTermsAndConditionScreenVisible to locate where to change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/stepdefinitions/EkycStepDefinition.java`:
- Around line 295-303: The two step definitions both call
ekycPage.clickOnTermsAndConditionCheckBox(), causing a double toggle; change the
"user verify terms and conditions checkbox is clickable in terms and conditions
screen" step (method userClicksOnCheckBoxInTermsAndConditonScreen) to perform a
non-mutating clickable assertion instead of clicking — e.g., call an existing
checker like ekycPage.isTermsAndConditionCheckBoxClickable() or implement and
call ekycPage.verifyTermsAndConditionCheckBoxClickable() (which should assert or
return true) so only the explicit "user click on checkbox in terms and
conditions screen" step performs the actual click.
---
Duplicate comments:
In `@ui-test/src/main/java/pages/EkycPage.java`:
- Around line 347-349: The isEkycTermsAndConditionScreenVisible wrapper
duplicates an existing check keyed off the shared locator
ekycTermsAndConditionsHeader (which collides with ekycProcessStepsScreenLabel),
so replace the duplicate with a singular terms-only locator (e.g., tncSubHeader
or tncContent) and update the helper accordingly: add a unique element field
(tncSubHeader/tncContent), change isEkycTermsAndConditionScreenVisible to call
isElementVisible(tncSubHeader or tncContent, "...terms screen is visible"),
remove the old duplicate wrapper, and update all call sites to use this single
API (referencing ekycTermsAndConditionsHeader, ekycProcessStepsScreenLabel, and
isEkycTermsAndConditionScreenVisible to locate where to change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 78103e71-e3bf-452f-9f59-a14dcd9109af
📒 Files selected for processing (2)
ui-test/src/main/java/pages/EkycPage.javaui-test/src/main/java/stepdefinitions/EkycStepDefinition.java
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui-test/src/main/java/stepdefinitions/EkycStepDefinition.java (1)
283-286:⚠️ Potential issue | 🟡 MinorFix the inverted scrollbar failure message.
Line 285 currently reports the success state on failure (
"scroll bar is visible"), which makes test failures misleading. The message should describe the actual failing condition instead.🛠️ Suggested fix
Assert.assertTrue(ekycPage.isTermsAndConditionContentScrollBarVisible(), - "Terms and conditions content body text frame scroll bar is visible"); + "Terms and conditions content body text frame scroll bar is not visible");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/stepdefinitions/EkycStepDefinition.java` around lines 283 - 286, The assertion message in userVerifyTermsAndConditionContentScrollBarIsEnabled() is inverted: when Assert.assertTrue(ekycPage.isTermsAndConditionContentScrollBarVisible(), ...) fails it currently logs "Terms and conditions content body text frame scroll bar is visible" which is misleading; update the message to describe the failing condition (e.g., "Terms and conditions content body text frame scroll bar is not visible") so it accurately reflects the failure of ekycPage.isTermsAndConditionContentScrollBarVisible().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ui-test/src/main/java/stepdefinitions/EkycStepDefinition.java`:
- Around line 283-286: The assertion message in
userVerifyTermsAndConditionContentScrollBarIsEnabled() is inverted: when
Assert.assertTrue(ekycPage.isTermsAndConditionContentScrollBarVisible(), ...)
fails it currently logs "Terms and conditions content body text frame scroll bar
is visible" which is misleading; update the message to describe the failing
condition (e.g., "Terms and conditions content body text frame scroll bar is not
visible") so it accurately reflects the failure of
ekycPage.isTermsAndConditionContentScrollBarVisible().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a09cc90a-9c33-4105-9d80-65f8ba9e1bb2
📒 Files selected for processing (2)
ui-test/src/main/java/stepdefinitions/EkycStepDefinition.javaui-test/src/main/resources/featurefiles/EkycPage.feature
Signed-off-by: Rachana S P <rachana.p@cyberpwn.com>
Signed-off-by: Rachana S P <rachana.p@cyberpwn.com>
Signed-off-by: Rachana S P <rachana.p@cyberpwn.com>
Signed-off-by: Rachana S P <rachana.p@cyberpwn.com>
Signed-off-by: Rachana S P <rachana.p@cyberpwn.com>
rachanaspsoratur
force-pushed
the
develop
branch
from
0ec6382 to
803bcb5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
ui-test/src/main/resources/featurefiles/EkycPage.feature (3)
97-99: Keep the list-screen scenario scoped to the list page.Line 99 now makes
Scenario: Verify list of eKYC service provider screenfail on a downstream terms-and-conditions regression even when the provider-list UI is fine, and the dedicated@TermsAndConditionsScreenscenario already covers that hop. Consider ending this scenario at the list-page assertions and leaving the redirect check to the terms-and-conditions scenario.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/resources/featurefiles/EkycPage.feature` around lines 97 - 99, The scenario "Verify list of eKYC service provider screen" in the feature file is over-scoped by asserting a redirect to the terms-and-conditions screen; remove the final step "Then user verify user is redirected to terms and conditions screen" from that scenario so it only contains list-page assertions (the clickable provider names and proceed button) and leave the redirect verification to the existing `@TermsAndConditionsScreen` scenario; update any scenario title or tags if needed to reflect that this scenario now only validates the provider list page.
101-153: Split this smoke flow into smaller scenarios.This single
@smokescenario exercises thestay,discontinue, andproceedbranches and repeats the full login/navigation path twice. That makes failures harder to localize and increases smoke-suite runtime. ABackgroundplus one scenario per branch would be easier to debug and maintain.Possible Gherkin split
-@smoke `@TermsAndConditionsScreen` -Scenario: Verify eKyc policy terms and conditions screen - ... +Background: + When click on Language selection option + And select the mandatory language + ... + Then user verify user is redirected to terms and conditions screen + +@smoke `@TermsAndConditionsStay` +Scenario: Verify stay action in the terms and conditions warning popup + ... + +@smoke `@TermsAndConditionsDiscontinue` +Scenario: Verify discontinue action in the terms and conditions warning popup + ... + +@smoke `@TermsAndConditionsProceed` +Scenario: Verify proceed from the terms and conditions screen + ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/resources/featurefiles/EkycPage.feature` around lines 101 - 153, The big `@smoke` Scenario bundles three branches and duplicates the whole login/navigation flow; refactor by extracting the common preconditions into a Background (language selection, login-with-OTP flow, consent/attention/proceed, navigation to list of eKYC providers and into the terms-and-conditions page) and replace the large Scenario with three focused Scenarios (e.g., "Verify eKYC terms - stay branch", "Verify eKYC terms - discontinue branch", "Verify eKYC terms - proceed branch") that each cover only the checkbox/cancel/proceed interactions and assertions; keep the `@smoke/`@TermsAndConditionsScreen tags as needed and remove the duplicated login/navigation steps from the individual Scenarios so failures are isolated and runtime is reduced.
117-152: Normalizeterms and condition(s)terminology in step definitions to improve consistency.All feature file steps are properly bound to step definitions—no undefined or duplicate bindings exist. However, the step definitions themselves use inconsistent terminology:
- Most steps (18 annotations) use "terms and conditions screen" (plural)
- Lines 333–339 in EkycStepDefinition use "terms and condition screen" (singular)
- Lines 156, 161 in ConsentStepDefinition use "terms and condition page" (singular)
Standardize the terminology across all step definitions to a single phrase (recommend "terms and conditions") to improve readability and consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/resources/featurefiles/EkycPage.feature` around lines 117 - 152, The step wording is inconsistent—some step definitions use "terms and condition" (singular) while most use "terms and conditions" (plural); search for and update all Cucumber step annotations and any matching method names in EkycStepDefinition and ConsentStepDefinition that contain "terms and condition" or "terms and condition page" to the canonical phrase "terms and conditions" (e.g., update annotation regex/strings and corresponding method identifiers so they match the feature file's "terms and conditions" phrasing everywhere), then run the step-matching tests to ensure no bindings break.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui-test/src/main/resources/featurefiles/EkycPage.feature`:
- Around line 97-99: The scenario "Verify list of eKYC service provider screen"
in the feature file is over-scoped by asserting a redirect to the
terms-and-conditions screen; remove the final step "Then user verify user is
redirected to terms and conditions screen" from that scenario so it only
contains list-page assertions (the clickable provider names and proceed button)
and leave the redirect verification to the existing `@TermsAndConditionsScreen`
scenario; update any scenario title or tags if needed to reflect that this
scenario now only validates the provider list page.
- Around line 101-153: The big `@smoke` Scenario bundles three branches and
duplicates the whole login/navigation flow; refactor by extracting the common
preconditions into a Background (language selection, login-with-OTP flow,
consent/attention/proceed, navigation to list of eKYC providers and into the
terms-and-conditions page) and replace the large Scenario with three focused
Scenarios (e.g., "Verify eKYC terms - stay branch", "Verify eKYC terms -
discontinue branch", "Verify eKYC terms - proceed branch") that each cover only
the checkbox/cancel/proceed interactions and assertions; keep the
`@smoke/`@TermsAndConditionsScreen tags as needed and remove the duplicated
login/navigation steps from the individual Scenarios so failures are isolated
and runtime is reduced.
- Around line 117-152: The step wording is inconsistent—some step definitions
use "terms and condition" (singular) while most use "terms and conditions"
(plural); search for and update all Cucumber step annotations and any matching
method names in EkycStepDefinition and ConsentStepDefinition that contain "terms
and condition" or "terms and condition page" to the canonical phrase "terms and
conditions" (e.g., update annotation regex/strings and corresponding method
identifiers so they match the feature file's "terms and conditions" phrasing
everywhere), then run the step-matching tests to ensure no bindings break.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aff35cec-9f6f-4387-bddb-7122e35bc90b
📒 Files selected for processing (4)
ui-test/src/main/java/pages/EkycPage.javaui-test/src/main/java/stepdefinitions/EkycStepDefinition.javaui-test/src/main/resources/featurefiles/ConsentPage.featureui-test/src/main/resources/featurefiles/EkycPage.feature
✅ Files skipped from review due to trivial changes (2)
- ui-test/src/main/resources/featurefiles/ConsentPage.feature
- ui-test/src/main/java/stepdefinitions/EkycStepDefinition.java
🚧 Files skipped from review as they are similar to previous changes (1)
- ui-test/src/main/java/pages/EkycPage.java
BhuvanaShreeBS
approved these changes
Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated Ekyc provider services screen testcases
Summary by CodeRabbit