MOSIP-44178:Update dashboard positive flow testcasesDevelop#680
MOSIP-44178:Update dashboard positive flow testcasesDevelop#680maheswaras wants to merge 6 commits intomosip:developfrom
Conversation
Signed-off-by: maheswaras <maheswara.s@cyberpwn.com>
Signed-off-by: maheswaras <maheswara.s@cyberpwn.com>
Signed-off-by: maheswaras <maheswara.s@cyberpwn.com>
WalkthroughAdds packet-count logging and a post-login active-user check: new abstract DashboardPage methods, a BasePage table helper, implementations across localized Dashboard pages to expose/log packet counts and check user active status, a LoginTest post-condition call/assert, and a minor import cleanup in LoginPageFrench. Changes
Sequence DiagramsequenceDiagram
participant LoginTest as LoginTest
participant Dashboard as DashboardPage
participant UI as UIElements
participant KC as KeycloakUserManager
participant Log as Logger
LoginTest->>Dashboard: logPacketCounts()
Dashboard->>UI: read packetCreatedNumber
Dashboard->>UI: read packetSyncedNumber
Dashboard->>UI: read packetUploadedNumber
Dashboard->>Log: log packet counts
LoginTest->>Dashboard: isLoginUserActive()
Dashboard->>KC: get moduleSpecificUser()
Dashboard->>UI: query userTable for moduleSpecificUser
Dashboard->>UI: read status column (offset)
Dashboard-->>LoginTest: return active? (boolean)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui-test/src/main/java/regclient/pages/hindi/DashboardPageHindi.java (1)
24-34:⚠️ Potential issue | 🔴 CriticalCritical: Hindi dashboard page uses French locators — copy-paste error.
The locators on this Hindi page are in French, not Hindi:
- Line 24:
"Tableau de bord"(French for "Dashboard")- Line 27:
"Identifiant utilisateur"(French for "User ID")- Line 30:
"Nom d'utilisateur"(French for "User Name")- Line 33:
"Statut"(French for "Status")These will fail element lookups when the app is running in Hindi. They should use Hindi-localized strings (or whichever content-desc the Hindi UI actually renders).
🤖 Fix all issues with AI agents
In `@ui-test/src/main/java/regclient/page/BasePage.java`:
- Around line 913-929: The case-insensitive comparison in isValuePresentInTable
currently lowercases actualValue but not expectedValue, making matches fragile;
update the comparison in isValuePresentInTable (method name) to perform a true
case-insensitive check by normalizing both strings (e.g.,
actualValue.toLowerCase(Locale.ROOT).contains(expectedValue.toLowerCase(Locale.ROOT)))
or use an appropriate locale-aware comparison so callers like
getVisibleValue/rowIdentifier/valueLineOffset with "Active" will match "active".
In `@ui-test/src/main/java/regclient/pages/english/DashboardPageEnglish.java`:
- Around line 50-52: In DashboardPageEnglish update the ambiguous locator for
the field userTable (which currently shares the same XPath as userIDTitle) to
target a distinct element; modify the XPath so it selects the table header or
container (for example by using a unique content-desc like "Users" / "Users
Table" or by anchoring to a parent/section element or header text) rather than
contains(`@content-desc`,'User ID'), ensuring userIDTitle remains unchanged and
userTable uniquely identifies the users table element within the
DashboardPageEnglish class.
- Around line 104-106: isLoginUserActive currently passes
KeycloakUserManager.moduleSpecificUser (a public static mutable field) directly
to isValuePresentInTable, which can silently return false if the field is still
null; add a precondition in isLoginUserActive to check
KeycloakUserManager.moduleSpecificUser for null and fail loudly (throw an
IllegalStateException or AssertionError with a clear message) if it's not set,
so tests don't mask setup problems—refer to isLoginUserActive,
KeycloakUserManager.moduleSpecificUser, isValuePresentInTable and the
createUsers() setup as the locations to consider for this guard.
In `@ui-test/src/main/java/regclient/pages/french/LoginPageFrench.java`:
- Around line 20-28: The packet-count WebElement fields (packetUploadedNumber,
packetSyncedNumber, packetCreatedNumber), their getters, and the method
logPacketCounts() are incorrectly declared in LoginPageFrench; move those fields
and methods into DashboardPageFrench (the concrete class that fulfills
DashboardPage.logPacketCounts()), remove them from LoginPageFrench, ensure
DashboardPageFrench imports AndroidFindBy and WebElement, declare the fields
with the same names and access modifiers, implement/override logPacketCounts()
there to satisfy the DashboardPage contract, and update any tests or callers to
reference dashboardPage.logPacketCounts() as before.
- Line 5: This file (LoginPageFrench) and several others still import and use
the legacy org.apache.log4j.Logger; replace those with org.slf4j.Logger and
obtain the logger via org.slf4j.LoggerFactory.getLogger(...) (e.g., change any
Logger logger = Logger.getLogger(LoginPageFrench.class) to private static final
Logger logger = LoggerFactory.getLogger(LoginPageFrench.class)), remove the old
import and update similar occurrences in the other affected classes
(DashboardPage variants, BaseTestCase, ArcConfigManager, AdminTestUtil, etc.) so
all logging uses SLF4J consistently with the project's log4j-slf4j2
implementation.
🧹 Nitpick comments (2)
ui-test/src/main/java/regclient/pages/kannada/DashboardPageKannada.java (1)
65-89: LGTM — consistent implementation across locale pages.The new methods follow the established pattern. One minor observation:
userTable(line 35) anduserIDTitle(line 26) share the same XPath selector. They'll resolve to the same element, which is fine since they serve different logical purposes (display check vs. content parsing), but you could consider reusing a single field.ui-test/src/main/java/regclient/pages/arabic/DashboardPageArabic.java (1)
67-86:logPacketCounts()and the three getter methods are identical across all locale pages.Since these methods don't depend on any locale-specific fields (the
WebElementfields they call are already locale-specific via@AndroidFindBy), the logic itself is uniform. Consider pullinglogPacketCounts()up intoDashboardPage(the abstract base) as a concrete method, leaving only the abstract getters in the subclasses — or making the getters concrete inDashboardPageifgetVisibleValueis available there.This would eliminate the duplicated method body in all six locale files.
ui-test/src/main/java/regclient/pages/french/LoginPageFrench.java
Outdated
Show resolved
Hide resolved
ui-test/src/main/java/regclient/pages/french/LoginPageFrench.java
Outdated
Show resolved
Hide resolved
Signed-off-by: maheswaras <maheswara.s@cyberpwn.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui-test/src/main/java/regclient/pages/hindi/DashboardPageHindi.java (1)
29-42:⚠️ Potential issue | 🔴 CriticalUpdate locators to use Hindi text instead of French.
Lines 29–39 use French localization strings identical to
DashboardPageFrench.java:
dashboardPageTitle: uses"Tableau de bord"(French for Dashboard)userIDTitle: uses"Identifiant utilisateur"(French for User ID)userNameTitle: uses"Nom d'utilisateur"(French for User Name)statusTitle: uses"Statut"(French for Status)These should match the Hindi UI text for the test to work when the app language is set to Hindi. Compare against
DashboardPageArabic.javaandDashboardPageEnglish.javawhich correctly use their respective languages.Additionally, the
userTablelocator (line 41) uses English"User ID". Verify whether this is intentional (content-desc may be locale-independent) or if it should also use the Hindi equivalent.ui-test/src/main/java/regclient/pages/kannada/DashboardPageKannada.java (1)
29-39:⚠️ Potential issue | 🔴 Critical
userTableshould use a different XPath fromuserIDTitleto target the actual table container, not the title element.Lines 29 and 38 use identical XPaths (
//android.view.View[contains(@content-desc,'User ID')]). Other locale implementations (French, Arabic) correctly use distinct locators: French uses "Identifiant utilisateur" for the title and "Utilisatrices" for the table; Arabic uses different Arabic strings for each. TheisValuePresentInTablemethod expectsuserTableto contain searchable table data, not just a title element. Align the Kannada implementation with the correct pattern used in French and Arabic locales.
🤖 Fix all issues with AI agents
In `@ui-test/src/main/java/regclient/pages/arabic/DashboardPageArabic.java`:
- Line 9: Remove the unused import of DashboardPageHindi from the
DashboardPageArabic class: locate the import statement "import
regclient.pages.hindi.DashboardPageHindi;" at the top of
DashboardPageArabic.java and delete it so the class no longer contains an unused
import reference.
In `@ui-test/src/main/java/regclient/pages/french/DashboardPageFrench.java`:
- Line 9: Remove the unused import of DashboardPageEnglish from the
DashboardPageFrench class: delete the line importing
regclient.pages.english.DashboardPageEnglish in DashboardPageFrench.java (or run
your IDE's "organize/imports" to remove unused imports) so the class no longer
references an import that isn't used.
In `@ui-test/src/main/java/regclient/pages/tamil/DashboardPageTamil.java`:
- Around line 92-94: Extract the magic number into a named constant (e.g.,
STATUS_COLUMN_INDEX) and use that in isLoginUserActive to make the intent clear
(replace literal 2 with STATUS_COLUMN_INDEX); additionally add a null-check for
KeycloakUserManager.moduleSpecificUser in isLoginUserActive (or before calling
isValuePresentInTable) and handle it explicitly—either return false or throw a
clear IllegalStateException/Assertion with a message like "moduleSpecificUser
not provisioned"—so you avoid passing null into isValuePresentInTable and get a
meaningful failure; reference symbols: isLoginUserActive, isValuePresentInTable,
userTable, KeycloakUserManager.moduleSpecificUser, STATUS_COLUMN_INDEX.
🧹 Nitpick comments (8)
ui-test/src/main/java/regclient/pages/arabic/DashboardPageArabic.java (1)
81-88: Use SLF4J parameterized logging instead of string concatenation.String concatenation in
logger.info()calls incurs the cost of building the string even when the log level is disabled. Use{}placeholders instead.Proposed fix
- logger.info("No. of Packets Created : " + created); - logger.info("No. of Packets Synced : " + synced); - logger.info("No. of Packets Uploaded : " + uploaded); + logger.info("No. of Packets Created : {}", created); + logger.info("No. of Packets Synced : {}", synced); + logger.info("No. of Packets Uploaded : {}", uploaded);ui-test/src/main/java/regclient/pages/english/DashboardPageEnglish.java (1)
94-101: Prefer SLF4J parameterized logging over string concatenation.Using
+to build log messages evaluates the concatenation even when the log level is disabled. SLF4J's{}placeholders defer this cost.♻️ Suggested change
public void logPacketCounts() { String created = getPacketsCreatedCount(); String synced = getPacketsSyncedCount(); String uploaded = getPacketsUploadedCount(); - logger.info("No. of Packets Created : " + created); - logger.info("No. of Packets Synced : " + synced); - logger.info("No. of Packets Uploaded : " + uploaded); + logger.info("No. of Packets Created : {}", created); + logger.info("No. of Packets Synced : {}", synced); + logger.info("No. of Packets Uploaded : {}", uploaded); }ui-test/src/main/java/regclient/pages/french/DashboardPageFrench.java (1)
84-91: Same nit as the English variant: prefer SLF4J parameterized logging.♻️ Suggested change
- logger.info("No. of Packets Created : " + created); - logger.info("No. of Packets Synced : " + synced); - logger.info("No. of Packets Uploaded : " + uploaded); + logger.info("No. of Packets Created : {}", created); + logger.info("No. of Packets Synced : {}", synced); + logger.info("No. of Packets Uploaded : {}", uploaded);ui-test/src/main/java/regclient/pages/tamil/DashboardPageTamil.java (2)
83-90: Use SLF4J parameterized logging instead of string concatenation.String concatenation in
logger.info()is eagerly evaluated regardless of log level. Use{}placeholders for idiomatic SLF4J usage.♻️ Proposed fix
public void logPacketCounts() { String created = getPacketsCreatedCount(); String synced = getPacketsSyncedCount(); String uploaded = getPacketsUploadedCount(); - logger.info("No. of Packets Created : " + created); - logger.info("No. of Packets Synced : " + synced); - logger.info("No. of Packets Uploaded : " + uploaded); + logger.info("No. of Packets Created : {}", created); + logger.info("No. of Packets Synced : {}", synced); + logger.info("No. of Packets Uploaded : {}", uploaded); }
40-41: Clarify intent of duplicate XPath foruserTableanduserIDTitle.The XPath for
userTable(line 40) anduserIDTitle(line 31) are functionally identical—both match//android.view.View[contains(@content-desc,'User ID')]. While the code works consistently across all locale implementations, having two fields with the same locator is confusing and indicates either:
- A naming/semantic issue: If
userTableis meant to represent the table container (distinct from the header), the XPath should reflect that.- Unintentional duplication: Consider consolidating or renaming for clarity.
Verify whether this duplication is intentional or refactor the XPath/naming to clarify the semantic difference between
userIDTitle(header verification) anduserTable(table data extraction).ui-test/src/main/java/regclient/pages/hindi/DashboardPageHindi.java (1)
84-91: Prefer SLF4J parameterized logging over string concatenation.Using
+forces eager string concatenation even when the log level is disabled. SLF4J's{}placeholders defer formatting.♻️ Suggested change
public void logPacketCounts() { String created = getPacketsCreatedCount(); String synced = getPacketsSyncedCount(); String uploaded = getPacketsUploadedCount(); - logger.info("No. of Packets Created : " + created); - logger.info("No. of Packets Synced : " + synced); - logger.info("No. of Packets Uploaded : " + uploaded); + logger.info("No. of Packets Created : {}", created); + logger.info("No. of Packets Synced : {}", synced); + logger.info("No. of Packets Uploaded : {}", uploaded); }ui-test/src/main/java/regclient/pages/kannada/DashboardPageKannada.java (2)
81-88: Prefer SLF4J parameterized logging over string concatenation.Using
+to build log messages defeats SLF4J's lazy evaluation (the string is always built, even if the log level is disabled).♻️ Proposed fix
public void logPacketCounts() { String created = getPacketsCreatedCount(); String synced = getPacketsSyncedCount(); String uploaded = getPacketsUploadedCount(); - logger.info("No. of Packets Created : " + created); - logger.info("No. of Packets Synced : " + synced); - logger.info("No. of Packets Uploaded : " + uploaded); + logger.info("No. of Packets Created : {}", created); + logger.info("No. of Packets Synced : {}", synced); + logger.info("No. of Packets Uploaded : {}", uploaded); }
90-92: Magic number2inisValuePresentInTablecall — consider using a named constant.The column index
2and the string"active"are opaque. A named constant (e.g.,STATUS_COLUMN_INDEX = 2) would improve readability and make future column-order changes safer.
ui-test/src/main/java/regclient/pages/arabic/DashboardPageArabic.java
Outdated
Show resolved
Hide resolved
ui-test/src/main/java/regclient/pages/french/DashboardPageFrench.java
Outdated
Show resolved
Hide resolved
Signed-off-by: maheswaras <maheswara.s@cyberpwn.com>
3 participants
Update Dashboard positive flow testcases
Summary by CodeRabbit