Skip to content

Conversation

@akhil-testsigma
Copy link
Contributor

@akhil-testsigma akhil-testsigma commented Oct 21, 2025

Addon Name: Image Based Actions
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-8078
Added right click and mouse hover for windows application

Summary by CodeRabbit

  • New Features

    • Added image-based hover actions on Windows with standard and threshold-based matching, including occurrence-based variants
    • Added image-based right-click actions on Windows with standard and threshold-based matching, including occurrence-based variants
  • Bug Fixes

    • Updated exception handling to use standard library exceptions
  • Chores

    • Bumped version from 1.0.12 to 1.0.14
    • Updated Lombok dependency to 1.18.30
    • Enhanced Maven build configuration with shade plugin optimization

@coderabbitai
Copy link

coderabbitai bot commented Oct 21, 2025

Walkthrough

This PR bumps the artifact version from 1.0.12 to 1.0.14, updates Lombok to 1.18.30, adds Maven Shade Plugin configuration with minimizeJar and META-INF filters, modifies an import statement to use JDK's NoSuchElementException, and introduces eight new Windows action classes for image-based mouse interactions (hover and right-click) with optional threshold and occurrence-based detection variants.

Changes

Cohort / File(s) Summary
Build & Dependencies
image_based_actions/pom.xml
Artifact version bumped to 1.0.14; Lombok updated to 1.18.30; Maven Shade Plugin configured with minimizeJar enabled and META-INF signature file filters (SF, DSA, RSA) added.
Exception Import Fix
image_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImageOccurrenceBased.java
Changed import from org.openqa.selenium.NoSuchElementException to java.util.NoSuchElementException; exception handling path remains unchanged.
Image-Based Hover Actions
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImage.java, HoverOnImageOccurrenceBased.java, HoverOnImageWithThreshold.java, HoverOnImageWithThresholdOccurrenceBased.java
Four new Windows action classes added for mouse hover on detected images. Variants support basic image matching, occurrence-based detection, threshold-based matching, and combined threshold+occurrence modes. All capture screenshot, upload to OCR, locate image, compute centroid, and move mouse to center.
Image-Based Right-Click Actions
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImage.java, RightClickOnImageOccurrenceBased.java, RightClickOnImageWithThreshold.java, RightClickOnImageWithThresholdOccurrenceBased.java
Four new Windows action classes added for right-click on detected images. Mirrors hover action variants (basic, occurrence-based, threshold, threshold+occurrence), following identical screenshot-to-action flow but using right-click instead of mouse movement.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Action as Image Action<br/>(Hover/RightClick)
    participant Robot as Robot<br/>(Screenshot)
    participant OCR as OCR Service
    participant Mouse as Mouse<br/>(AWT)
    
    User->>Action: Execute with image-url<br/>(+ threshold/occurrence)
    Action->>Robot: captureScreen()
    Robot-->>Action: Screenshot bytes
    Action->>Action: Save temp file
    Action->>OCR: uploadFile(screenshot)
    OCR-->>Action: Upload URL
    Action->>OCR: findImage(url, threshold,<br/>occurrence)
    alt Image Found
        OCR-->>Action: Bounding box (x1,y1,x2,y2)
        Action->>Action: Compute centroid
        Action->>Mouse: moveMouse(centerX,<br/>centerY)
        Mouse-->>Action: Success
        alt Action Type
            Action->>Mouse: mousePress(BUTTON3)
            Note over Mouse: Right-click variant
            Mouse-->>Action: Success
        else Hover variant
            Note over Action: Movement complete
        end
        Action-->>User: Result.SUCCESS with<br/>bounding box details
    else Image Not Found
        OCR-->>Action: isFound = false
        Action-->>User: Result.FAILED with<br/>error message
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

The diff introduces eight new action classes with consistent but slightly varied patterns (hover vs. right-click, threshold-based vs. occurrence-based). Each class requires individual verification of OCR integration, coordinate calculations, and mouse action logic. While repetitive structure reduces cognitive load, the heterogeneity of variants (four distinct combinations) and integration points with external OCR services demand careful reasoning per implementation.

