feat/CUS-11616-Added class verify the text is present on screen in if condition#367
Conversation
📝 WalkthroughWalkthroughThe PR updates Maven dependencies and version numbers in the module, refactors Appium context-switching calls from execute-based commands to the SupportsContextSwitching interface, simplifies ContextUtils methods to delegate directly to the interface, adds a new Windows IF-condition action for OCR-based text verification, and applies formatting and logging adjustments to existing action classes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (5)
image_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImage.java (1)
3-3: Remove unused imports.Same as in
ClickOnImageWithThreshold.java, the following imports are no longer used after the refactor:
com.google.common.collect.ImmutableMap(line 3)org.openqa.selenium.remote.DriverCommand(line 17)🧹 Proposed fix to remove unused imports
-import com.google.common.collect.ImmutableMap; import com.testsigma.sdk.ApplicationType;-import org.openqa.selenium.remote.DriverCommand;Also applies to: 17-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@image_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImage.java` at line 3, The file ClickOnImage.java contains unused imports; remove the import declarations for com.google.common.collect.ImmutableMap and org.openqa.selenium.remote.DriverCommand from the top of the class (the unused symbols relate to the ClickOnImage class), then rebuild/organize imports to ensure no other unused imports remain and compilation still succeeds.image_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImageWithThreshold.java (1)
3-3: Remove unused imports.After refactoring to use
SupportsContextSwitching.context(), the following imports are no longer used:
com.google.common.collect.ImmutableMap(line 3)org.openqa.selenium.remote.DriverCommand(line 17)🧹 Proposed fix to remove unused imports
-import com.google.common.collect.ImmutableMap; import com.testsigma.sdk.ApplicationType;-import org.openqa.selenium.remote.DriverCommand;Also applies to: 17-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@image_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImageWithThreshold.java` at line 3, Remove the now-unused imports in ClickOnImageWithThreshold: delete the import statements for com.google.common.collect.ImmutableMap and org.openqa.selenium.remote.DriverCommand which are no longer needed after switching to SupportsContextSwitching.context(); update the import list in that class to only include actually used types so the file compiles without unused-import warnings.image_based_actions/pom.xml (2)
20-20: Inconsistent Lombok version declaration.The
lombok.versionproperty is defined on line 20 but not used in the dependency declaration on line 33, which hardcodes the version.🔧 Proposed fix to use the property
<dependency> <groupId>org.projectlombok</groupId> <artifactId>lombok</artifactId> - <version>1.18.30</version> + <version>${lombok.version}</version> <optional>true</optional> </dependency>Also applies to: 31-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@image_based_actions/pom.xml` at line 20, The pom defines a property named lombok.version but the lombok dependency still uses a hardcoded version; update the org.projectlombok:lombok dependency declaration in the POM (artifactId "lombok") to reference the property (${lombok.version}) instead of the hardcoded value and ensure the <lombok.version> property remains defined so all lombok references use the single property.
85-86: Disabling jar minimization will increase artifact size.Setting
<minimizeJar>false</minimizeJar>is likely necessary to ensure thatSupportsContextSwitchingand other Appium classes accessed via casting are retained in the shaded JAR. However, this may significantly increase the final artifact size.If JAR size becomes a concern, consider using explicit
<includes>filters to keep only the required Appium classes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@image_based_actions/pom.xml` around lines 85 - 86, The pom currently sets <minimizeJar>false</minimizeJar> which prevents shading from removing unused classes and will inflate the artifact; to address this, either keep minimizeJar=false if you must preserve Appium runtime casts (e.g., SupportsContextSwitching) or re-enable minimization and add explicit includes for only the required Appium classes (use the shade plugin's <includes> filter to whitelist SupportsContextSwitching and any other Appium types accessed via casting) so the shaded JAR retains necessary classes but reduces artifact size.image_based_actions/src/main/java/com/testsigma/addons/web/ClickOnTextSystemOrDesktop.java (1)
93-95: Log exceptions directly to the logger instead of converting to string.The current approach using
Arrays.toString(e.getStackTrace())loses important context like the exception message, class name, and cause chaining. The best practice is to pass the exception directly to the logger, which automatically captures the full stack trace:} catch (Exception e) { - logger.info("Exception: " + Arrays.toString(e.getStackTrace())); + logger.info("Exception occurred while searching for the given text", e); setErrorMessage("Exception occurred while searching for the given text"); result = Result.FAILED; }If you need the stack trace as a string for other purposes,
ExceptionUtils.getStackTrace(e)from Apache Commons Lang is preferable to the current approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@image_based_actions/src/main/java/com/testsigma/addons/web/ClickOnTextSystemOrDesktop.java` around lines 93 - 95, In ClickOnTextSystemOrDesktop replace the current catch block that logs Arrays.toString(e.getStackTrace()) with a proper logger call that passes the exception object (e) directly (e.g., logger.error(...) with the Throwable) so the logger records the exception class, message and full stack trace; keep the existing setErrorMessage("Exception occurred while searching for the given text") but ensure the logger call includes context text and the exception; if you need the stack as a string elsewhere, use ExceptionUtils.getStackTrace(e) from Apache Commons Lang instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@image_based_actions/src/main/java/com/testsigma/addons/web/ClickOnTextSystemOrDesktop.java`:
- Around line 115-118: The printAllCoordinates method should guard against a
null or empty textPoints list to prevent NPEs when OCR returns null; update
printAllCoordinates to check if textPoints is null or empty and return early
(optionally log a debug/info message) instead of iterating, and also defensively
handle any null OCRTextPoint entries (check textPoint != null before calling
getText/getX1/getY1/getX2/getY2) to avoid further null-pointer issues.
In
`@image_based_actions/src/main/java/com/testsigma/addons/windows/VerifyTextOnImageIfCondition.java`:
- Around line 48-53: The code in VerifyTextOnImageIfCondition creates temporary
JPGs (variables filename/path and ImageIO.write calls) and never removes them;
modify both occurrences (the block using tmp/tempDir/filename/path and the other
block at 73-79) to create and manage a proper temp File (e.g.,
File.createTempFile or a File object) and ensure it is deleted after use by
wrapping ImageIO.write and subsequent processing in a try/finally (or use
file.deleteOnExit() as a fallback) and calling file.delete() in the finally so
temp files are cleaned up on both success and failure.
- Around line 105-108: The printAllCoordinates method currently assumes
textPoints is non-null and will throw if OCR returned null; update
printAllCoordinates to first check that the List<OCRTextPoint> textPoints is not
null (and optionally not empty) and return early or log a clear message if it is
null, then proceed to iterate and log each OCRTextPoint (using
textPoint.getText(), getX1(), getY1(), getX2(), getY2()); ensure the null-guard
handles both null and empty cases so the loop never executes on a null
reference.
- Around line 92-98: The predicate in getTextPointFromText is reversed leading
to false positives (e.g., input "Submit" matches OCR fragment "Sub"); update the
condition to check whether the OCR fragment contains the input (use
textPoint.getText().contains(text)) instead of
text.contains(textPoint.getText()), and normalize comparisons (trim and
toLowerCase) or use equalsIgnoreCase if you need exact matches; keep the method
getTextPointFromText and OCRTextPoint.getText() as the locus of the fix.
---
Nitpick comments:
In `@image_based_actions/pom.xml`:
- Line 20: The pom defines a property named lombok.version but the lombok
dependency still uses a hardcoded version; update the org.projectlombok:lombok
dependency declaration in the POM (artifactId "lombok") to reference the
property (${lombok.version}) instead of the hardcoded value and ensure the
<lombok.version> property remains defined so all lombok references use the
single property.
- Around line 85-86: The pom currently sets <minimizeJar>false</minimizeJar>
which prevents shading from removing unused classes and will inflate the
artifact; to address this, either keep minimizeJar=false if you must preserve
Appium runtime casts (e.g., SupportsContextSwitching) or re-enable minimization
and add explicit includes for only the required Appium classes (use the shade
plugin's <includes> filter to whitelist SupportsContextSwitching and any other
Appium types accessed via casting) so the shaded JAR retains necessary classes
but reduces artifact size.
In
`@image_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImage.java`:
- Line 3: The file ClickOnImage.java contains unused imports; remove the import
declarations for com.google.common.collect.ImmutableMap and
org.openqa.selenium.remote.DriverCommand from the top of the class (the unused
symbols relate to the ClickOnImage class), then rebuild/organize imports to
ensure no other unused imports remain and compilation still succeeds.
In
`@image_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImageWithThreshold.java`:
- Line 3: Remove the now-unused imports in ClickOnImageWithThreshold: delete the
import statements for com.google.common.collect.ImmutableMap and
org.openqa.selenium.remote.DriverCommand which are no longer needed after
switching to SupportsContextSwitching.context(); update the import list in that
class to only include actually used types so the file compiles without
unused-import warnings.
In
`@image_based_actions/src/main/java/com/testsigma/addons/web/ClickOnTextSystemOrDesktop.java`:
- Around line 93-95: In ClickOnTextSystemOrDesktop replace the current catch
block that logs Arrays.toString(e.getStackTrace()) with a proper logger call
that passes the exception object (e) directly (e.g., logger.error(...) with the
Throwable) so the logger records the exception class, message and full stack
trace; keep the existing setErrorMessage("Exception occurred while searching for
the given text") but ensure the logger call includes context text and the
exception; if you need the stack as a string elsewhere, use
ExceptionUtils.getStackTrace(e) from Apache Commons Lang instead.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5daeed19-f374-454a-b840-eedf76a7f9c2
📒 Files selected for processing (8)
image_based_actions/pom.xmlimage_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImage.javaimage_based_actions/src/main/java/com/testsigma/addons/mobile_web/ClickOnImageWithThreshold.javaimage_based_actions/src/main/java/com/testsigma/addons/mobile_web/util/ContextUtils.javaimage_based_actions/src/main/java/com/testsigma/addons/web/ClickOnTextSystemOrDesktop.javaimage_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImage.javaimage_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImageWithThreshold.javaimage_based_actions/src/main/java/com/testsigma/addons/windows/VerifyTextOnImageIfCondition.java
| private void printAllCoordinates(List<OCRTextPoint> textPoints) { | ||
| for(OCRTextPoint textPoint: textPoints) { | ||
| logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1() + ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() +"\n\n\n\n"); | ||
| for (OCRTextPoint textPoint : textPoints) { | ||
| logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1() | ||
| + ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() + "\n\n\n\n"); |
There was a problem hiding this comment.
Add a null/empty guard before iterating OCR points.
printAllCoordinates can throw when OCR returns null; that failure is avoidable and currently routes through generic exception handling.
Suggested fix
private void printAllCoordinates(List<OCRTextPoint> textPoints) {
+ if (textPoints == null || textPoints.isEmpty()) {
+ logger.info("No OCR text points found");
+ return;
+ }
for (OCRTextPoint textPoint : textPoints) {
logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1()
+ ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() + "\n\n\n\n");
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@image_based_actions/src/main/java/com/testsigma/addons/web/ClickOnTextSystemOrDesktop.java`
around lines 115 - 118, The printAllCoordinates method should guard against a
null or empty textPoints list to prevent NPEs when OCR returns null; update
printAllCoordinates to check if textPoints is null or empty and return early
(optionally log a debug/info message) instead of iterating, and also defensively
handle any null OCRTextPoint entries (check textPoint != null before calling
getText/getX1/getY1/getX2/getY2) to avoid further null-pointer issues.
| String tempDir = System.getProperty("java.io.tmpdir"); | ||
| String filename = "screenshot"+System.currentTimeMillis()+".jpg"; | ||
| String path = tempDir + filename; | ||
|
|
||
| // To copy source image in to destination path | ||
| ImageIO.write(tmp, "jpg",new File(path)); |
There was a problem hiding this comment.
Temporary screenshot files are never cleaned up.
This action creates temp JPGs on every run and leaves them behind, which can accumulate and exhaust disk over time in long test runs.
Suggested fix
- String filename = "screenshot"+System.currentTimeMillis()+".jpg";
- String path = tempDir + filename;
+ String filename = "screenshot"+System.currentTimeMillis()+".jpg";
+ String path = tempDir + filename;
+ File firstScreenshotFile = new File(path);
...
- ImageIO.write(tmp, "jpg",new File(path));
+ ImageIO.write(tmp, "jpg", firstScreenshotFile);
...
- File baseImageFile = new File(path);
+ File baseImageFile = firstScreenshotFile;
...
- ImageIO.write(tmp, "jpg",new File(path));
- baseImageFile = new File(path);
+ File secondScreenshotFile = new File(path);
+ ImageIO.write(tmp, "jpg", secondScreenshotFile);
+ baseImageFile = secondScreenshotFile;
String url = testStepResult.getScreenshotUrl();
ocr.uploadFile(url, baseImageFile);
+ if (!secondScreenshotFile.delete()) {
+ logger.info("Unable to delete temp screenshot: " + secondScreenshotFile.getAbsolutePath());
+ }
setSuccessMessage("Text is found at " +
" Text coordinates :" + "x1-" + textPoint.getX1() + ", x2-" + textPoint.getX2() + ", y1-" + textPoint.getY1() + ", y2-" + textPoint.getY2());
}
+ if (!firstScreenshotFile.delete()) {
+ logger.info("Unable to delete temp screenshot: " + firstScreenshotFile.getAbsolutePath());
+ }Also applies to: 73-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@image_based_actions/src/main/java/com/testsigma/addons/windows/VerifyTextOnImageIfCondition.java`
around lines 48 - 53, The code in VerifyTextOnImageIfCondition creates temporary
JPGs (variables filename/path and ImageIO.write calls) and never removes them;
modify both occurrences (the block using tmp/tempDir/filename/path and the other
block at 73-79) to create and manage a proper temp File (e.g.,
File.createTempFile or a File object) and ensure it is deleted after use by
wrapping ImageIO.write and subsequent processing in a try/finally (or use
file.deleteOnExit() as a fallback) and calling file.delete() in the finally so
temp files are cleaned up on both success and failure.
| private OCRTextPoint getTextPointFromText(List<OCRTextPoint> textPoints,String text) { | ||
| if(textPoints == null) { | ||
| return null; | ||
| } | ||
| for(OCRTextPoint textPoint: textPoints) { | ||
| if(text.contains(textPoint.getText())) { | ||
| return textPoint; |
There was a problem hiding this comment.
Text-match predicate is reversed and can return false positives.
Current logic marks success when any OCR fragment is contained in input text (e.g., input "Submit" matches OCR "Sub"). For IF conditions, this can incorrectly pass.
Suggested fix
- private OCRTextPoint getTextPointFromText(List<OCRTextPoint> textPoints,String text) {
+ private OCRTextPoint getTextPointFromText(List<OCRTextPoint> textPoints, String text) {
if(textPoints == null) {
return null;
}
for(OCRTextPoint textPoint: textPoints) {
- if(text.contains(textPoint.getText())) {
+ String ocrText = textPoint.getText() == null ? "" : textPoint.getText().replaceAll("\\s+", "");
+ if(ocrText.contains(text)) {
return textPoint;
}
}
return null;
}📝 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.
| private OCRTextPoint getTextPointFromText(List<OCRTextPoint> textPoints,String text) { | |
| if(textPoints == null) { | |
| return null; | |
| } | |
| for(OCRTextPoint textPoint: textPoints) { | |
| if(text.contains(textPoint.getText())) { | |
| return textPoint; | |
| private OCRTextPoint getTextPointFromText(List<OCRTextPoint> textPoints, String text) { | |
| if(textPoints == null) { | |
| return null; | |
| } | |
| for(OCRTextPoint textPoint: textPoints) { | |
| String ocrText = textPoint.getText() == null ? "" : textPoint.getText().replaceAll("\\s+", ""); | |
| if(ocrText.contains(text)) { | |
| return textPoint; | |
| } | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@image_based_actions/src/main/java/com/testsigma/addons/windows/VerifyTextOnImageIfCondition.java`
around lines 92 - 98, The predicate in getTextPointFromText is reversed leading
to false positives (e.g., input "Submit" matches OCR fragment "Sub"); update the
condition to check whether the OCR fragment contains the input (use
textPoint.getText().contains(text)) instead of
text.contains(textPoint.getText()), and normalize comparisons (trim and
toLowerCase) or use equalsIgnoreCase if you need exact matches; keep the method
getTextPointFromText and OCRTextPoint.getText() as the locus of the fix.
| private void printAllCoordinates(List<OCRTextPoint> textPoints) { | ||
| for(OCRTextPoint textPoint: textPoints) { | ||
| logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1() + ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() +"\n\n\n\n"); | ||
| } |
There was a problem hiding this comment.
Guard against null OCR lists before logging coordinates.
If OCR returns null, this loop throws and falls into the generic exception path unnecessarily.
Suggested fix
private void printAllCoordinates(List<OCRTextPoint> textPoints) {
+ if (textPoints == null || textPoints.isEmpty()) {
+ logger.info("No OCR text points found");
+ return;
+ }
for(OCRTextPoint textPoint: textPoints) {
logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1() + ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() +"\n\n\n\n");
}
}📝 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.
| private void printAllCoordinates(List<OCRTextPoint> textPoints) { | |
| for(OCRTextPoint textPoint: textPoints) { | |
| logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1() + ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() +"\n\n\n\n"); | |
| } | |
| private void printAllCoordinates(List<OCRTextPoint> textPoints) { | |
| if (textPoints == null || textPoints.isEmpty()) { | |
| logger.info("No OCR text points found"); | |
| return; | |
| } | |
| for(OCRTextPoint textPoint: textPoints) { | |
| logger.info("text =" + textPoint.getText() + "x1 = " + textPoint.getX1() + ", y1 =" + textPoint.getY1() + ", x2 = " + textPoint.getX2() + ", y2 =" + textPoint.getY2() +"\n\n\n\n"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@image_based_actions/src/main/java/com/testsigma/addons/windows/VerifyTextOnImageIfCondition.java`
around lines 105 - 108, The printAllCoordinates method currently assumes
textPoints is non-null and will throw if OCR returned null; update
printAllCoordinates to first check that the List<OCRTextPoint> textPoints is not
null (and optionally not empty) and return early or log a clear message if it is
null, then proceed to iterate and log each OCRTextPoint (using
textPoint.getText(), getX1(), getY1(), getX2(), getY2()); ensure the null-guard
handles both null and empty cases so the loop never executes on a null
reference.
Publish this addon as PUBLIC
Addon Name: Image Based Actions
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-11616
Added class verify the text is present on screen in if condition
Summary by CodeRabbit
New Features
Chores
Style
Refactor