[CUS-10319] Addon nlp to open deep link.#316
Conversation
|
|
📝 WalkthroughWalkthroughThis pull request introduces a new deeplink addon artifact for the Testsigma testing framework with Maven configuration and action implementations for opening deep links in Android and iOS mobile applications via Appium's mobile:deepLink script. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@deeplink_action`
1/src/main/java/com/testsigma/addons/android/OpenDeepLink.java:
- Around line 14-16: The actionText in the `@Action` annotation on class
OpenDeepLink mentions a parameter "package-name" that isn't defined; either
remove "package-name" from the actionText or add a `@TestData` field to accept it.
Update the `@Action`(actionText = ...) to stop referencing "package-name" if you
intend to use the existing capability value appPackage, or add a new
`@TestData-annotated` field (e.g., String packageName) and use that in the
implementation instead of appPackage; ensure references in OpenDeepLink's
execute logic (appPackage usage) are adjusted to use the new field if you add
it.
- Line 29: The assignment to deepLink using testData.getValue().toString() in
OpenDeepLink may throw NPE if testData.getValue() is null; update the code to
null-check the value from testData before calling toString() (e.g., retrieve
Object val = testData.getValue(); if val == null handle appropriately—throw a
meaningful exception or set deepLink to an empty string/default and log a
warning) and use val.toString() only when non-null so the deepLink variable and
downstream logic are safe.
- Around line 34-40: The code may NPE when appPackage is null; update
OpenDeepLink.java to null-check the result of
androidDriver.getCapabilities().getCapability("appium:appPackage") (variable
appPackage) before using it in ImmutableMap.of() or in logs, cast it to a String
safely, and build the arguments map conditionally (include the "package" entry
only if appPackage != null) when calling androidDriver.executeScript("mobile:
deepLink", ...); also add a safe log message indicating the package is missing
or the fallback used so execution is explicit.
In `@deeplink_action` 1/src/main/java/com/testsigma/addons/ios/OpenDeepLink.java:
- Around line 52-54: The format string in OpenDeepLink.setErrorMessage call has
four %s placeholders but only three arguments (deepLink, bundleId,
e.getMessage()), causing IllegalFormatException; either remove the extra "%s" so
the message uses three placeholders or add the missing fourth argument (e.g.,
bundleId again) to the String.format call in the setErrorMessage invocation
inside the OpenDeepLink class/method.
- Line 31: The assignment String deepLink = testData.getValue().toString(); can
NPE if testData.getValue() is null; update the OpenDeepLink code that creates
deepLink (reference testData and variable deepLink) to safely handle null by
checking testData != null and testData.getValue() != null (or use
Objects.toString(testData.getValue(), "") / String.valueOf(testData.getValue()))
before calling toString(), and either throw a clear exception or use a
default/empty string so downstream logic won’t NPE.
- Line 34: The call to
iosDriver.getCapabilities().getCapability("appium:bundleId") in
OpenDeepLink.java may return null and will cause problems when passed to
ImmutableMap.of(); update the code in the OpenDeepLink class (where bundleId is
retrieved) to null-check the capability before building the ImmutableMap (or
provide a sensible default or throw a clear IllegalArgumentException), and only
include "bundleId" in the map when non-null so ImmutableMap.of(...) is never
invoked with a null value.
- Around line 35-42: In OpenDeepLink, the block that detects a missing URL
scheme both uses SLF4J-style "{}" placeholders with String.format and never
actually constructs or returns a corrected URL; change the String.format calls
to use "%s" placeholders, build a corrected deepLink string by prepending the
app identifier (e.g., bundleId + "://" + deepLink) and assign it back to the
variable used later, and replace the setErrorMessage (or convert it to a
non-error log like logger.info) so execution continues with the constructed URL;
ensure any callers that expect a failure instead are handled by
returning/throwing if you choose that alternative.
🧹 Nitpick comments (4)
deeplink_action 1/pom.xml (2)
17-17: Use a stable JUnit version instead of milestone release.
5.8.0-M1is a milestone/pre-release version. Consider using a stable release like5.10.0or later for production code.Suggested fix
- <junit.jupiter.version>5.8.0-M1</junit.jupiter.version> + <junit.jupiter.version>5.10.0</junit.jupiter.version>
42-46: TestNG version 6.14.3 is outdated.TestNG 6.14.3 is from 2018. Consider upgrading to the 7.x series for better compatibility and bug fixes. Also, having both JUnit Jupiter and TestNG as dependencies may cause confusion—consider using only one testing framework.
deeplink_action 1/src/main/java/com/testsigma/addons/android/OpenDeepLink.java (1)
50-52: Unusedresultvariable.The
resultvariable initialized at line 25 is never modified and can be replaced with a direct return.Suggested simplification
- Result result = Result.SUCCESS; logger.info("Initiating execution"); ... setSuccessMessage(String.format("Successfully opened deep link %s for package %s", deepLink, appPackage)); - return result; + return Result.SUCCESS;deeplink_action 1/src/main/java/com/testsigma/addons/ios/OpenDeepLink.java (1)
58-59: Inconsistent terminology: uses "package" instead of "bundleId" in iOS context.The success message refers to "package" which is Android terminology. For iOS, it should say "bundleId" for consistency.
Suggested fix
- setSuccessMessage(String.format("Successfully opened deep link %s for package %s", + setSuccessMessage(String.format("Successfully opened deep link %s for bundleId %s", deepLink, bundleId));
| @Action(actionText = "Open the deep link test-data for the package package-name", | ||
| applicationType = ApplicationType.ANDROID, | ||
| useCustomScreenshot = false) |
There was a problem hiding this comment.
Action text references package-name but no corresponding input is defined.
The actionText mentions package-name as a parameter, but there's no @TestData annotation for it. The implementation uses appPackage from capabilities instead. Either remove package-name from the action text or add a corresponding input field.
Suggested fix - update action text to match implementation
-@Action(actionText = "Open the deep link test-data for the package package-name",
+@Action(actionText = "Open the deep link test-data",
applicationType = ApplicationType.ANDROID,
useCustomScreenshot = false)🤖 Prompt for AI Agents
In `@deeplink_action`
1/src/main/java/com/testsigma/addons/android/OpenDeepLink.java around lines 14 -
16, The actionText in the `@Action` annotation on class OpenDeepLink mentions a
parameter "package-name" that isn't defined; either remove "package-name" from
the actionText or add a `@TestData` field to accept it. Update the
`@Action`(actionText = ...) to stop referencing "package-name" if you intend to
use the existing capability value appPackage, or add a new `@TestData-annotated`
field (e.g., String packageName) and use that in the implementation instead of
appPackage; ensure references in OpenDeepLink's execute logic (appPackage usage)
are adjusted to use the new field if you add it.
| logger.info("Initiating execution"); | ||
| AndroidDriver androidDriver = (AndroidDriver) this.driver; | ||
|
|
||
| String deepLink = testData.getValue().toString(); |
There was a problem hiding this comment.
Potential NullPointerException if testData value is null.
testData.getValue() could return null. Consider adding null validation before calling toString().
Suggested fix
- String deepLink = testData.getValue().toString();
+ Object value = testData.getValue();
+ if (value == null || value.toString().isEmpty()) {
+ setErrorMessage("Deep link test data cannot be null or empty");
+ return Result.FAILED;
+ }
+ String deepLink = value.toString();🤖 Prompt for AI Agents
In `@deeplink_action`
1/src/main/java/com/testsigma/addons/android/OpenDeepLink.java at line 29, The
assignment to deepLink using testData.getValue().toString() in OpenDeepLink may
throw NPE if testData.getValue() is null; update the code to null-check the
value from testData before calling toString() (e.g., retrieve Object val =
testData.getValue(); if val == null handle appropriately—throw a meaningful
exception or set deepLink to an empty string/default and log a warning) and use
val.toString() only when non-null so the deepLink variable and downstream logic
are safe.
| Object appPackage = androidDriver.getCapabilities().getCapability("appium:appPackage"); | ||
|
|
||
| try { | ||
| androidDriver.executeScript("mobile: deepLink", ImmutableMap.of( | ||
| "url", deepLink, | ||
| "package", appPackage | ||
| )); |
There was a problem hiding this comment.
Potential NullPointerException if appium:appPackage capability is not set.
getCapability("appium:appPackage") can return null if the capability is not present, which would cause issues when passed to ImmutableMap.of() or logged.
Suggested fix
Object appPackage = androidDriver.getCapabilities().getCapability("appium:appPackage");
+ if (appPackage == null) {
+ appPackage = currentPackage; // fallback to current package
+ logger.info("appium:appPackage capability not found, using current package: " + appPackage);
+ }
try {🤖 Prompt for AI Agents
In `@deeplink_action`
1/src/main/java/com/testsigma/addons/android/OpenDeepLink.java around lines 34 -
40, The code may NPE when appPackage is null; update OpenDeepLink.java to
null-check the result of
androidDriver.getCapabilities().getCapability("appium:appPackage") (variable
appPackage) before using it in ImmutableMap.of() or in logs, cast it to a String
safely, and build the arguments map conditionally (include the "package" entry
only if appPackage != null) when calling androidDriver.executeScript("mobile:
deepLink", ...); also add a safe log message indicating the package is missing
or the fallback used so execution is explicit.
| logger.info("Initiating execution"); | ||
| IOSDriver iosDriver = (IOSDriver) this.driver; | ||
|
|
||
| String deepLink = testData.getValue().toString(); |
There was a problem hiding this comment.
Potential NullPointerException if testData value is null.
Same issue as the Android version - testData.getValue() could return null.
Suggested fix
- String deepLink = testData.getValue().toString();
+ Object value = testData.getValue();
+ if (value == null || value.toString().isEmpty()) {
+ setErrorMessage("Deep link test data cannot be null or empty");
+ return Result.FAILED;
+ }
+ String deepLink = value.toString();📝 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.
| String deepLink = testData.getValue().toString(); | |
| Object value = testData.getValue(); | |
| if (value == null || value.toString().isEmpty()) { | |
| setErrorMessage("Deep link test data cannot be null or empty"); | |
| return Result.FAILED; | |
| } | |
| String deepLink = value.toString(); |
🤖 Prompt for AI Agents
In `@deeplink_action` 1/src/main/java/com/testsigma/addons/ios/OpenDeepLink.java
at line 31, The assignment String deepLink = testData.getValue().toString(); can
NPE if testData.getValue() is null; update the OpenDeepLink code that creates
deepLink (reference testData and variable deepLink) to safely handle null by
checking testData != null and testData.getValue() != null (or use
Objects.toString(testData.getValue(), "") / String.valueOf(testData.getValue()))
before calling toString(), and either throw a clear exception or use a
default/empty string so downstream logic won’t NPE.
| String deepLink = testData.getValue().toString(); | ||
| logger.info("Opening deep link: " + deepLink); | ||
|
|
||
| Object bundleId = iosDriver.getCapabilities().getCapability("appium:bundleId"); |
There was a problem hiding this comment.
Potential NullPointerException if appium:bundleId capability is not set.
getCapability("appium:bundleId") can return null, which would cause issues when passed to ImmutableMap.of().
Suggested fix
Object bundleId = iosDriver.getCapabilities().getCapability("appium:bundleId");
+ if (bundleId == null) {
+ setErrorMessage("appium:bundleId capability is required but not found");
+ return Result.FAILED;
+ }📝 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.
| Object bundleId = iosDriver.getCapabilities().getCapability("appium:bundleId"); | |
| Object bundleId = iosDriver.getCapabilities().getCapability("appium:bundleId"); | |
| if (bundleId == null) { | |
| setErrorMessage("appium:bundleId capability is required but not found"); | |
| return Result.FAILED; | |
| } |
🤖 Prompt for AI Agents
In `@deeplink_action` 1/src/main/java/com/testsigma/addons/ios/OpenDeepLink.java
at line 34, The call to
iosDriver.getCapabilities().getCapability("appium:bundleId") in
OpenDeepLink.java may return null and will cause problems when passed to
ImmutableMap.of(); update the code in the OpenDeepLink class (where bundleId is
retrieved) to null-check the capability before building the ImmutableMap (or
provide a sensible default or throw a clear IllegalArgumentException), and only
include "bundleId" in the map when non-null so ImmutableMap.of(...) is never
invoked with a null value.
| if (!deepLink.contains("://")) { | ||
| // No scheme found, construct using appIdentifier as custom URL scheme | ||
| logger.debug(String.format("Deep link missing scheme, constructing URL " + | ||
| "with appIdentifier: {}://{}", bundleId, deepLink)); | ||
| setErrorMessage(String.format("Deep link missing scheme, constructing URL " + | ||
| "with appIdentifier: {}://{}", bundleId, deepLink)); | ||
|
|
||
| } |
There was a problem hiding this comment.
Incomplete handling for missing URL scheme and incorrect format specifiers.
Multiple issues in this block:
- Uses
{}(SLF4J style) instead of%sforString.format()- placeholders won't be substituted - Sets an error message but continues execution instead of returning or actually constructing the URL
- The comment says "construct using appIdentifier" but no construction happens
Suggested fix - either fail or construct the URL
if (!deepLink.contains("://")) {
// No scheme found, construct using appIdentifier as custom URL scheme
- logger.debug(String.format("Deep link missing scheme, constructing URL " +
- "with appIdentifier: {}://{}", bundleId, deepLink));
- setErrorMessage(String.format("Deep link missing scheme, constructing URL " +
- "with appIdentifier: {}://{}", bundleId, deepLink));
-
+ setErrorMessage(String.format("Deep link missing scheme. Expected format: scheme://path. Got: %s", deepLink));
+ return Result.FAILED;
}📝 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.
| if (!deepLink.contains("://")) { | |
| // No scheme found, construct using appIdentifier as custom URL scheme | |
| logger.debug(String.format("Deep link missing scheme, constructing URL " + | |
| "with appIdentifier: {}://{}", bundleId, deepLink)); | |
| setErrorMessage(String.format("Deep link missing scheme, constructing URL " + | |
| "with appIdentifier: {}://{}", bundleId, deepLink)); | |
| } | |
| if (!deepLink.contains("://")) { | |
| // No scheme found, construct using appIdentifier as custom URL scheme | |
| setErrorMessage(String.format("Deep link missing scheme. Expected format: scheme://path. Got: %s", deepLink)); | |
| return Result.FAILED; | |
| } |
🤖 Prompt for AI Agents
In `@deeplink_action` 1/src/main/java/com/testsigma/addons/ios/OpenDeepLink.java
around lines 35 - 42, In OpenDeepLink, the block that detects a missing URL
scheme both uses SLF4J-style "{}" placeholders with String.format and never
actually constructs or returns a corrected URL; change the String.format calls
to use "%s" placeholders, build a corrected deepLink string by prepending the
app identifier (e.g., bundleId + "://" + deepLink) and assign it back to the
variable used later, and replace the setErrorMessage (or convert it to a
non-error log like logger.info) so execution continues with the constructed URL;
ensure any callers that expect a failure instead are handled by
returning/throwing if you choose that alternative.
| setErrorMessage(String.format("Failed to open deep link %s for package %s. Error: %s .try with this " + | ||
| "package %s", | ||
| deepLink, bundleId, e.getMessage())); |
There was a problem hiding this comment.
String.format argument count mismatch - will throw exception at runtime.
The format string has 4 placeholders (%s) but only 3 arguments are provided (deepLink, bundleId, e.getMessage()). This will cause an IllegalFormatException at runtime.
Fix the format string
- setErrorMessage(String.format("Failed to open deep link %s for package %s. Error: %s .try with this " +
- "package %s",
- deepLink, bundleId, e.getMessage()));
+ setErrorMessage(String.format("Failed to open deep link %s for bundleId %s. Error: %s",
+ deepLink, bundleId, e.getMessage()));📝 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.
| setErrorMessage(String.format("Failed to open deep link %s for package %s. Error: %s .try with this " + | |
| "package %s", | |
| deepLink, bundleId, e.getMessage())); | |
| setErrorMessage(String.format("Failed to open deep link %s for bundleId %s. Error: %s", | |
| deepLink, bundleId, e.getMessage())); |
🤖 Prompt for AI Agents
In `@deeplink_action` 1/src/main/java/com/testsigma/addons/ios/OpenDeepLink.java
around lines 52 - 54, The format string in OpenDeepLink.setErrorMessage call has
four %s placeholders but only three arguments (deepLink, bundleId,
e.getMessage()), causing IllegalFormatException; either remove the extra "%s" so
the message uses three placeholders or add the missing fourth argument (e.g.,
bundleId again) to the String.format call in the setErrorMessage invocation
inside the OpenDeepLink class/method.
please review this addon and publish as PUBLIC
Addon name : DeepLink Action
Addon accont: https://jarvis-in.testsigma.com/ui/tenants/11/addons
Jira: https://testsigma.atlassian.net/browse/CUS-10319
fix
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.