Suggested reviewers

  • vigneshtestsigma
  • Ganesh-Testsigma

Poem

🐰 Eight new actions hop in place,
Finding images all over the place!
With thresholds set and clicks to spare,
Mouse hovers floating through the air!
Shade and Lombok dance in time,
Image-based tests sublime!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "feat/CUS-8078-Added right click and mouse hover for windows application" directly corresponds to the primary changes in the changeset. The bulk of the modifications involve adding eight new Windows action classes: four for right-click functionality (RightClickOnImage, RightClickOnImageOccurrenceBased, RightClickOnImageWithThreshold, RightClickOnImageWithThresholdOccurrenceBased) and four for mouse hover functionality (HoverOnImage, HoverOnImageOccurrenceBased, HoverOnImageWithThreshold, HoverOnImageWithThresholdOccurrenceBased). Secondary changes like the pom.xml version bumps and the exception import update are supporting modifications. The title is specific, clear, and accurately captures the core intent of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/CUS-8078-Added-right-click-and-mouse-hover-for-windows-application

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
image_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImageOccurrenceBased.java (1)

67-67: Add validation for occurrence parameter.

Parsing testData2.getValue().toString() as an integer without validation can throw NumberFormatException if the input is invalid, causing the test step to fail with a generic exception message. Consider validating the input or providing a more descriptive error.

Apply this diff to add validation:

-int occurrence = Integer.parseInt(testData2.getValue().toString());
+String occurrenceStr = testData2.getValue().toString();
+int occurrence;
+try {
+    occurrence = Integer.parseInt(occurrenceStr);
+    if (occurrence < 1) {
+        throw new IllegalArgumentException("Occurrence must be a positive integer");
+    }
+} catch (NumberFormatException e) {
+    logger.error("Invalid occurrence value: " + occurrenceStr, e);
+    setErrorMessage("Invalid occurrence value: '" + occurrenceStr + "'. Expected a positive integer.");
+    return Result.FAILED;
+}
image_based_actions/pom.xml (1)

20-21: Align Lombok version property with dependency.

Property says 1.18.20 but dependency pins 1.18.30. Unify to property to avoid drift.

-        <lombok.version>1.18.20</lombok.version>
+        <lombok.version>1.18.30</lombok.version>
@@
-        <dependency>
-            <groupId>org.projectlombok</groupId>
-            <artifactId>lombok</artifactId>
-            <version>1.18.30</version>
+        <dependency>
+            <groupId>org.projectlombok</groupId>
+            <artifactId>lombok</artifactId>
+            <version>${lombok.version}</version>

Also applies to: 31-34

🧹 Nitpick comments (19)
image_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImageOccurrenceBased.java (1)

54-54: Use platform-independent path separator.

The path construction concatenates tempDir and filename directly. While many systems include a trailing separator in java.io.tmpdir, using File.separator or Paths.get() ensures cross-platform compatibility.

Apply this diff to improve path construction:

-String path = tempDir + filename;
+String path = tempDir + File.separator + filename;

Or use the modern approach:

+import java.nio.file.Paths;
...
-String path = tempDir + filename;
+String path = Paths.get(tempDir, filename).toString();
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageWithThreshold.java (3)

73-80: Clamp target coordinates to screen bounds.

Defensive check in case OCR returns out-of-bounds box on HiDPI/multi-screen setups.

-                int hoverX = (x1 + x2) / 2;
-                int hoverY = (y1 + y2) / 2;
+                int hoverX = Math.max(0, Math.min(screenSize.width - 1, (x1 + x2) / 2));
+                int hoverY = Math.max(0, Math.min(screenSize.height - 1, (y1 + y2) / 2));

83-84: Avoid Thread.sleep; handle interrupts if you must delay.

Hard sleeps slow suites and swallow interrupts.

-                Thread.sleep(1500);
+                robot.waitForIdle(); // or make delay configurable

If keeping sleep:

-                Thread.sleep(1500);
+                try { Thread.sleep(1500); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); }

88-92: Use error level for exceptions and include message.

Currently logs stack trace at INFO and loses cause in user message.

