RCF-1349: Automation Task for Phone Screen#1087
Merged
Merged
Conversation
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
…tomation. Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
…tomation. Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
…tomation. Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
WalkthroughThis PR adds a comprehensive no-network registration test, expands test infrastructure for "different center" user scenarios, refactors page object navigation/gesture logic for better UI reliability, and extends Settings UI validation to cover Master Data Sync and global configuration features in English and Arabic locales. ChangesEnd-to-End No-Network Registration Test
Test Infrastructure & User Management for Different-Center Scenario
Page Object Refactoring: Scrolling, Gestures, and Navigation
Settings Page Contract Expansion and Multi-Locale Implementation
Biometric Page Scan Title Visibility Updates
Document Upload and Demographic Details UI Workflow Updates
Manage Applications and UpdateMyUin Test Refactoring
Test Class Formatting and Whitespace Cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
ui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.java (1)
200-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
isExceptionScanTitleDisplayed()omits thescrollToTop()that its siblings now use.The right/left/thumbs/face checks were updated to scroll to top first, but the exception variant was left as-is. If this method is exercised in tests, it may behave inconsistently (stale viewport) compared to the others. If exception scan is intentionally untested here, ignore.
Proposed alignment
public boolean isExceptionScanTitleDisplayed() { + scrollToTop(); return isElementDisplayed(exceptionScanTitle); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.java` around lines 200 - 202, The isExceptionScanTitleDisplayed() method currently returns isElementDisplayed(exceptionScanTitle) but omits the scrollToTop() call used by the sibling checks; update isExceptionScanTitleDisplayed() in SupervisorBiometricVerificationpageEnglish to call scrollToTop() before checking isElementDisplayed(exceptionScanTitle) so its viewport behavior matches the other thumb/face methods.ui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.java (1)
207-209:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame
scrollToTop()omission asSupervisorBiometricVerificationpageEnglish.
isExceptionScanTitleDisplayed()was not aligned with the other four scan-title checks. AddscrollToTop()here too if exception scan is verified in tests; otherwise this is fine to leave.Proposed alignment
public boolean isExceptionScanTitleDisplayed() { + scrollToTop(); return isElementDisplayed(exceptionScanTitle); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.java` around lines 207 - 209, The isExceptionScanTitleDisplayed() method omits the scrollToTop() call used by the other scan-title checks; update the method (isExceptionScanTitleDisplayed) to call scrollToTop() before returning isElementDisplayed(exceptionScanTitle) so the exception scan title is always scrolled into view prior to verification, matching the pattern used in SupervisorBiometricVerificationpageEnglish and the other four scan-title checks.ui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.java (1)
379-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the second preview assertion on the Update UIN title.
The first preview check correctly uses
updateUINTitleDisplayed(), but after navigating back through the edit flow this switches toisNewRegistrationTitleDisplayed(). That turns the revisit path into a false failure even though the test is still in the Update My UIN journey.Proposed fix
- assertTrue(previewPage.isNewRegistrationTitleDisplayed(), "Verify if new Registration title is displayed"); + assertTrue(previewPage.updateUINTitleDisplayed(), "Verify if update UIN title is displayed");Also applies to: 482-483
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.java` at line 379, The second preview assertion incorrectly calls previewPage.isNewRegistrationTitleDisplayed() after navigating back through the edit flow; replace that call with previewPage.updateUINTitleDisplayed() so both preview checks validate the Update UIN title (apply the same replacement where similar assertions occur around the other affected spots such as the assertions near lines referenced in the review).ui-test/src/main/java/regclient/page/KeycloakPage.java (1)
83-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSwitch to
NATIVE_APPbefore trying to scroll the native password tab.
openKeycloakWebView()leaves the driver in WEBVIEW, but this method now tries to horizontally scroll a native XPath before changing context. In that state the scroll loop just searches/swipes in the wrong context, so the later native wait still times out when the tab is off-screen.Minimal fix
public boolean openKeycloakPassword() { scrollToTopSafe(); By nativePwd = By.xpath("//*[`@text`='Password']"); - scrollHorizontallyUntilVisible(nativePwd); try { ((SupportsContextSwitching) driver).context("NATIVE_APP"); + scrollHorizontallyUntilVisible(nativePwd); new WebDriverWait(driver, Duration.ofSeconds(8)) .until(ExpectedConditions.visibilityOfElementLocated(nativePwd)); return true;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/page/KeycloakPage.java` around lines 83 - 88, The scroll is running in WEBVIEW because openKeycloakWebView() leaves the driver in WEBVIEW; switch the driver to the native context before attempting the native horizontal scroll. Change the order in KeycloakPage so that you call ((SupportsContextSwitching) driver).context("NATIVE_APP") (or a helper that switches context) immediately before calling scrollHorizontallyUntilVisible(nativePwd), then proceed with the WebDriverWait for nativePwd; keep the subsequent flow (waiting and any later context switches) intact.ui-test/src/main/java/regclient/page/BasePage.java (1)
657-669:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFive swipes is not enough to guarantee top/bottom on these flows.
This PR also adds page checks that call
scrollToTop()immediately before asserting scan/title visibility. Hard-capping both helpers at 5 swipes can leave long biometric/document screens mid-page, so those assertions still run against the wrong viewport.Safer options
protected void scrollToTop() { - int maxScrolls = 5; + int maxScrolls = 8; for (int i = 0; i < maxScrolls; i++) { swipeDown(); } } protected void scrollToBottom() { - int maxScrolls = 5; + int maxScrolls = 15; for (int i = 0; i < maxScrolls; i++) { swipeUp(); } }Or stop on a stable boundary instead of using a smaller fixed count.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/page/BasePage.java` around lines 657 - 669, The hard cap of 5 swipes in scrollToTop() and scrollToBottom() can leave long pages mid-scroll; update these helpers to stop on a stable boundary rather than a fixed small count: repeatedly call swipeDown()/swipeUp() until either the page/viewport stops changing (detect via stable page source, stable element position, or an isAtTop()/isAtBottom() check) or a much larger safety cap (e.g., 20) is reached; modify scrollToTop(), scrollToBottom() to implement this loop and use swipeDown()/swipeUp() as before, so callers that assert visibility after scrolling reliably see the intended viewport.
🧹 Nitpick comments (3)
ui-test/src/main/java/regclient/androidTestCases/Settings.java (1)
172-173: ⚡ Quick winAvoid fixed sleep before sync action.
Line 172 uses a hard wait that adds test latency and can still be flaky under slower/faster devices; use condition-based wait on the target job card/button instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java` around lines 172 - 173, Remove the fixed sleep BasePage.waitTime(10) and replace it with an explicit, condition-based wait for the specific sync UI element before calling settingsPage.clickOnSyncButton("Master Data Sync"); specifically, wait until the "Master Data Sync" job card or its sync button is visible and/or clickable (use your project's WebDriverWait/ExpectedConditions helper or add a settingsPage.waitForSyncJob("Master Data Sync") that waits for visibility/clickability) and then invoke settingsPage.clickOnSyncButton; ensure the hard wait is deleted so the test uses the condition-based wait for reliability and lower latency.ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java (1)
226-401: ⚡ Quick winFail fast on unsupported
screenOrderentries.This loop silently ignores any UI-spec screen id outside the four hard-coded branches, so spec drift turns into a later page-mismatch instead of a clear contract failure. Add an
elsethat fails on unknown ids, and ideally assert the mandatory screens expected fornewProcessbefore starting the loop.Suggested fix
- for (String screen : screenOrder) { + for (String screen : screenOrder) { if (screen.equals("consentdet") || screen.equals("consent")) { ... } else if (screen.equals("DemographicDetails")) { ... } else if (screen.equals("Documents")) { ... } else if (screen.equals("BiometricDetails")) { ... + } else { + throw new AssertionError("Unsupported screen in UI spec: " + screen); } }Based on learnings: In
ExportPacket.javaand similar Android UI test flows, thescreenOrderis sourced from the UI spec, so tests should validate that mandatory screens defined in the UI spec are always included and not skipped, and that the flow respects the spec-defined order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java` around lines 226 - 401, The loop over screenOrder in CreatePacketWithoutNetwork silently ignores unknown screen ids; add an else branch after the existing if/else-if chain that throws an IllegalStateException (or fails the test) with the unknown screen id to fail fast, and before the loop add a short verification that screenOrder contains the mandatory screens (e.g., "consentdet"/"consent", "DemographicDetails", "Documents", "BiometricDetails")—use screenOrder.contains(...) and throw a clear IllegalStateException if any mandatory screen is missing so the test fails immediately and points to spec drift.ui-test/src/main/java/regclient/api/KeycloakUserManager.java (1)
301-361: ⚡ Quick winInconsistent role assignment pattern.
The
createUserForDifferentCenter()method usesArcConfigManager.getRolesForUser()at line 350, whereas other specialized user creation methods follow a dedicated configuration pattern:
createUsersWithOutSupervisorRole()usesgetRolesForOperatorUser()createOnboardingUser()usesgetRolesForOnboardUser()Consider adding a dedicated
getRolesForDifferentCenterUser()method toArcConfigManagerif the "different center" user requires a distinct set of roles. This would maintain consistency with the established pattern and allow independent configuration of roles for each user type.♻️ Proposed refactor to align with established pattern
First, add the dedicated role getter to
ArcConfigManager:+ public static String getRolesForDifferentCenterUser() { + return getProperty("rolesForDifferentCenterUser", ""); + }Then update the role assignment in
KeycloakUserManager:- List<String> toBeAssignedRoles = List.of(ArcConfigManager.getRolesForUser().split(",")); + List<String> toBeAssignedRoles = List.of(ArcConfigManager.getRolesForDifferentCenterUser().split(","));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/api/KeycloakUserManager.java` around lines 301 - 361, The role assignment in createUserForDifferentCenter() is using ArcConfigManager.getRolesForUser(), which is inconsistent with other creators; add a dedicated getter like ArcConfigManager.getRolesForDifferentCenterUser() and update createUserForDifferentCenter() to call that method instead of getRolesForUser() so the different-center user's roles can be configured independently (update the List<String> to use the new getter and adjust any config parsing there).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java`:
- Around line 597-604: The loop blindly re-clicks the mutating operation
manageApplicationsPage.clickOnUploadButton(), which can double-submit if the UI
is slow; change the logic to perform a single click then wait for a clear state
transition (for example implement/wait for a method like
manageApplicationsPage.waitForUploadToSettle(), waitForRowDisappearance(), or
waitUntilUploadStatusChanges()) and only attempt a retry if that wait reports an
explicit failure/timeout; use
manageApplicationsPage.isZeroApplicationDisplayed() or an explicit error
indicator to decide whether to retry, and remove immediate re-clicks while the
prior upload is still pending.
- Line 163: The test currently calls BasePage.disableWifiAndData() without
guaranteeing re-enablement; wrap the test logic that runs while offline in a
try/finally so BasePage.enableWifiAndData() is always called in the finally
block; locate the call to BasePage.disableWifiAndData() in
CreatePacketWithoutNetwork and replace the surrounding test flow to ensure
BasePage.enableWifiAndData() is invoked in finally (also update the similar
occurrences around the lines referenced 539-540).
In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java`:
- Line 148: The test re-enabled a known-flaky assertion: remove or guard the
assertTrue(settingsPage.isNoDevicesFoundDisplayed(), ...) in the Settings test
that follows setAllToNotReadyAndSave(); instead revert to the previous safe
state by commenting out that assertion or wrapping it in an
environment-check/feature-flag so it only runs when MockSBI visibility is
guaranteed (locate the assert in Settings.java and the related
setAllToNotReadyAndSave() call to apply the change).
In `@ui-test/src/main/java/regclient/api/AdminTestUtil.java`:
- Around line 143-150: AdminTestUtil calls
propsKernel.getProperty("diffRegCenterId") and passes the result to
mapUserToCenter which can send a null "regCenterId"; add a null/empty check
before calling mapUserToCenter in AdminTestUtil (validate
propsKernel.getProperty("diffRegCenterId") and throw or log/fail fast) and/or
ensure Kernel.properties contains a diffRegCenterId entry; specifically update
the code around KeycloakUserManager.createUserForDifferentCenter() to validate
the diffRegCenterId value and avoid invoking mapUserToCenter with a null/empty
id.
In `@ui-test/src/main/java/regclient/page/BasePage.java`:
- Around line 217-231: The drag targets in cropCaptureImage (and similarly
cropCaptureImageEnglishFrench) use hardcoded viewport coordinates (e.g., 260,320
and 623,261) which break on different device resolutions; replace those
hardcoded numbers by computing target coordinates from the element bounds (use
element.getLocation() and element.getSize() or getCenterOfElement and
add/subtract offsets based on element width/height or fractions) and pass the
computed absolute viewport coordinates into createPointerMove so the pointer
moves relative to the element center/size rather than fixed pixels.
In `@ui-test/src/main/java/regclient/page/KeycloakPage.java`:
- Around line 136-138: clickOnPasswordOption currently calls the WebElement
overloads (scrollHorizontallyUntilVisible(passwordOption) and
clickOnElement(passwordOption)) which can cause staleness; change it to use the
locator-based overloads on BasePage instead: call
scrollHorizontallyUntilVisible(<By locator for passwordOption>) and
clickOnElement(<By locator for passwordOption>) so the element is re-resolved if
it becomes stale (use the existing By field or add one named e.g.
passwordOptionLocator if missing).
In
`@ui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.java`:
- Around line 77-87: The isPacketSizeDisplayed method currently uses
String.matches which requires the whole contentDescription to match and can
false-negative if there is surrounding text; update isPacketSizeDisplayed in
DocumentUploadPageEnglish to compile a Pattern like "Size:
\\d+(\\.\\d+)?\\s?(KB|MB)" and use Matcher.find() on
packetSize.getAttribute("contentDescription") (or String#contains with a looser
regex) to check for the size substring, keeping the
driver.findElement(By.xpath(...)) lookup and the existing exception handling.
In
`@ui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.java`:
- Around line 314-326: In scrollTillDisplayingApplicationCountVisible() the
lookup uses textContains("Displaying") but the page exposes the count via
accessibility/content-desc keys (displayZeroApplication /
displayOneApplication), so update the visibility check to look for those
accessibility locators instead of text: query for
driver.findElements(MobileBy.AccessibilityId("displayZeroApplication")) and
driver.findElements(MobileBy.AccessibilityId("displayOneApplication")) (or a
single descriptionContains selector that matches either) and return when found;
keep the existing scrollBackward() loop and only scroll if neither accessibility
element is present.
---
Outside diff comments:
In
`@ui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.java`:
- Line 379: The second preview assertion incorrectly calls
previewPage.isNewRegistrationTitleDisplayed() after navigating back through the
edit flow; replace that call with previewPage.updateUINTitleDisplayed() so both
preview checks validate the Update UIN title (apply the same replacement where
similar assertions occur around the other affected spots such as the assertions
near lines referenced in the review).
In `@ui-test/src/main/java/regclient/page/BasePage.java`:
- Around line 657-669: The hard cap of 5 swipes in scrollToTop() and
scrollToBottom() can leave long pages mid-scroll; update these helpers to stop
on a stable boundary rather than a fixed small count: repeatedly call
swipeDown()/swipeUp() until either the page/viewport stops changing (detect via
stable page source, stable element position, or an isAtTop()/isAtBottom() check)
or a much larger safety cap (e.g., 20) is reached; modify scrollToTop(),
scrollToBottom() to implement this loop and use swipeDown()/swipeUp() as before,
so callers that assert visibility after scrolling reliably see the intended
viewport.
In `@ui-test/src/main/java/regclient/page/KeycloakPage.java`:
- Around line 83-88: The scroll is running in WEBVIEW because
openKeycloakWebView() leaves the driver in WEBVIEW; switch the driver to the
native context before attempting the native horizontal scroll. Change the order
in KeycloakPage so that you call ((SupportsContextSwitching)
driver).context("NATIVE_APP") (or a helper that switches context) immediately
before calling scrollHorizontallyUntilVisible(nativePwd), then proceed with the
WebDriverWait for nativePwd; keep the subsequent flow (waiting and any later
context switches) intact.
In
`@ui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.java`:
- Around line 200-202: The isExceptionScanTitleDisplayed() method currently
returns isElementDisplayed(exceptionScanTitle) but omits the scrollToTop() call
used by the sibling checks; update isExceptionScanTitleDisplayed() in
SupervisorBiometricVerificationpageEnglish to call scrollToTop() before checking
isElementDisplayed(exceptionScanTitle) so its viewport behavior matches the
other thumb/face methods.
In
`@ui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.java`:
- Around line 207-209: The isExceptionScanTitleDisplayed() method omits the
scrollToTop() call used by the other scan-title checks; update the method
(isExceptionScanTitleDisplayed) to call scrollToTop() before returning
isElementDisplayed(exceptionScanTitle) so the exception scan title is always
scrolled into view prior to verification, matching the pattern used in
SupervisorBiometricVerificationpageEnglish and the other four scan-title checks.
---
Nitpick comments:
In
`@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java`:
- Around line 226-401: The loop over screenOrder in CreatePacketWithoutNetwork
silently ignores unknown screen ids; add an else branch after the existing
if/else-if chain that throws an IllegalStateException (or fails the test) with
the unknown screen id to fail fast, and before the loop add a short verification
that screenOrder contains the mandatory screens (e.g., "consentdet"/"consent",
"DemographicDetails", "Documents", "BiometricDetails")—use
screenOrder.contains(...) and throw a clear IllegalStateException if any
mandatory screen is missing so the test fails immediately and points to spec
drift.
In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java`:
- Around line 172-173: Remove the fixed sleep BasePage.waitTime(10) and replace
it with an explicit, condition-based wait for the specific sync UI element
before calling settingsPage.clickOnSyncButton("Master Data Sync"); specifically,
wait until the "Master Data Sync" job card or its sync button is visible and/or
clickable (use your project's WebDriverWait/ExpectedConditions helper or add a
settingsPage.waitForSyncJob("Master Data Sync") that waits for
visibility/clickability) and then invoke settingsPage.clickOnSyncButton; ensure
the hard wait is deleted so the test uses the condition-based wait for
reliability and lower latency.
In `@ui-test/src/main/java/regclient/api/KeycloakUserManager.java`:
- Around line 301-361: The role assignment in createUserForDifferentCenter() is
using ArcConfigManager.getRolesForUser(), which is inconsistent with other
creators; add a dedicated getter like
ArcConfigManager.getRolesForDifferentCenterUser() and update
createUserForDifferentCenter() to call that method instead of getRolesForUser()
so the different-center user's roles can be configured independently (update the
List<String> to use the new getter and adjust any config parsing there).
🪄 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: 24b0026f-9b04-4142-a4e7-95fff46c38a3
📒 Files selected for processing (29)
ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.javaui-test/src/main/java/regclient/androidTestCases/LoginTest.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationAdultException.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationAdultUploadMultipleDoccuments.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationInfant.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationMinor.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationMinorException.javaui-test/src/main/java/regclient/androidTestCases/Settings.javaui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.javaui-test/src/main/java/regclient/api/AdminTestUtil.javaui-test/src/main/java/regclient/api/ArcConfigManager.javaui-test/src/main/java/regclient/api/KeycloakUserManager.javaui-test/src/main/java/regclient/page/BasePage.javaui-test/src/main/java/regclient/page/KeycloakPage.javaui-test/src/main/java/regclient/page/MockSBIPage.javaui-test/src/main/java/regclient/page/SettingsPage.javaui-test/src/main/java/regclient/pages/arabic/SettingsPageArabic.javaui-test/src/main/java/regclient/pages/english/ApplicantBiometricsPageEnglish.javaui-test/src/main/java/regclient/pages/english/DemographicDetailsPageEnglish.javaui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.javaui-test/src/main/java/regclient/pages/english/IntroducerBiometricPageEnglish.javaui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.javaui-test/src/main/java/regclient/pages/english/PreviewPageEnglish.javaui-test/src/main/java/regclient/pages/english/SelectLanguagePageEnglish.javaui-test/src/main/java/regclient/pages/english/SettingsPageEnglish.javaui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.javaui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.javaui-test/src/main/java/regclient/pages/english/UpdateUINPageEnglish.javaui-test/src/main/java/regclient/pages/french/SettingsPageFrench.java
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
ui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.java (1)
200-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
isExceptionScanTitleDisplayed()omits thescrollToTop()that its siblings now use.The right/left/thumbs/face checks were updated to scroll to top first, but the exception variant was left as-is. If this method is exercised in tests, it may behave inconsistently (stale viewport) compared to the others. If exception scan is intentionally untested here, ignore.
Proposed alignment
public boolean isExceptionScanTitleDisplayed() { + scrollToTop(); return isElementDisplayed(exceptionScanTitle); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.java` around lines 200 - 202, The isExceptionScanTitleDisplayed() method currently returns isElementDisplayed(exceptionScanTitle) but omits the scrollToTop() call used by the sibling checks; update isExceptionScanTitleDisplayed() in SupervisorBiometricVerificationpageEnglish to call scrollToTop() before checking isElementDisplayed(exceptionScanTitle) so its viewport behavior matches the other thumb/face methods.ui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.java (1)
207-209:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame
scrollToTop()omission asSupervisorBiometricVerificationpageEnglish.
isExceptionScanTitleDisplayed()was not aligned with the other four scan-title checks. AddscrollToTop()here too if exception scan is verified in tests; otherwise this is fine to leave.Proposed alignment
public boolean isExceptionScanTitleDisplayed() { + scrollToTop(); return isElementDisplayed(exceptionScanTitle); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.java` around lines 207 - 209, The isExceptionScanTitleDisplayed() method omits the scrollToTop() call used by the other scan-title checks; update the method (isExceptionScanTitleDisplayed) to call scrollToTop() before returning isElementDisplayed(exceptionScanTitle) so the exception scan title is always scrolled into view prior to verification, matching the pattern used in SupervisorBiometricVerificationpageEnglish and the other four scan-title checks.ui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.java (1)
379-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the second preview assertion on the Update UIN title.
The first preview check correctly uses
updateUINTitleDisplayed(), but after navigating back through the edit flow this switches toisNewRegistrationTitleDisplayed(). That turns the revisit path into a false failure even though the test is still in the Update My UIN journey.Proposed fix
- assertTrue(previewPage.isNewRegistrationTitleDisplayed(), "Verify if new Registration title is displayed"); + assertTrue(previewPage.updateUINTitleDisplayed(), "Verify if update UIN title is displayed");Also applies to: 482-483
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.java` at line 379, The second preview assertion incorrectly calls previewPage.isNewRegistrationTitleDisplayed() after navigating back through the edit flow; replace that call with previewPage.updateUINTitleDisplayed() so both preview checks validate the Update UIN title (apply the same replacement where similar assertions occur around the other affected spots such as the assertions near lines referenced in the review).ui-test/src/main/java/regclient/page/KeycloakPage.java (1)
83-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSwitch to
NATIVE_APPbefore trying to scroll the native password tab.
openKeycloakWebView()leaves the driver in WEBVIEW, but this method now tries to horizontally scroll a native XPath before changing context. In that state the scroll loop just searches/swipes in the wrong context, so the later native wait still times out when the tab is off-screen.Minimal fix
public boolean openKeycloakPassword() { scrollToTopSafe(); By nativePwd = By.xpath("//*[`@text`='Password']"); - scrollHorizontallyUntilVisible(nativePwd); try { ((SupportsContextSwitching) driver).context("NATIVE_APP"); + scrollHorizontallyUntilVisible(nativePwd); new WebDriverWait(driver, Duration.ofSeconds(8)) .until(ExpectedConditions.visibilityOfElementLocated(nativePwd)); return true;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/page/KeycloakPage.java` around lines 83 - 88, The scroll is running in WEBVIEW because openKeycloakWebView() leaves the driver in WEBVIEW; switch the driver to the native context before attempting the native horizontal scroll. Change the order in KeycloakPage so that you call ((SupportsContextSwitching) driver).context("NATIVE_APP") (or a helper that switches context) immediately before calling scrollHorizontallyUntilVisible(nativePwd), then proceed with the WebDriverWait for nativePwd; keep the subsequent flow (waiting and any later context switches) intact.ui-test/src/main/java/regclient/page/BasePage.java (1)
657-669:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFive swipes is not enough to guarantee top/bottom on these flows.
This PR also adds page checks that call
scrollToTop()immediately before asserting scan/title visibility. Hard-capping both helpers at 5 swipes can leave long biometric/document screens mid-page, so those assertions still run against the wrong viewport.Safer options
protected void scrollToTop() { - int maxScrolls = 5; + int maxScrolls = 8; for (int i = 0; i < maxScrolls; i++) { swipeDown(); } } protected void scrollToBottom() { - int maxScrolls = 5; + int maxScrolls = 15; for (int i = 0; i < maxScrolls; i++) { swipeUp(); } }Or stop on a stable boundary instead of using a smaller fixed count.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/page/BasePage.java` around lines 657 - 669, The hard cap of 5 swipes in scrollToTop() and scrollToBottom() can leave long pages mid-scroll; update these helpers to stop on a stable boundary rather than a fixed small count: repeatedly call swipeDown()/swipeUp() until either the page/viewport stops changing (detect via stable page source, stable element position, or an isAtTop()/isAtBottom() check) or a much larger safety cap (e.g., 20) is reached; modify scrollToTop(), scrollToBottom() to implement this loop and use swipeDown()/swipeUp() as before, so callers that assert visibility after scrolling reliably see the intended viewport.
🧹 Nitpick comments (3)
ui-test/src/main/java/regclient/androidTestCases/Settings.java (1)
172-173: ⚡ Quick winAvoid fixed sleep before sync action.
Line 172 uses a hard wait that adds test latency and can still be flaky under slower/faster devices; use condition-based wait on the target job card/button instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java` around lines 172 - 173, Remove the fixed sleep BasePage.waitTime(10) and replace it with an explicit, condition-based wait for the specific sync UI element before calling settingsPage.clickOnSyncButton("Master Data Sync"); specifically, wait until the "Master Data Sync" job card or its sync button is visible and/or clickable (use your project's WebDriverWait/ExpectedConditions helper or add a settingsPage.waitForSyncJob("Master Data Sync") that waits for visibility/clickability) and then invoke settingsPage.clickOnSyncButton; ensure the hard wait is deleted so the test uses the condition-based wait for reliability and lower latency.ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java (1)
226-401: ⚡ Quick winFail fast on unsupported
screenOrderentries.This loop silently ignores any UI-spec screen id outside the four hard-coded branches, so spec drift turns into a later page-mismatch instead of a clear contract failure. Add an
elsethat fails on unknown ids, and ideally assert the mandatory screens expected fornewProcessbefore starting the loop.Suggested fix
- for (String screen : screenOrder) { + for (String screen : screenOrder) { if (screen.equals("consentdet") || screen.equals("consent")) { ... } else if (screen.equals("DemographicDetails")) { ... } else if (screen.equals("Documents")) { ... } else if (screen.equals("BiometricDetails")) { ... + } else { + throw new AssertionError("Unsupported screen in UI spec: " + screen); } }Based on learnings: In
ExportPacket.javaand similar Android UI test flows, thescreenOrderis sourced from the UI spec, so tests should validate that mandatory screens defined in the UI spec are always included and not skipped, and that the flow respects the spec-defined order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java` around lines 226 - 401, The loop over screenOrder in CreatePacketWithoutNetwork silently ignores unknown screen ids; add an else branch after the existing if/else-if chain that throws an IllegalStateException (or fails the test) with the unknown screen id to fail fast, and before the loop add a short verification that screenOrder contains the mandatory screens (e.g., "consentdet"/"consent", "DemographicDetails", "Documents", "BiometricDetails")—use screenOrder.contains(...) and throw a clear IllegalStateException if any mandatory screen is missing so the test fails immediately and points to spec drift.ui-test/src/main/java/regclient/api/KeycloakUserManager.java (1)
301-361: ⚡ Quick winInconsistent role assignment pattern.
The
createUserForDifferentCenter()method usesArcConfigManager.getRolesForUser()at line 350, whereas other specialized user creation methods follow a dedicated configuration pattern:
createUsersWithOutSupervisorRole()usesgetRolesForOperatorUser()createOnboardingUser()usesgetRolesForOnboardUser()Consider adding a dedicated
getRolesForDifferentCenterUser()method toArcConfigManagerif the "different center" user requires a distinct set of roles. This would maintain consistency with the established pattern and allow independent configuration of roles for each user type.♻️ Proposed refactor to align with established pattern
First, add the dedicated role getter to
ArcConfigManager:+ public static String getRolesForDifferentCenterUser() { + return getProperty("rolesForDifferentCenterUser", ""); + }Then update the role assignment in
KeycloakUserManager:- List<String> toBeAssignedRoles = List.of(ArcConfigManager.getRolesForUser().split(",")); + List<String> toBeAssignedRoles = List.of(ArcConfigManager.getRolesForDifferentCenterUser().split(","));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/api/KeycloakUserManager.java` around lines 301 - 361, The role assignment in createUserForDifferentCenter() is using ArcConfigManager.getRolesForUser(), which is inconsistent with other creators; add a dedicated getter like ArcConfigManager.getRolesForDifferentCenterUser() and update createUserForDifferentCenter() to call that method instead of getRolesForUser() so the different-center user's roles can be configured independently (update the List<String> to use the new getter and adjust any config parsing there).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java`:
- Around line 597-604: The loop blindly re-clicks the mutating operation
manageApplicationsPage.clickOnUploadButton(), which can double-submit if the UI
is slow; change the logic to perform a single click then wait for a clear state
transition (for example implement/wait for a method like
manageApplicationsPage.waitForUploadToSettle(), waitForRowDisappearance(), or
waitUntilUploadStatusChanges()) and only attempt a retry if that wait reports an
explicit failure/timeout; use
manageApplicationsPage.isZeroApplicationDisplayed() or an explicit error
indicator to decide whether to retry, and remove immediate re-clicks while the
prior upload is still pending.
- Line 163: The test currently calls BasePage.disableWifiAndData() without
guaranteeing re-enablement; wrap the test logic that runs while offline in a
try/finally so BasePage.enableWifiAndData() is always called in the finally
block; locate the call to BasePage.disableWifiAndData() in
CreatePacketWithoutNetwork and replace the surrounding test flow to ensure
BasePage.enableWifiAndData() is invoked in finally (also update the similar
occurrences around the lines referenced 539-540).
In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java`:
- Line 148: The test re-enabled a known-flaky assertion: remove or guard the
assertTrue(settingsPage.isNoDevicesFoundDisplayed(), ...) in the Settings test
that follows setAllToNotReadyAndSave(); instead revert to the previous safe
state by commenting out that assertion or wrapping it in an
environment-check/feature-flag so it only runs when MockSBI visibility is
guaranteed (locate the assert in Settings.java and the related
setAllToNotReadyAndSave() call to apply the change).
In `@ui-test/src/main/java/regclient/api/AdminTestUtil.java`:
- Around line 143-150: AdminTestUtil calls
propsKernel.getProperty("diffRegCenterId") and passes the result to
mapUserToCenter which can send a null "regCenterId"; add a null/empty check
before calling mapUserToCenter in AdminTestUtil (validate
propsKernel.getProperty("diffRegCenterId") and throw or log/fail fast) and/or
ensure Kernel.properties contains a diffRegCenterId entry; specifically update
the code around KeycloakUserManager.createUserForDifferentCenter() to validate
the diffRegCenterId value and avoid invoking mapUserToCenter with a null/empty
id.
In `@ui-test/src/main/java/regclient/page/BasePage.java`:
- Around line 217-231: The drag targets in cropCaptureImage (and similarly
cropCaptureImageEnglishFrench) use hardcoded viewport coordinates (e.g., 260,320
and 623,261) which break on different device resolutions; replace those
hardcoded numbers by computing target coordinates from the element bounds (use
element.getLocation() and element.getSize() or getCenterOfElement and
add/subtract offsets based on element width/height or fractions) and pass the
computed absolute viewport coordinates into createPointerMove so the pointer
moves relative to the element center/size rather than fixed pixels.
In `@ui-test/src/main/java/regclient/page/KeycloakPage.java`:
- Around line 136-138: clickOnPasswordOption currently calls the WebElement
overloads (scrollHorizontallyUntilVisible(passwordOption) and
clickOnElement(passwordOption)) which can cause staleness; change it to use the
locator-based overloads on BasePage instead: call
scrollHorizontallyUntilVisible(<By locator for passwordOption>) and
clickOnElement(<By locator for passwordOption>) so the element is re-resolved if
it becomes stale (use the existing By field or add one named e.g.
passwordOptionLocator if missing).
In
`@ui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.java`:
- Around line 77-87: The isPacketSizeDisplayed method currently uses
String.matches which requires the whole contentDescription to match and can
false-negative if there is surrounding text; update isPacketSizeDisplayed in
DocumentUploadPageEnglish to compile a Pattern like "Size:
\\d+(\\.\\d+)?\\s?(KB|MB)" and use Matcher.find() on
packetSize.getAttribute("contentDescription") (or String#contains with a looser
regex) to check for the size substring, keeping the
driver.findElement(By.xpath(...)) lookup and the existing exception handling.
In
`@ui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.java`:
- Around line 314-326: In scrollTillDisplayingApplicationCountVisible() the
lookup uses textContains("Displaying") but the page exposes the count via
accessibility/content-desc keys (displayZeroApplication /
displayOneApplication), so update the visibility check to look for those
accessibility locators instead of text: query for
driver.findElements(MobileBy.AccessibilityId("displayZeroApplication")) and
driver.findElements(MobileBy.AccessibilityId("displayOneApplication")) (or a
single descriptionContains selector that matches either) and return when found;
keep the existing scrollBackward() loop and only scroll if neither accessibility
element is present.
---
Outside diff comments:
In
`@ui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.java`:
- Line 379: The second preview assertion incorrectly calls
previewPage.isNewRegistrationTitleDisplayed() after navigating back through the
edit flow; replace that call with previewPage.updateUINTitleDisplayed() so both
preview checks validate the Update UIN title (apply the same replacement where
similar assertions occur around the other affected spots such as the assertions
near lines referenced in the review).
In `@ui-test/src/main/java/regclient/page/BasePage.java`:
- Around line 657-669: The hard cap of 5 swipes in scrollToTop() and
scrollToBottom() can leave long pages mid-scroll; update these helpers to stop
on a stable boundary rather than a fixed small count: repeatedly call
swipeDown()/swipeUp() until either the page/viewport stops changing (detect via
stable page source, stable element position, or an isAtTop()/isAtBottom() check)
or a much larger safety cap (e.g., 20) is reached; modify scrollToTop(),
scrollToBottom() to implement this loop and use swipeDown()/swipeUp() as before,
so callers that assert visibility after scrolling reliably see the intended
viewport.
In `@ui-test/src/main/java/regclient/page/KeycloakPage.java`:
- Around line 83-88: The scroll is running in WEBVIEW because
openKeycloakWebView() leaves the driver in WEBVIEW; switch the driver to the
native context before attempting the native horizontal scroll. Change the order
in KeycloakPage so that you call ((SupportsContextSwitching)
driver).context("NATIVE_APP") (or a helper that switches context) immediately
before calling scrollHorizontallyUntilVisible(nativePwd), then proceed with the
WebDriverWait for nativePwd; keep the subsequent flow (waiting and any later
context switches) intact.
In
`@ui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.java`:
- Around line 200-202: The isExceptionScanTitleDisplayed() method currently
returns isElementDisplayed(exceptionScanTitle) but omits the scrollToTop() call
used by the sibling checks; update isExceptionScanTitleDisplayed() in
SupervisorBiometricVerificationpageEnglish to call scrollToTop() before checking
isElementDisplayed(exceptionScanTitle) so its viewport behavior matches the
other thumb/face methods.
In
`@ui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.java`:
- Around line 207-209: The isExceptionScanTitleDisplayed() method omits the
scrollToTop() call used by the other scan-title checks; update the method
(isExceptionScanTitleDisplayed) to call scrollToTop() before returning
isElementDisplayed(exceptionScanTitle) so the exception scan title is always
scrolled into view prior to verification, matching the pattern used in
SupervisorBiometricVerificationpageEnglish and the other four scan-title checks.
---
Nitpick comments:
In
`@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java`:
- Around line 226-401: The loop over screenOrder in CreatePacketWithoutNetwork
silently ignores unknown screen ids; add an else branch after the existing
if/else-if chain that throws an IllegalStateException (or fails the test) with
the unknown screen id to fail fast, and before the loop add a short verification
that screenOrder contains the mandatory screens (e.g., "consentdet"/"consent",
"DemographicDetails", "Documents", "BiometricDetails")—use
screenOrder.contains(...) and throw a clear IllegalStateException if any
mandatory screen is missing so the test fails immediately and points to spec
drift.
In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java`:
- Around line 172-173: Remove the fixed sleep BasePage.waitTime(10) and replace
it with an explicit, condition-based wait for the specific sync UI element
before calling settingsPage.clickOnSyncButton("Master Data Sync"); specifically,
wait until the "Master Data Sync" job card or its sync button is visible and/or
clickable (use your project's WebDriverWait/ExpectedConditions helper or add a
settingsPage.waitForSyncJob("Master Data Sync") that waits for
visibility/clickability) and then invoke settingsPage.clickOnSyncButton; ensure
the hard wait is deleted so the test uses the condition-based wait for
reliability and lower latency.
In `@ui-test/src/main/java/regclient/api/KeycloakUserManager.java`:
- Around line 301-361: The role assignment in createUserForDifferentCenter() is
using ArcConfigManager.getRolesForUser(), which is inconsistent with other
creators; add a dedicated getter like
ArcConfigManager.getRolesForDifferentCenterUser() and update
createUserForDifferentCenter() to call that method instead of getRolesForUser()
so the different-center user's roles can be configured independently (update the
List<String> to use the new getter and adjust any config parsing there).
🪄 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: 24b0026f-9b04-4142-a4e7-95fff46c38a3
📒 Files selected for processing (29)
ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.javaui-test/src/main/java/regclient/androidTestCases/LoginTest.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationAdultException.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationAdultUploadMultipleDoccuments.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationInfant.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationMinor.javaui-test/src/main/java/regclient/androidTestCases/NewRegistrationMinorException.javaui-test/src/main/java/regclient/androidTestCases/Settings.javaui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDocuments.javaui-test/src/main/java/regclient/api/AdminTestUtil.javaui-test/src/main/java/regclient/api/ArcConfigManager.javaui-test/src/main/java/regclient/api/KeycloakUserManager.javaui-test/src/main/java/regclient/page/BasePage.javaui-test/src/main/java/regclient/page/KeycloakPage.javaui-test/src/main/java/regclient/page/MockSBIPage.javaui-test/src/main/java/regclient/page/SettingsPage.javaui-test/src/main/java/regclient/pages/arabic/SettingsPageArabic.javaui-test/src/main/java/regclient/pages/english/ApplicantBiometricsPageEnglish.javaui-test/src/main/java/regclient/pages/english/DemographicDetailsPageEnglish.javaui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.javaui-test/src/main/java/regclient/pages/english/IntroducerBiometricPageEnglish.javaui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.javaui-test/src/main/java/regclient/pages/english/PreviewPageEnglish.javaui-test/src/main/java/regclient/pages/english/SelectLanguagePageEnglish.javaui-test/src/main/java/regclient/pages/english/SettingsPageEnglish.javaui-test/src/main/java/regclient/pages/english/SupervisorBiometricVerificationpageEnglish.javaui-test/src/main/java/regclient/pages/english/UpdateOperatorBiometricspageEnglish.javaui-test/src/main/java/regclient/pages/english/UpdateUINPageEnglish.javaui-test/src/main/java/regclient/pages/french/SettingsPageFrench.java
🛑 Comments failed to post (8)
ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java (2)
163-163:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlways restore connectivity in a
finallyblock.If any assertion fails between disabling and re-enabling Wi‑Fi/data, the device stays offline and can poison every later test in the suite.
Suggested fix
- BasePage.disableWifiAndData(); - - loginPage.selectLanguage(); - ... - BasePage.enableWifiAndData(); - BasePage.waitTime(10); + BasePage.disableWifiAndData(); + try { + loginPage.selectLanguage(); + ... + BasePage.enableWifiAndData(); + BasePage.waitTime(10); + ... + } finally { + BasePage.enableWifiAndData(); + }Also applies to: 539-540
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java` at line 163, The test currently calls BasePage.disableWifiAndData() without guaranteeing re-enablement; wrap the test logic that runs while offline in a try/finally so BasePage.enableWifiAndData() is always called in the finally block; locate the call to BasePage.disableWifiAndData() in CreatePacketWithoutNetwork and replace the surrounding test flow to ensure BasePage.enableWifiAndData() is invoked in finally (also update the similar occurrences around the lines referenced 539-540).
597-604:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t blind-retry the upload action.
clickOnUploadButton()is a mutating operation. Re-clicking it before the first attempt settles can double-submit the same packet if the UI is just slow to refresh. Retry only after a confirmed failure, or wait for a state change such as row disappearance/upload status transition before clicking again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/CreatePacketWithoutNetwork.java` around lines 597 - 604, The loop blindly re-clicks the mutating operation manageApplicationsPage.clickOnUploadButton(), which can double-submit if the UI is slow; change the logic to perform a single click then wait for a clear state transition (for example implement/wait for a method like manageApplicationsPage.waitForUploadToSettle(), waitForRowDisappearance(), or waitUntilUploadStatusChanges()) and only attempt a retry if that wait reports an explicit failure/timeout; use manageApplicationsPage.isZeroApplicationDisplayed() or an explicit error indicator to decide whether to retry, and remove immediate re-clicks while the prior upload is still pending.ui-test/src/main/java/regclient/androidTestCases/Settings.java (1)
148-148:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winReintroduced known-failing assertion in this environment.
Line 148 re-enables
isNoDevicesFoundDisplayed()in the exact path previously documented as not visible after switching back from MockSBI, which can make this test fail nondeterministically in CI.Suggested stabilization diff
- assertTrue(settingsPage.isNoDevicesFoundDisplayed(), "Verify if no devices found displayed"); + // Intentionally skipped in current env: banner is not consistently visible after app switch. + // Keep scan button assertion as the stable post-condition. + // assertTrue(settingsPage.isNoDevicesFoundDisplayed(), "Verify if no devices found displayed");Based on learnings: In
ui-test/src/main/java/regclient/androidTestCases/Settings.java,settingsPage.isNoDevicesFoundDisplayed()is intentionally commented out because it is not currently visible in the test environment aftersetAllToNotReadyAndSave().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/androidTestCases/Settings.java` at line 148, The test re-enabled a known-flaky assertion: remove or guard the assertTrue(settingsPage.isNoDevicesFoundDisplayed(), ...) in the Settings test that follows setAllToNotReadyAndSave(); instead revert to the previous safe state by commenting out that assertion or wrapping it in an environment-check/feature-flag so it only runs when MockSBI visibility is guaranteed (locate the assert in Settings.java and the related setAllToNotReadyAndSave() call to apply the change).ui-test/src/main/java/regclient/api/AdminTestUtil.java (1)
143-150:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Search for diffRegCenterId property definition in config files rg -n "diffRegCenterId" --type properties --type xml --type jsonRepository: mosip/android-registration-client
Length of output: 112
🏁 Script executed:
#!/bin/bash set -euo pipefail # 1) Find all occurrences (code + config) of the key rg -n "diffRegCenterId" . # 2) Specifically look for likely definition patterns in .properties / .env / YAML / JSON / XML rg -n "(^|[[:space:]])diffRegCenterId[[:space:]]*=" . -g'*.properties' -g'*.env' -g'*.yml' -g'*.yaml' -g'*.json' -g'*.xml' -g'*.conf' || true rg -n "\"diffRegCenterId\"[[:space:]]*:" . -g'*.json' -g'*.yaml' -g'*.yml' || true rg -n "diffRegCenterId" . -g'*.xml' || true # 3) Locate propsKernel.getProperty call site(s) for context on validation/behavior when missing rg -n "getProperty\\(\"diffRegCenterId\"\\)" . # 4) Find where propsKernel is defined/loaded to understand missing-property behavior rg -n "propsKernel" ui-test/src/main/java/regclient/api/AdminTestUtil.java rg -n "class .*PropsKernel|propsKernel.*getProperty|getProperty\\(" ui-test/src/main/java/regclient/api/AdminTestUtil.java ui-test/src/main/java -S || trueRepository: mosip/android-registration-client
Length of output: 17387
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the relevant code around the snippet sed -n '120,175p' ui-test/src/main/java/regclient/api/AdminTestUtil.java | nl -ba # Inspect how propsKernel.getProperty is implemented/validated sed -n '1,120p' ui-test/src/main/java/regclient/utils/PropertiesReader.java | nl -ba # Find where propsKernel is created/loaded rg -n "propsKernel" ui-test/src/main/java/regclient -SRepository: mosip/android-registration-client
Length of output: 120
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== AdminTestUtil snippet (approx. 120-175) ===" sed -n '120,175p' ui-test/src/main/java/regclient/api/AdminTestUtil.java | cat -n echo "=== PropertiesReader implementation ===" sed -n '1,120p' ui-test/src/main/java/regclient/utils/PropertiesReader.java | cat -n echo "=== Where propsKernel is created/loaded ===" rg -n "propsKernel" ui-test/src/main/java/regclient -SRepository: mosip/android-registration-client
Length of output: 10821
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate Kernel.properties and print any matching keys fd -a -e properties 'Kernel.properties' ui-test || true fd -a 'Kernel.properties' . || true # Search all config/*.properties that likely back propsKernel rg -n "diffRegCenterId" . -g'*.properties' || true rg -n "diffRegCenterId" ui-test -g'*.properties' || true # Show the relevant line(s) and some surrounding context in Kernel.properties (if present) for f in $(fd -a 'Kernel.properties' ui-test); do echo "=== $f ===" rg -n "diffRegCenterId" "$f" || true done # Also inspect the code that loads propsKernel (BaseTestCase / KeycloakUserManager getproperty) rg -n "getproperty\\(TestRunner\\.getResourcePath\\(\\) \\+ \"/config/Kernel\\.properties\"\\)" ui-test/src/main/java/regclient/api -S sed -n '1,120p' ui-test/src/main/java/regclient/api/BaseTestCase.java | cat -n sed -n '1,120p' ui-test/src/main/java/regclient/api/KeycloakUserManager.java | cat -nRepository: mosip/android-registration-client
Length of output: 11833
🏁 Script executed:
#!/bin/bash set -euo pipefail KERNEL="ui-test/src/main/resources/config/Kernel.properties" echo "=== Does Kernel.properties contain diffRegCenterId? ===" rg -n "diffRegCenterId" "$KERNEL" || true echo echo "=== Kernel.properties lines mentioning regCenterId/zoneMapping (context) ===" rg -n "(regCenterId|zoneMappingUrl|zoneMappingActivateUrl)" "$KERNEL" || true echo echo "=== mapUserToCenter implementation in AdminTestUtil.java ===" rg -n "mapUserToCenter\\(" ui-test/src/main/java/regclient/api/AdminTestUtil.java # print surrounding method start=$(rg -n "mapUserToCenter\\(" ui-test/src/main/java/regclient/api/AdminTestUtil.java | head -n1 | cut -d: -f1) # show a window around the first occurrence sed -n "$((start-10)),$((start+80))p" ui-test/src/main/java/regclient/api/AdminTestUtil.java | cat -nRepository: mosip/android-registration-client
Length of output: 6550
🏁 Script executed:
#!/bin/bash set -euo pipefail # 1) Confirm where diffRegCenterId is defined anywhere in the repo configs rg -n "diffRegCenterId" . -g'*.properties' -g'*.env' -g'*.yml' -g'*.yaml' -g'*.json' -g'*.xml' -g'*.conf' || true # 2) Inspect mapUserToCenter method behavior when center is null/empty sed -n '200,320p' ui-test/src/main/java/regclient/api/AdminTestUtil.java | cat -nRepository: mosip/android-registration-client
Length of output: 6206
🏁 Script executed:
#!/bin/bash set -euo pipefail # Global search for the key (all file types) rg -n "diffRegCenterId" . || true # Show nearby kernel keys for context KERNEL="ui-test/src/main/resources/config/Kernel.properties" rg -n "iam-users-to-create-differentCenterUser|diffRegCenterId|regCenterId|zone" "$KERNEL" || trueRepository: mosip/android-registration-client
Length of output: 477
Define/validate
diffRegCenterIdfor different-center user mapping
AdminTestUtilusespropsKernel.getProperty("diffRegCenterId")(line 149) butdiffRegCenterIdis not present inui-test/src/main/resources/config/Kernel.properties(only referenced in this code), somapUserToCenterwill send"regCenterId": nullto/v1/masterdata/usercentermapping, breaking the “differentCenterUser” initialization path.- Add
diffRegCenterIdtoKernel.propertiesand/or fail fast with a non-null validation before callingmapUserToCenter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/api/AdminTestUtil.java` around lines 143 - 150, AdminTestUtil calls propsKernel.getProperty("diffRegCenterId") and passes the result to mapUserToCenter which can send a null "regCenterId"; add a null/empty check before calling mapUserToCenter in AdminTestUtil (validate propsKernel.getProperty("diffRegCenterId") and throw or log/fail fast) and/or ensure Kernel.properties contains a diffRegCenterId entry; specifically update the code around KeycloakUserManager.createUserForDifferentCenter() to validate the diffRegCenterId value and avoid invoking mapUserToCenter with a null/empty id.ui-test/src/main/java/regclient/page/BasePage.java (1)
217-231:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winCompute crop targets from the element bounds, not fixed viewport pixels.
Both crop helpers now drag to absolute coordinates (
260,320/623,261). That makes the gesture depend on the exact device resolution/orientation used when this was recorded, so on a different emulator/phone the drag can miss the crop area entirely.Suggested direction
protected void cropCaptureImage(WebElement element) { + Rectangle rect = element.getRect(); + int endX = rect.x + (int) (rect.width * 0.75); + int endY = rect.y + (int) (rect.height * 0.25); PointerInput finger1 = new PointerInput(PointerInput.Kind.TOUCH, "finger1"); Sequence sequence = new Sequence(finger1, 1) .addAction(finger1.createPointerMove(Duration.ZERO, PointerInput.Origin.viewport(), getCenterOfElement(element.getLocation(), element.getSize()))) .addAction(finger1.createPointerDown(PointerInput.MouseButton.LEFT.asArg())) .addAction(new Pause(finger1, Duration.ofMillis(200))) - .addAction(finger1.createPointerMove(Duration.ofMillis(500), PointerInput.Origin.viewport(), 260, 320)) + .addAction(finger1.createPointerMove(Duration.ofMillis(500), PointerInput.Origin.viewport(), endX, endY)) .addAction(finger1.createPointerUp(PointerInput.MouseButton.LEFT.asArg())); driver.perform(Collections.singletonList(sequence)); }Apply the same pattern to
cropCaptureImageEnglishFrench(...).Also applies to: 234-249
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/page/BasePage.java` around lines 217 - 231, The drag targets in cropCaptureImage (and similarly cropCaptureImageEnglishFrench) use hardcoded viewport coordinates (e.g., 260,320 and 623,261) which break on different device resolutions; replace those hardcoded numbers by computing target coordinates from the element bounds (use element.getLocation() and element.getSize() or getCenterOfElement and add/subtract offsets based on element width/height or fractions) and pass the computed absolute viewport coordinates into createPointerMove so the pointer moves relative to the element center/size rather than fixed pixels.ui-test/src/main/java/regclient/page/KeycloakPage.java (1)
136-138:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the locator-based horizontal scroll here too.
This still calls the
WebElementoverload, so ifpasswordOptiongoes stale while the tab strip is being swiped it can never be re-found. The newByoverload added inBasePagefixes exactly that failure mode.Suggested change
public void clickOnPasswordOption() { - scrollHorizontallyUntilVisible(passwordOption); + scrollHorizontallyUntilVisible(By.xpath("//android.widget.TextView[`@text`='Password']")); clickOnElement(passwordOption); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/page/KeycloakPage.java` around lines 136 - 138, clickOnPasswordOption currently calls the WebElement overloads (scrollHorizontallyUntilVisible(passwordOption) and clickOnElement(passwordOption)) which can cause staleness; change it to use the locator-based overloads on BasePage instead: call scrollHorizontallyUntilVisible(<By locator for passwordOption>) and clickOnElement(<By locator for passwordOption>) so the element is re-resolved if it becomes stale (use the existing By field or add one named e.g. passwordOptionLocator if missing).ui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.java (1)
77-87:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
matches()may produce false negatives ifcontent-deschas surrounding text.The XPath uses
contains(@content-desc,'Size:'), so the matched element'scontentDescriptioncould legitimately include extra text (e.g. a prefix/suffix).String.matches()requires the entire string to match the pattern, so any surrounding text makes this returnfalseand would fail the downstreamassertTrue(...isPacketSizeDisplayed())inCreatePacketWithoutNetwork. Prefer afind()-based check.🛡️ Proposed fix
String sizeText = packetSize.getAttribute("contentDescription"); - return sizeText.matches("Size: \\d+(\\.\\d+)?\\s?(KB|MB)"); + return sizeText != null + && java.util.regex.Pattern.compile("Size:\\s*\\d+(\\.\\d+)?\\s?(KB|MB)") + .matcher(sizeText).find();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.java` around lines 77 - 87, The isPacketSizeDisplayed method currently uses String.matches which requires the whole contentDescription to match and can false-negative if there is surrounding text; update isPacketSizeDisplayed in DocumentUploadPageEnglish to compile a Pattern like "Size: \\d+(\\.\\d+)?\\s?(KB|MB)" and use Matcher.find() on packetSize.getAttribute("contentDescription") (or String#contains with a looser regex) to check for the size substring, keeping the driver.findElement(By.xpath(...)) lookup and the existing exception handling.ui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.java (1)
314-326:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMatch the application-count lookup to the page's accessibility locators.
This helper searches for
textContains("Displaying"), but this page exposes the count as accessibility/content-desc (displayZeroApplication/displayOneApplication). In practice that means the method can keep scrolling even when the count is already visible.Proposed fix
- if (driver.findElements(MobileBy.AndroidUIAutomator("new UiSelector().textContains(\"Displaying\")")) + if (driver.findElements(MobileBy.AndroidUIAutomator("new UiSelector().descriptionContains(\"Displaying\")")) .size() > 0) { return; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.public void scrollTillDisplayingApplicationCountVisible() { for (int i = 0; i < 3; i++) { if (driver.findElements(MobileBy.AndroidUIAutomator("new UiSelector().descriptionContains(\"Displaying\")")) .size() > 0) { return; } driver.findElement(MobileBy.AndroidUIAutomator( "new UiScrollable(new UiSelector().className(\"android.widget.HorizontalScrollView\"))" ".setAsHorizontalList()" + ".scrollBackward()")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.java` around lines 314 - 326, In scrollTillDisplayingApplicationCountVisible() the lookup uses textContains("Displaying") but the page exposes the count via accessibility/content-desc keys (displayZeroApplication / displayOneApplication), so update the visibility check to look for those accessibility locators instead of text: query for driver.findElements(MobileBy.AccessibilityId("displayZeroApplication")) and driver.findElements(MobileBy.AccessibilityId("displayOneApplication")) (or a single descriptionContains selector that matches either) and return when found; keep the existing scrollBackward() loop and only scroll if neither accessibility element is present.
jayesh12234
approved these changes
Jun 4, 2026
mohanachandran-s
approved these changes
Jun 4, 2026
Prafulrakhade
approved these changes
Jun 4, 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
4 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.
RCF-1349: Automation Task for Phone Screen
Summary by CodeRabbit
Release Notes