Execute script and Android startActivity action +semver: feature #52
Conversation
# Conflicts: # azure-pipelines.yml
Update Selenium reference to v.4.34.0 Update license year
mialeska
added
enhancement
WalkthroughThis update introduces new abstractions and implementations for Android activity management, centralizes script execution logic in the application layer, and adds related logging and localization entries. Dependency and pipeline configuration versions are updated. The test code is refactored to use the new Android actions abstraction, and minor updates are made to the license and localization files. Changes
Sequence Diagram(s)sequenceDiagram
participant TestCode as AndroidScreen (Test)
participant Services as AqualityServices
participant Actions as AndroidActions
participant App as Application
TestCode->>Services: getAndroidActions()
Services->>Actions: (resolve via DI)
TestCode->>Actions: startActivity(appPackage, appActivity, stopApp)
Actions->>App: executeScript("mobile: startActivity", params)
App->>App: log execution, retry logic
App->>Actions: return result
Actions->>TestCode: wait until activity matches, else throw error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/main/resources/localization/ru.json (1)
16-17: Same placeholder-numbering nitpick as in the EN bundle
See comment inen.json. Harmonising formatting rules will save future localisation effort.src/main/resources/localization/be.json (1)
16-17: Same placeholder-numbering nitpick as in the EN bundle
See comment inen.json. Harmonising formatting rules will save future localisation effort.src/main/resources/localization/uk.json (1)
16-17: Same placeholder-numbering nitpick as in the EN bundle
See comment inen.json. Harmonising formatting rules will save future localisation effort.
🧹 Nitpick comments (4)
src/main/resources/localization/en.json (1)
16-17: Consider using numbered placeholders for consistency across the resource bundleMost entries that accept parameters rely on numbered specifiers (
%1$s,%2$s) to guard against argument-order differences between languages.
Staying consistent reduces the risk of localisation bugs when translators reorder placeholders.- "loc.application.execute.script": "Executing script '%s'", - "loc.application.script.result": "Script result: '%s'", + "loc.application.execute.script": "Executing script '%1$s'", + "loc.application.script.result": "Script result: '%1$s'",Not mandatory, but worth aligning before these keys are consumed widely.
src/main/java/aquality/appium/mobile/application/Application.java (2)
108-119: LGTM: Solid implementation with minor suggestion for clarity.The
executeScriptimplementation is well-structured with proper retry logic and logging. The conditional result logging is a nice touch.Minor suggestion for improved readability:
- String argsString = params == null || params.isEmpty() ? "" : ", " + params; + String argsString = (params == null || params.isEmpty()) ? "" : ", " + params;The extra parentheses make the boolean logic clearer.
128-128: Consider using empty map instead of null for consistency.While this works correctly, passing
Collections.emptyMap()instead ofnullwould be more consistent with the interface's default method implementation and clearer in intent.- Map<String, Object> result = executeScript(iosExtName, null); + Map<String, Object> result = executeScript(iosExtName, Collections.emptyMap());src/main/java/aquality/appium/mobile/actions/AndroidActions.java (1)
11-11: Consider adding input parameter validation.The method doesn't validate that
appPackageandappActivityare not null or empty, which could lead to runtime issues.+ if (appPackage == null || appPackage.trim().isEmpty()) { + throw new IllegalArgumentException("appPackage cannot be null or empty"); + } + if (appActivity == null || appActivity.trim().isEmpty()) { + throw new IllegalArgumentException("appActivity cannot be null or empty"); + } AqualityServices.getLocalizedLogger().info("loc.application.android.activity.start", appPackage, appActivity);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
LICENSE(1 hunks)azure-pipelines.yml(1 hunks)pom.xml(2 hunks)src/main/java/aquality/appium/mobile/actions/AndroidActions.java(1 hunks)src/main/java/aquality/appium/mobile/actions/IActionsModule.java(1 hunks)src/main/java/aquality/appium/mobile/actions/IAndroidActions.java(1 hunks)src/main/java/aquality/appium/mobile/application/Application.java(1 hunks)src/main/java/aquality/appium/mobile/application/AqualityServices.java(2 hunks)src/main/java/aquality/appium/mobile/application/IMobileApplication.java(2 hunks)src/main/java/aquality/appium/mobile/application/MobileModule.java(2 hunks)src/main/resources/localization/be.json(1 hunks)src/main/resources/localization/en.json(1 hunks)src/main/resources/localization/pl.json(1 hunks)src/main/resources/localization/ru.json(1 hunks)src/main/resources/localization/uk.json(1 hunks)src/test/java/samples/android/nativeapp/apidemos/screens/AndroidScreen.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/aquality/appium/mobile/actions/AndroidActions.java (1)
src/main/java/aquality/appium/mobile/application/AqualityServices.java (1)
AqualityServices(15-213)
src/test/java/samples/android/nativeapp/apidemos/screens/AndroidScreen.java (1)
src/main/java/aquality/appium/mobile/application/AqualityServices.java (1)
AqualityServices(15-213)
🔇 Additional comments (15)
LICENSE (1)
189-189: Confirm license year consistency across the codebaseI ran an automated search for any occurrences of
Copyright 20[0-9]{2} Aquality Automationoutside of
LICENSEand found no mismatches—suggesting all headers are already at 2025. To be absolutely sure, please manually verify by running:rg -n "Copyright 20[0-9]{2} Aquality Automation" --glob "!LICENSE" | grep -v "2025"pom.xml (1)
187-187: Verify downstream compatibility for upgraded core & test libraries
aquality-selenium-core→ 4.7.0 andcommons-lang3→ 3.18.0 look like safe minor bumps, but they may introduce API deprecations or tightened transitive‐dependency ranges.Please:
- Run the full CI (unit + integration) on a clean workspace.
- Check for warnings about removed/deprecated methods (e.g.
RandomStringUtilschanges in recentcommons-lang3).- Confirm that the updated core still aligns to the new Selenium 4.34.0 version declared elsewhere in the PR.
No changes required if everything compiles and tests green.
Also applies to: 206-206
src/main/resources/localization/pl.json (1)
16-17: LGTM - Localization entries are properly formatted and positioned.The new Polish localization entries for script execution logging follow the established naming conventions and formatting patterns. The translations are appropriately placed after other application-related entries.
src/main/java/aquality/appium/mobile/application/MobileModule.java (2)
4-4: Import added correctly for new Android actions interface.The import for
IAndroidActionsis properly added to support the new binding.
35-35: DI binding follows established pattern.The binding for
IAndroidActionsto its implementation follows the same pattern as other action bindings in the module and is positioned logically after the touch actions binding.src/main/java/aquality/appium/mobile/actions/IActionsModule.java (1)
17-22: Well-structured addition following established patterns.The new
getAndroidActionsImplementation()method follows the exact same pattern as the existinggetTouchActionsImplementation()method, including proper JavaDoc documentation and consistent naming conventions.src/main/java/aquality/appium/mobile/application/AqualityServices.java (2)
3-3: Import correctly added for Android actions interface.The import for
IAndroidActionsis properly positioned and supports the new service accessor method.
205-212: Service accessor method follows established patterns.The new
getAndroidActions()method maintains consistency with other service accessor methods in the class, including proper JavaDoc documentation and using the genericget()method for dependency resolution.src/test/java/samples/android/nativeapp/apidemos/screens/AndroidScreen.java (1)
15-15: ConfirmstartActivitysignature in AqualityServicesI couldn’t locate the
startActivity(String, String, boolean)implementation in the repo, so please verify in the AqualityServices library that:
AqualityServices.getAndroidActions().startActivity(...)indeed accepts(String appPackage, String appActivity, boolean flag)- The
falseflag preserves the original behaviorsrc/main/java/aquality/appium/mobile/application/IMobileApplication.java (2)
11-12: LGTM: Well-designed import additions.The new imports for
CollectionsandMapare correctly added to support the new script execution methods.
68-83: LGTM: Excellent interface design for script execution.The new
executeScriptmethods follow Java best practices:
- Generic return type
<T>provides type flexibility- Clear parameter naming and documentation
- Default method appropriately delegates to the main implementation with an empty map
- Method signatures are intuitive and consistent with the existing interface style
This provides a clean abstraction for script execution functionality.
azure-pipelines.yml (2)
12-12: LGTM: Task version upgrades improve pipeline robustness.The upgrades to SonarCloud tasks (v1→v3) and Maven task (v3→v4) bring latest features and security improvements. The explicit JDK specification and polling timeout are good additions.
Also applies to: 21-21, 35-35, 41-41
29-29: Verify JDK version format consistency.The JDK version changed from '11' to '1.11' in the sonar job, but remains '11' in the test job. Ensure this format difference is intentional and compatible with the newer Maven task version.
src/main/java/aquality/appium/mobile/actions/AndroidActions.java (1)
12-17: LGTM: Well-structured activity start implementation.The logging, parameter construction, and script execution are well-implemented. The intent string formatting follows Android conventions.
src/main/java/aquality/appium/mobile/actions/IAndroidActions.java (1)
5-36: LGTM: Excellent interface design for Android actions.This interface demonstrates excellent design principles:
- Clear, comprehensive documentation for all methods
- Logical method overloading providing convenience while maintaining flexibility
- Sensible default behavior (stopping current app when starting new activity)
- Proper separation between convenience methods and core functionality
- Clean delegation pattern in default methods
The interface provides a clean abstraction for Android-specific actions while maintaining type safety and usability.
github-project-automation
bot
moved this from In progress
to Review in progress
in Aquality Appium Mobile
github-project-automation
bot
moved this from Review in progress
to Done
in Aquality Appium Mobile
2 participants
Execute script and Android startActivity action
Update Selenium reference to v.4.34.0
Update license year