-            logger.info("Exception: " + ExceptionUtils.getStackTrace(e));
-            setErrorMessage("Exception occurred while performing mouse hover action.");
+            logger.error("Exception during mouse hover:", e);
+            setErrorMessage("Mouse hover failed: " + e.getMessage());
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageWithThresholdOccurrenceBased.java (2)

78-85: Clamp coordinates to screen bounds (optional but safer).

Same change as suggested in the threshold variant: bound hoverX/hoverY to screenSize.


88-89: Avoid Thread.sleep; handle interrupts. Use error log level.

Replicate the hover/sleep and logging fixes from the other file.

Also applies to: 93-97

image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImage.java (1)

70-77: Clamp coordinates; avoid Thread.sleep; log exceptions at error level.

Replicate the coordinate clamping, waitForIdle/interrupt-handling, and error logging changes from the threshold file.

Also applies to: 80-81, 85-89

image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThreshold.java (2)

73-81: Clamp coordinates; avoid arbitrary sleep; handle interrupts.

-                int clickX = (x1 + x2) / 2;
-                int clickY = (y1 + y2) / 2;
+                int clickX = Math.max(0, Math.min(screenSize.width - 1, (x1 + x2) / 2));
+                int clickY = Math.max(0, Math.min(screenSize.height - 1, (y1 + y2) / 2));
@@
-                Thread.sleep(500);
+                robot.waitForIdle();
+                // or if keeping a pause:
+                // try { Thread.sleep(500); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); }

90-94: Use error log level and propagate cause in message.

Same logging change as in the hover files.

image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThresholdOccurrenceBased.java (2)

50-52: Handle multi‑monitor virtual bounds.

Toolkit.getScreenSize() may capture only primary display; compute union of all screens.

-            Rectangle screenSize = new Rectangle(Toolkit.getDefaultToolkit().getScreenSize());
-            BufferedImage tmp = robot.createScreenCapture(screenSize);
+            Rectangle all = new Rectangle();
+            for (GraphicsDevice gd : GraphicsEnvironment.getLocalGraphicsEnvironment().getScreenDevices()) {
+                all = all.union(gd.getDefaultConfiguration().getBounds());
+            }
+            BufferedImage tmp = robot.createScreenCapture(all);

96-99: Log exceptions at error level and preserve interrupts.

Use structured logging and restore interrupt flag when applicable.

-            logger.info("Exception: " + ExceptionUtils.getStackTrace(e));
+            if (e instanceof InterruptedException) { Thread.currentThread().interrupt(); }
+            logger.error("Right-click on image failed", e);
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImage.java (3)

45-47: Support multi‑monitor captures.

-            Rectangle screenSize = new Rectangle(Toolkit.getDefaultToolkit().getScreenSize());
-            BufferedImage tmp = robot.createScreenCapture(screenSize);
+            Rectangle all = new Rectangle();
+            for (GraphicsDevice gd : GraphicsEnvironment.getLocalGraphicsEnvironment().getScreenDevices()) {
+                all = all.union(gd.getDefaultConfiguration().getBounds());
+            }
+            BufferedImage tmp = robot.createScreenCapture(all);

88-91: Use logger.error with throwable; keep interrupt state.

-            logger.info("Exception: " + ExceptionUtils.getStackTrace(e));
-            setErrorMessage("Exception occurred while performing right-click action on image.");
+            if (e instanceof InterruptedException) { Thread.currentThread().interrupt(); }
+            logger.error("Right-click on image failed", e);
+            setErrorMessage("Right-click failed: " + e.getClass().getSimpleName());

38-83: DRY: extract common capture/upload/find utilities.

The capture→tempfile→upload→find pattern is duplicated across classes. Factor into a small helper (e.g., ImageActions.captureAndUpload(ocr, testStepResult)) returning (tempPath, BufferedImage/FindImageResponse).

image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageOccurrenceBased.java (3)

47-49: Capture all monitors, not just primary.

-            Rectangle screenSize = new Rectangle(Toolkit.getDefaultToolkit().getScreenSize());
-            BufferedImage tmp = robot.createScreenCapture(screenSize);
+            Rectangle all = new Rectangle();
+            for (GraphicsDevice gd : GraphicsEnvironment.getLocalGraphicsEnvironment().getScreenDevices()) {
+                all = all.union(gd.getDefaultConfiguration().getBounds());
+            }
+            BufferedImage tmp = robot.createScreenCapture(all);

84-85: Hardcoded hover delay.

Consider making 1500ms configurable (test data or constant) to avoid unnecessary waits.


90-92: Use logger.error and restore interrupts.

-            logger.info("Exception: " + ExceptionUtils.getStackTrace(e));
-            setErrorMessage("Exception occurred while performing mouse hover action.");
+            if (e instanceof InterruptedException) { Thread.currentThread().interrupt(); }
+            logger.error("Mouse hover failed", e);
+            setErrorMessage("Mouse hover failed: " + e.getClass().getSimpleName());
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageOccurrenceBased.java (2)

47-49: Multi‑monitor capture support.

-            Rectangle screenSize = new Rectangle(Toolkit.getDefaultToolkit().getScreenSize());
-            BufferedImage tmp = robot.createScreenCapture(screenSize);
+            Rectangle all = new Rectangle();
+            for (GraphicsDevice gd : GraphicsEnvironment.getLocalGraphicsEnvironment().getScreenDevices()) {
+                all = all.union(gd.getDefaultConfiguration().getBounds());
+            }
+            BufferedImage tmp = robot.createScreenCapture(all);

92-95: Error logging level and interrupt preservation.

-            logger.info("Exception: " + ExceptionUtils.getStackTrace(e));
-            setErrorMessage("Exception occurred while performing right-click action on image.");
+            if (e instanceof InterruptedException) { Thread.currentThread().interrupt(); }
+            logger.error("Right-click (occurrence based) failed", e);
+            setErrorMessage("Right-click failed: " + e.getClass().getSimpleName());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6be588f and 434bf7a.

📒 Files selected for processing (10)
  • image_based_actions/pom.xml (4 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImageOccurrenceBased.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImage.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageOccurrenceBased.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageWithThreshold.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageWithThresholdOccurrenceBased.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImage.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageOccurrenceBased.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThreshold.java (1 hunks)
  • image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThresholdOccurrenceBased.java (1 hunks)
🔇 Additional comments (7)
image_based_actions/src/main/java/com/testsigma/addons/windows/ClickOnImageOccurrenceBased.java (2)

38-38: Clarify the necessity of the throws declaration.

The method signature declares throws NoSuchElementException, but the implementation catches all exceptions generically (line 93) and returns Result.FAILED rather than propagating exceptions. Unless Robot, OCR, or framework methods implicitly throw NoSuchElementException, this declaration may be unnecessary.

Can you confirm whether:

  1. The NoSuchElementException is thrown by Robot, OCR, or other framework components?
  2. The throws declaration is required by the WindowsAction interface or framework contract?

20-20: Verify exception type compatibility across the codebase.

The change from org.openqa.selenium.NoSuchElementException to java.util.NoSuchElementException is a breaking change if any calling code explicitly catches the Selenium-specific exception type. Ensure that all callers and the new Windows action classes consistently use java.util.NoSuchElementException.

Run the following script to verify consistency:

image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageWithThresholdOccurrenceBased.java (1)

65-71: I'll search for information about the Testsigma SDK OCR findImage method and its occurrence parameter indexing.<!-- [scratchpad_update]
[observations]

  • Web search returned general OCR documentation and Testsigma blogs, but no specific API documentation for OCR#findImage
  • Result index 2 points to OCR extraction addon docs, but does not contain specific method signature details
  • Need more targeted search for Testsigma SDK source or Java API documentation

[pending]

  • Search for Testsigma SDK Java documentation or GitHub repository
  • Look for OCR#findImage method signature or API specification

[actions]

  • Perform more targeted search for Testsigma Java SDK OCR method specifications
    [scratchpad_end] -->

Let me search more specifically for the Testsigma Java SDK API documentation.<!-- [scratchpad_update]
[observations]

  • Found Testsigma Java SDK GitHub repo (result 2), but the excerpt does not contain OCR API details
  • Results 3-10 are general OCR tutorials, not specific to Testsigma SDK OCR#findImage
  • Need to search for actual Testsigma addons or source that may contain usage examples

[pending]

  • Search for Testsigma addons github repository which may have example usage of OCR#findImage
  • Look for any existing image-based actions addon that uses OCR#findImage

[actions]

  • Search for testsigma-addons repository and look for image-based action examples
    [scratchpad_end] -->

Let me search for the Testsigma addons repository that might contain examples of OCR usage.<!-- [scratchpad_update]
[observations]

  • Found testsigma-addons repository link, but web results don't show specific method signatures
  • The repository exists but search results are generic
  • Cannot extract specific API method details from web searches alone
  • Likely need to inspect the repository directly via shell scripts for source code

[pending]

  • Cannot find OCR#findImage occurrence parameter documentation via web search
  • The API specification is not publicly documented in accessible web pages
  • Need to either inspect source code via shell script or admit inability to verify

[analysis]

  • Web searches returned general repository descriptions but no actual OCR API specifications
  • The occurrence parameter indexing (0-based vs 1-based) is not explicitly documented in web results
  • Without access to the SDK source code or detailed API docs, cannot definitively verify the claim

[actions]

  • Cannot proceed further with web search to find the specific API documentation
  • This is an inconclusive verification - need to tag as verify_review_comment

[done]
Searched for Testsigma SDK OCR#findImage occurrence parameter documentation through multiple web queries. No definitive API specification found in public documentation.
[scratchpad_end] -->

Based on my web searches, I was unable to locate definitive public documentation specifying whether the com.testsigma.sdk.OCR#findImage method's occurrence parameter uses 0-based or 1-based indexing. The Testsigma SDK GitHub repositories and official documentation pages don't contain explicit API specifications for this detail.

Validate threshold and occurrence index before search.

  • Enforce 0.0–1.0 threshold range validation.
  • Verify occurrence parameter indexing expectation (1-based vs 0-based) against SDK source or internal documentation before applying the suggested validation.

The diff suggestion assumes 1-based indexing (occurrence < 1), but this requires confirmation. Consider reviewing the OCR SDK method signature or reaching out to Testsigma support to confirm the exact indexing convention before merging.

image_based_actions/pom.xml (1)

78-79: Shade plugin: version and minimizeJar cautions.

  • 3.2.4 is dated; consider updating to a recent 3.5.x.
  • minimizeJar can strip classes used via reflection/ServiceLoader (SDKs often do this). Verify at runtime and be ready to exclude packages (e.g., com.testsigma., META-INF/services/).

Please confirm latest plugin and known minimize pitfalls:

If you hit class-missing issues, consider:

<configuration>
  <minimizeJar>false</minimizeJar>
  <filters> ... (keep META-INF as already done) ... </filters>
  <!-- or -->
  <!-- <relocations/> with <artifactSet/> and <transformers> as needed -->
</configuration>

Also applies to: 85-97

image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThresholdOccurrenceBased.java (1)

68-71: Occurrence indexing semantics—please confirm.

Does ocr.findImage(..., occurrence, threshold) expect 1‑based or 0‑based? Code and validation assume 1‑based. Confirm and adjust if needed.

image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageOccurrenceBased.java (1)

62-67: Occurrence input validation and semantics.

-            int occurrence = Integer.parseInt(testData2.getValue().toString());
+            int occurrence;
+            try { occurrence = Integer.parseInt(testData2.getValue().toString()); }
+            catch (NumberFormatException nfe) {
+                setErrorMessage("Invalid occurrence index.");
+                return Result.FAILED;
+            }
+            if (occurrence < 1) {
+                setErrorMessage("Occurrence must be ≥ 1 (first match = 1).");
+                return Result.FAILED;
+            }

Please confirm whether ocr.findImage expects 1‑based or 0‑based occurrence.

image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageOccurrenceBased.java (1)

65-67: Occurrence parsing/validation.

-            int occurrence = Integer.parseInt(testData2.getValue().toString());
+            int occurrence;
+            try { occurrence = Integer.parseInt(testData2.getValue().toString()); }
+            catch (NumberFormatException nfe) {
+                setErrorMessage("Invalid occurrence index.");
+                return Result.FAILED;
+            }
+            if (occurrence < 1) {
+                setErrorMessage("Occurrence must be ≥ 1 (first match = 1).");
+                return Result.FAILED;
+            }

Confirm 1‑based vs 0‑based expected by ocr.findImage.

Comment on lines +47 to +58
// Save screenshot temporarily
String tempDir = System.getProperty("java.io.tmpdir");
String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
String path = tempDir + filename;
ImageIO.write(tmp, "jpg", new File(path));

logger.info("Screen captured and saved at: " + path);
logger.info("Screen dimensions: " + tmp.getWidth() + "x" + tmp.getHeight());

File baseImageFile = new File(path);
String url = testStepResult.getScreenshotUrl();
logger.info("Amazon S3 URL where the base image is stored: " + url);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Same temp file handling: join path safely, prefer PNG, and clean up.

Apply the same diff pattern as in HoverOnImageWithThreshold (Files.createTempFile + PNG + delete after upload).

🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImage.java
around lines 47 to 58, the code writes a JPG to a constructed temp path string
and leaves the temp file behind; change to use Files.createTempFile to create a
safe temp file, write the image as PNG (ImageIO.write(tmp, "png", tempFile)),
use the tempFile.toPath()/tempFile reference instead of string concatenation for
any further file operations, and delete the temp file after uploading
(Files.deleteIfExists or tempFile.delete()) to ensure cleanup.

Comment on lines +57 to +61
String url = testStepResult.getScreenshotUrl();
logger.info("Amazon S3 URL where the base image is stored: " + url);

// Upload to OCR
ocr.uploadFile(url, baseImageFile);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not log storage URLs.

Replace with a debug message that does not include the URL.

🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImage.java
around lines 57 to 61, the code logs the S3 storage URL which must not be
recorded; replace the logger.info call so it does not include the URL (use
logger.debug or logger.info with a generic message like "Base image upload
initiated" or "Base image available in storage" without the URL) and keep the
ocr.uploadFile(...) call unchanged; ensure no other logs in this block print the
URL.

Comment on lines +51 to +55
String tempDir = System.getProperty("java.io.tmpdir");
String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
String path = tempDir + filename;
ImageIO.write(tmp, "jpg", new File(path));

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Temp file hygiene and PNG format.

-            String tempDir = System.getProperty("java.io.tmpdir");
-            String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
-            String path = tempDir + filename;
-            ImageIO.write(tmp, "jpg", new File(path));
+            java.nio.file.Path tmpFile = java.nio.file.Files.createTempFile("ts-screen-", ".png");
+            ImageIO.write(tmp, "png", tmpFile.toFile());

Add finally to delete tmpFile.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageOccurrenceBased.java
around lines 51 to 55, the code writes a screenshot to a temp file as a JPG and
never ensures the temp file is removed; change the output to PNG (use ".png"
extension and write with "png") and wrap the file creation/write in try/finally
so the temporary File is deleted in the finally block (use File.delete() or a
safe delete helper and guard against null/failed deletes).

Comment on lines +60 to +61
String url = testStepResult.getScreenshotUrl();
logger.info("Amazon S3 URL where the base image is stored: " + url);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid logging pre‑signed S3 URLs.

-            String url = testStepResult.getScreenshotUrl();
-            logger.info("Amazon S3 URL where the base image is stored: " + url);
+            String url = testStepResult.getScreenshotUrl();
+            logger.info("Uploading screenshot to storage (URL redacted)");
🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageOccurrenceBased.java
around lines 60 to 61, remove or redact the presigned S3 URL from logs; instead
extract and log non-sensitive metadata (for example S3 bucket and object key or
just the filename) or log a placeholder like "S3 URL redacted" so the full
pre-signed URL is never written to logs. Update the logger call to either parse
the URL and log only safe parts (bucket/key) or replace the value with a
constant redaction string while preserving useful context in the message.

Comment on lines +50 to +58
// Save screenshot temporarily
String tempDir = System.getProperty("java.io.tmpdir");
String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
String path = tempDir + filename;
ImageIO.write(tmp, "jpg", new File(path));

logger.info("Screen captured and saved at: " + path);
logger.info("Screen dimensions: " + tmp.getWidth() + "x" + tmp.getHeight());

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Temp file handling: fix path join, prefer PNG, and delete the temp file.

  • String concatenation for temp path is brittle.
  • JPG adds lossy artifacts; PNG preserves fidelity for matching.
  • Temp screenshot is never deleted.

Apply:

@@
-import javax.imageio.ImageIO;
+import javax.imageio.ImageIO;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.io.IOException;
@@
-            String tempDir = System.getProperty("java.io.tmpdir");
-            String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
-            String path = tempDir + filename;
-            ImageIO.write(tmp, "jpg", new File(path));
+            Path tempFile = Files.createTempFile("testsigma-screen-", ".png");
+            ImageIO.write(tmp, "png", tempFile.toFile());
@@
-            File baseImageFile = new File(path);
+            File baseImageFile = tempFile.toFile();
@@
-            ocr.uploadFile(url, baseImageFile);
+            try {
+                ocr.uploadFile(url, baseImageFile);
+            } finally {
+                try { Files.deleteIfExists(tempFile); }
+                catch (IOException ioe) { logger.warn("Failed to delete temp screenshot: " + tempFile, ioe); }
+            }

Also applies to: 59-62

🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/HoverOnImageWithThreshold.java
around lines 50-58 (and similarly 59-62), the temp screenshot path is built via
string concatenation, the image is written as JPG, and the temp file is never
deleted; change to build the path using the platform-safe API (e.g., Paths.get
or new File(tempDir, filename)) so the separator is correct, switch the image
format to PNG to avoid lossy compression, and ensure the temp file is cleaned up
(either schedule deleteOnExit or wrap the usage in try/finally and delete the
file after use). Ensure the filename still includes a timestamp/unique suffix
and log the final path and image dimensions as before.

Comment on lines +50 to +58
// Save screenshot temporarily
String tempDir = System.getProperty("java.io.tmpdir");
String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
String path = tempDir + filename;
ImageIO.write(tmp, "jpg", new File(path));

logger.info("Screen captured and saved at: " + path);
logger.info("Screen dimensions: " + tmp.getWidth() + "x" + tmp.getHeight());

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Temp file path, PNG fidelity, cleanup, and sensitive URL logging.

Same concerns and fixes as the hover actions.

Also applies to: 59-62

Comment on lines +65 to +66
FindImageResponse responseObject = ocr.findImage(testData1.getValue().toString(),Float.valueOf(testData2.getValue().toString()));

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Validate threshold input (0.0–1.0) before search.

Add parsing and range check prior to calling ocr.findImage(...).

🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThreshold.java
around lines 65-66, the threshold argument is passed directly to ocr.findImage
without validation; parse the second testData value to a float (catch
NumberFormatException), then verify it is within 0.0–1.0; if parsing fails or
the value is out of range, either throw a clear IllegalArgumentException or set
a safe default (e.g., 0.8f) and log/report the invalid input, and only call
ocr.findImage(...) with the validated float.

Comment on lines +54 to +58
String tempDir = System.getProperty("java.io.tmpdir");
String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
String path = tempDir + filename;
ImageIO.write(tmp, "jpg", new File(path));

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Stop leaking temp files; use PNG and safe path join.

Use Files.createTempFile (PNG) and delete in finally; current code may leak files and JPG hurts matching.

Apply:

-            String tempDir = System.getProperty("java.io.tmpdir");
-            String filename = "screenshot_" + System.currentTimeMillis() + ".jpg";
-            String path = tempDir + filename;
-            ImageIO.write(tmp, "jpg", new File(path));
+            java.nio.file.Path tmpFile = java.nio.file.Files.createTempFile("ts-screen-", ".png");
+            ImageIO.write(tmp, "png", tmpFile.toFile());

And later (cleanup):

-        } catch (Exception e) {
+        } catch (Exception e) {
           ...
-        }
+        } finally {
+            try { if (tmpFile != null) java.nio.file.Files.deleteIfExists(tmpFile); } catch (Exception ignore) {}
+        }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThresholdOccurrenceBased.java
around lines 54 to 58, the code writes a JPG into the system temp dir via string
concatenation and can leak files; change to use Files.createTempFile to create a
secure temp file (use a .png suffix), write the BufferedImage as PNG, avoid
manual path concatenation, and ensure the temp file is deleted in a finally
block (or try-with-resources + Files.deleteIfExists in finally) so the file is
always cleaned up after use.

Comment on lines +63 to +65
String url = testStepResult.getScreenshotUrl();
logger.info("Amazon S3 URL where the base image is stored: " + url);
Float threshold = Float.valueOf(testData2.getValue().toString());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not log pre‑signed S3 URLs.

Pre‑signed URLs are sensitive; remove or mask them.

-            String url = testStepResult.getScreenshotUrl();
-            logger.info("Amazon S3 URL where the base image is stored: " + url);
+            String url = testStepResult.getScreenshotUrl();
+            logger.info("Uploading screenshot to storage (URL redacted)");
📝 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.

Suggested change
String url = testStepResult.getScreenshotUrl();
logger.info("Amazon S3 URL where the base image is stored: " + url);
Float threshold = Float.valueOf(testData2.getValue().toString());
String url = testStepResult.getScreenshotUrl();
logger.info("Uploading screenshot to storage (URL redacted)");
Float threshold = Float.valueOf(testData2.getValue().toString());
🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThresholdOccurrenceBased.java
around lines 63-65, the code logs a pre-signed S3 URL which is sensitive; remove
or mask that URL before logging. Replace the logger.info that prints the full
URL with either no log at all or a sanitized value (for example log only the S3
object key or bucket name, or strip the query string and/or replace characters
after the '?' with "[REDACTED]" using a simple string operation or regex) so no
pre-signed token or query parameters are emitted to logs.

Comment on lines +65 to +67
Float threshold = Float.valueOf(testData2.getValue().toString());
int occurrence = Integer.parseInt(testData3.getValue().toString());

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate threshold ∈ [0,1] and occurrence ≥ 1; fail fast with clear message.

Avoid NumberFormatException and ambiguous indexing.

-            Float threshold = Float.valueOf(testData2.getValue().toString());
-            int occurrence = Integer.parseInt(testData3.getValue().toString());
+            float threshold;
+            int occurrence;
+            try {
+                threshold = Float.parseFloat(testData2.getValue().toString());
+                occurrence = Integer.parseInt(testData3.getValue().toString());
+            } catch (NumberFormatException nfe) {
+                setErrorMessage("Invalid threshold or occurrence format.");
+                return Result.FAILED;
+            }
+            if (threshold < 0f || threshold > 1f) {
+                setErrorMessage("Threshold must be between 0.0 and 1.0.");
+                return Result.FAILED;
+            }
+            if (occurrence < 1) {
+                setErrorMessage("Occurrence must be ≥ 1 (first match = 1).");
+                return Result.FAILED;
+            }
🤖 Prompt for AI Agents
In
image_based_actions/src/main/java/com/testsigma/addons/windows/RightClickOnImageWithThresholdOccurrenceBased.java
around lines 65 to 67, the code parses threshold and occurrence without
validation or error handling; add robust input validation and fail-fast error
messages: ensure testData2 and testData3 values are non-null, parse them inside
a try-catch to catch NumberFormatException, validate that threshold is a float
between 0.0 and 1.0 inclusive and that occurrence parses to an integer >= 1
(document whether occurrence is 1-based), and if any check fails throw or return
an explicit, descriptive error (e.g., IllegalArgumentException or step-failure)
stating which value is invalid and why to avoid ambiguous indexing and parsing
failures.

@akhil-testsigma akhil-testsigma merged commit 51d1ade into dev Oct 21, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants