-
Notifications
You must be signed in to change notification settings - Fork 16
[feat] Addons for windows advanced application type. #211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces a new Windows Advanced Actions Maven module with multiple keyboard automation actions, AI-assisted on-screen text verification/click workflows, shared keyboard/screenshot utilities, and a testsigma SDK properties file. Includes shaded JAR build, Java 11 targeting, and dependencies for Selenium, Appium, Lombok, HttpClient, and Testsigma SDK. Changes
Sequence Diagram(s)sequenceDiagram
participant Step as Test Step
participant Action as ClickOnTextWithWait
participant Robot as AWT Robot
participant AI as AI Service
participant SS as ScreenshotUtils
Step->>Action: execute()
loop until timeout or found
Action->>Robot: Capture screen
Action->>AI: Send prompt + image
AI-->>Action: Coordinates or NOT FOUND
alt Found
Action->>Robot: Move & click
Action->>SS: Upload success screenshot
Action-->>Step: Result.SUCCESS
else Not found
Action->>Action: Wait interval
end
end
Action-->>Step: Result.FAILED (on timeout/error)
sequenceDiagram
participant Step as Test Step
participant Action as PressModifierKeyWithBasicKeyCombination
participant KU as KeyboardUtils
participant Robot as AWT Robot
participant SS as ScreenshotUtils
Step->>Action: execute()
Action->>KU: Map keys to keyCodes
Action->>Robot: press(modifier)
Action->>Robot: press(basic)
Action->>Robot: release(basic)
Action->>Robot: release(modifier)
Action->>SS: Upload screenshot
Action-->>Step: Result (SUCCESS/FAILED)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (11)
windows_advanced_actions/pom.xml (2)
45-45: Consider upgrading TestNG to a more recent version.TestNG version 6.14.3 is quite old (released in 2018). Consider upgrading to a more recent version for better security, bug fixes, and feature improvements.
Apply this diff to upgrade TestNG:
- <version>6.14.3</version> + <version>7.8.0</version>
82-94: Maven Shade Plugin configuration lacks conflict resolution.The Maven Shade Plugin configuration is minimal and may cause issues with conflicting dependencies or service files. Consider adding transformers and conflict resolution strategies.
Apply this diff to improve the shade plugin configuration:
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> <version>3.2.4</version> + <configuration> + <transformers> + <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer"/> + <transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer"/> + </transformers> + <filters> + <filter> + <artifact>*:*</artifact> + <excludes> + <exclude>META-INF/*.SF</exclude> + <exclude>META-INF/*.DSA</exclude> + <exclude>META-INF/*.RSA</exclude> + </excludes> + </filter> + </filters> + </configuration> <executions> <execution> <phase>package</phase> <goals> <goal>shade</goal> </goals> </execution> </executions> </plugin>windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (1)
30-30: Unnecessary fully qualified class name.Line 30 uses
new java.awt.Robot()whenRobotis already imported. The fully qualified name is redundant.Apply this diff to use the imported class:
- Robot robot = new java.awt.Robot(); + Robot robot = new Robot();windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java (3)
44-44: Misleading success message.The success message mentions "on the given image" which doesn't align with the actual functionality of entering data via keyboard/clipboard. This could confuse users.
Apply this diff to correct the success message:
- setSuccessMessage("Given data entered successfully on the given image"); + setSuccessMessage("Given data entered successfully using keyboard");
47-47: Misleading error message.The error message mentions "initializing the Robot class" but exceptions could occur during clipboard operations, key presses, or thread sleep as well.
Apply this diff to make the error message more accurate:
- setErrorMessage("An error occurred while initializing the Robot class: " + e.getMessage()); - logger.debug("Error initializing Robot class: " + ExceptionUtils.getStackTrace(e)); + setErrorMessage("An error occurred while entering data using keyboard: " + e.getMessage()); + logger.debug("Error entering data using keyboard: " + ExceptionUtils.getStackTrace(e));
36-43: Potential clipboard data loss risk.The current implementation saves and restores clipboard contents, but if the original clipboard data is
nullor if an exception occurs between saving and restoring, the clipboard state may be corrupted.Apply this diff to improve clipboard handling:
Clipboard clipboard = Toolkit.getDefaultToolkit().getSystemClipboard(); - Transferable data = clipboard.getContents(null); + Transferable originalData = clipboard.getContents(null); clipboard.setContents(stringSelection, null); robot.keyPress(KeyEvent.VK_CONTROL); robot.keyPress(KeyEvent.VK_V); robot.keyRelease(KeyEvent.VK_V); robot.keyRelease(KeyEvent.VK_CONTROL); Thread.sleep(500); - clipboard.setContents(data, null); + // Restore original clipboard content only if it was not null + if (originalData != null) { + clipboard.setContents(originalData, null); + }windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
278-282: Redundant fallback logic in getSpecificKeyCode.Lines 278-282 duplicate the single character handling that's already done at lines 222-229. This creates unnecessary code duplication.
Apply this diff to remove redundant code:
default: - // Try to get the key code for the character - if (key.length() == 1) { - return KeyEvent.getExtendedKeyCodeForChar(key.charAt(0)); - } throw new IllegalArgumentException("Unsupported specific key: " + key);windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
23-24: Consider allowing WINDOW key for both modifier key positions.The second modifier key (
keyType2) excludes "WINDOW" from its allowed values while the first modifier key (keyType1) includes it. This asymmetry may limit functionality - users might want to press WINDOW + another modifier key in any order.Consider making the allowed values consistent:
- @TestData(reference = "key-type-2", allowedValues = {"Alt", "Ctrl", "Enter", "Shift", "Tab"}) + @TestData(reference = "key-type-2", allowedValues = {"Alt", "Ctrl", "Enter", "Shift", "Tab", "WINDOW"})windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplicationWithAI.java (3)
32-36: Consider enhancing the AI prompt for better accuracyThe current prompt could be more specific about handling edge cases like partial matches, case sensitivity, and special characters. Consider adding instructions about:
- Whether partial text matches should be considered
- How to handle text with special formatting (bold, italic, etc.)
- Whether the search should be case-sensitive
- private final String prompt = "You are provided with a screenshot of a computer application." + - " Your task is to analyze this screenshot and determine if the specified text is present anywhere in " + - "the image.Look for the text in any form - it could be in buttons, labels, text fields, menus, " + - "or any other UI element. Return only 'YES' if the text is found, or 'NO' if the text is not found. " + - "The text to search for is: "; + private final String prompt = "You are provided with a screenshot of a computer application." + + " Your task is to analyze this screenshot and determine if the specified text is present anywhere in " + + "the image. Look for the text in any form - it could be in buttons, labels, text fields, menus, " + + "or any other UI element. The search should be case-insensitive and match the exact text provided. " + + "Return only 'YES' if the exact text is found, or 'NO' if the text is not found. " + + "The text to search for is: ";
47-50: Consider adding screen validation before captureThe code assumes the screen is available and doesn't handle potential issues like multiple monitors or different screen resolutions gracefully.
// Capture the current screen Robot robot = new Robot(); -Rectangle screenRect = new Rectangle(Toolkit.getDefaultToolkit().getScreenSize()); +Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize(); +if (screenSize.width == 0 || screenSize.height == 0) { + throw new RuntimeException("Invalid screen dimensions detected"); +} +Rectangle screenRect = new Rectangle(screenSize); BufferedImage screenCapture = robot.createScreenCapture(screenRect); logger.info("Screen capture dimensions: " + screenCapture.getWidth() + "x" + screenCapture.getHeight());
86-90: Improve error handling with more specific exceptionsThe current catch block catches all exceptions generically. Consider handling specific exceptions differently to provide better error messages to users.
-} catch (Exception e) { - logger.debug("Exception during AI text verification: " + e.getMessage()); - setErrorMessage("Error during text verification: " + e.getMessage()); - return Result.FAILED; -} +} catch (AWTException e) { + logger.error("Failed to initialize Robot for screen capture: " + e.getMessage()); + setErrorMessage("Unable to capture screen: " + e.getMessage()); + return Result.FAILED; +} catch (RuntimeException e) { + logger.error("Runtime error during text verification: " + e.getMessage()); + setErrorMessage(e.getMessage()); + return Result.FAILED; +} catch (Exception e) { + logger.error("Unexpected error during AI text verification: " + e.getMessage(), e); + setErrorMessage("Unexpected error during text verification: " + e.getMessage()); + return Result.FAILED; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
windows_advanced_actions/pom.xml(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplicationWithAI.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java(1 hunks)windows_advanced_actions/src/main/resources/testsigma-sdk.properties(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (7)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
KeyboardUtils(8-285)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (1)
Data(14-47)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java (1)
Data(18-53)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
Data(13-66)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java (1)
Data(13-60)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
Data(13-58)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
Data(13-67)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (6)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
Data(13-46)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java (1)
Data(18-53)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
Data(13-66)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
Data(13-58)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
Data(13-67)windows_desktop_actions/src/main/java/com/testsigma/addons/web/PressControlOrFunctionKey.java (1)
Data(14-48)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (5)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
KeyboardUtils(8-285)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
Data(13-46)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java (1)
Data(13-60)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
Data(13-58)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
Data(13-67)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (5)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
KeyboardUtils(8-285)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
Data(13-46)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
Data(13-66)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java (1)
Data(13-60)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
Data(13-58)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (5)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
KeyboardUtils(8-285)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
Data(13-46)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
Data(13-66)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java (1)
Data(13-60)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
Data(13-67)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplicationWithAI.java (6)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (1)
Data(14-47)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
Data(13-46)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java (1)
Data(18-53)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
Data(13-66)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
Data(13-58)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
Data(13-67)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java (6)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
KeyboardUtils(8-285)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (1)
Data(14-47)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
Data(13-46)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
Data(13-66)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
Data(13-58)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
Data(13-67)
🪛 Gitleaks (8.27.2)
windows_advanced_actions/src/main/resources/testsigma-sdk.properties
1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (7)
windows_advanced_actions/pom.xml (1)
51-51: Confirm Selenium Java client version compatibilitySelenium Java client 4.33.0 is generally stable and has no widespread, blocking issues for Java bindings. The latest stable release is 4.34.0 (June 29, 2025), which includes incremental fixes and enhancements. If you encounter any test failures or edge-case bugs, consider upgrading to 4.34.0; otherwise, 4.33.0 is safe to use.
• File:
windows_advanced_actions/pom.xml(line 51)
• Location to update (optional):- <version>4.33.0</version> + <version>4.34.0</version>windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (1)
44-44: Missing closing brace.Line 44 is missing a closing brace for the catch block, causing a syntax error.
Apply this diff to fix the syntax error:
- result = Result.FAILED; } + result = Result.FAILED; + }Likely an incorrect or invalid review comment.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java (1)
1-66: LGTM! Well-implemented keyboard action with proper error handling and key sequence management.The implementation follows the established patterns from other keyboard actions in this PR, correctly uses the KeyboardUtils utility for key mapping, and properly handles key press/release sequencing. The error handling and success messaging are consistent with the codebase standards.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
1-46: LGTM! Clean and consistent implementation following established patterns.The implementation correctly follows the established patterns for Windows Advanced Actions in this PR. The use of KeyboardUtils for key mapping, proper error handling with ExceptionUtils, and consistent messaging align well with the codebase standards. The key press/release sequence is appropriate for single modifier keys.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java (1)
1-67: LGTM! Excellent implementation of complex key combination handling.The implementation correctly manages the complex sequence of pressing two modifier keys plus a specific key. The key press order (modifier1 → modifier2 → specific) and release order (specific → modifier2 → modifier1) follow proper keyboard handling conventions. The error handling and messaging are consistent with other actions in this PR.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
26-58: LGTM! Proper implementation of simultaneous key press handling.The key press and release sequence is correctly implemented for simultaneous modifier key combinations. The error handling and messaging follow the established patterns in this PR.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithSpecificKey.java (1)
1-60: LGTM! Well-structured implementation with proper key sequence handling.The implementation correctly handles the modifier + specific key combination with appropriate press/release ordering. The use of KeyboardUtils for key mapping and consistent error handling align with the established patterns across all keyboard actions in this PR.
...ws_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java
Show resolved
Hide resolved
...advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java
Show resolved
Hide resolved
...advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java
Show resolved
Hide resolved
...advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java
Show resolved
Hide resolved
...advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java
Show resolved
Hide resolved
...ctions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplicationWithAI.java
Show resolved
Hide resolved
...ctions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplicationWithAI.java
Show resolved
Hide resolved
rajesh-testsigma
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.For all these actions, we need to take the Screenshot using the ROBOT.
2.For VerifyText..action, Check if it is possible to highlight the Verified text(if text found) in the screenshot
3.We need to have a IF condition action for "Text present on screen". If this is not possible in WINDOWS_ADVANCED, we need to store true or false based on the availability of the text and compare the value in IF condition
...ws_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java
Show resolved
Hide resolved
...anced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java
Show resolved
Hide resolved
...ws_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java
Show resolved
Hide resolved
...ws_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithAlphanumericKey.java
Outdated
Show resolved
Hide resolved
...dvanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithSpecificKey.java
Outdated
Show resolved
Hide resolved
...ctions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplicationWithAI.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
♻️ Duplicate comments (9)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (5)
84-92: Remove redundant logic for single character handling.The
ifblocks handling single digits (lines 85-87) and single letters (lines 90-92) are redundant because the same logic is already implemented in the switch statement below (lines 96-167).Apply this diff to remove redundant logic:
public static int getAlphanumericKeyCode(String key) { - // Handle digits - if (key.length() == 1 && Character.isDigit(key.charAt(0))) { - return KeyEvent.getExtendedKeyCodeForChar(key.charAt(0)); - } - - // Handle letters - if (key.length() == 1 && Character.isLetter(key.charAt(0))) { - return KeyEvent.getExtendedKeyCodeForChar(Character.toUpperCase(key.charAt(0))); - } - // Handle specific key mappings switch (key.toUpperCase()) {
181-181: Clarify mapping for QUESTION mark - requires Shift+Slash.QUESTION is mapped to VK_SLASH, but the question mark (?) typically requires Shift+Slash on most keyboards.
Apply this diff to document the modifier requirement:
case "QUESTION": - return KeyEvent.VK_SLASH; + return KeyEvent.VK_SLASH; // Note: Actual '?' requires Shift+Slash
186-199: Incorrect key mappings for symbol characters - modifier keys required.The mappings for DOLLAR, PERCENT, CARET, AMPERSAND, ASTERISK, LEFT_PARENTHESIS, and RIGHT_PARENTHESIS are mapped to number keys (VK_4, VK_5, etc.) without accounting for the required Shift modifier.
These symbols require Shift to be held when pressing the number keys. Consider:
- Adding documentation to clarify the modifier requirement
- Creating a data structure to indicate when Shift is required
- Or ensuring callers handle the Shift key appropriately
Apply this diff to document the issue:
case "DOLLAR": - return KeyEvent.VK_4; + return KeyEvent.VK_4; // Note: Requires Shift+4 case "PERCENT": - return KeyEvent.VK_5; + return KeyEvent.VK_5; // Note: Requires Shift+5 case "CARET": - return KeyEvent.VK_6; + return KeyEvent.VK_6; // Note: Requires Shift+6 case "AMPERSAND": - return KeyEvent.VK_7; + return KeyEvent.VK_7; // Note: Requires Shift+7 case "ASTERISK": - return KeyEvent.VK_8; + return KeyEvent.VK_8; // Note: Requires Shift+8 case "LEFT_PARENTHESIS": - return KeyEvent.VK_9; + return KeyEvent.VK_9; // Note: Requires Shift+9 case "RIGHT_PARENTHESIS": - return KeyEvent.VK_0; + return KeyEvent.VK_0; // Note: Requires Shift+0
214-219: Document duplicate key mappings that require modifier keys.PIPE maps to VK_BACK_SLASH (same as BACKSLASH), and LEFT_BRACE/RIGHT_BRACE map to VK_OPEN_BRACKET/VK_CLOSE_BRACKET (same as LEFT_BRACKET/RIGHT_BRACKET). These symbols require Shift to be held.
Apply this diff to clarify the modifier requirements:
case "PIPE": - return KeyEvent.VK_BACK_SLASH; + return KeyEvent.VK_BACK_SLASH; // Note: Requires Shift+Backslash case "LEFT_BRACE": - return KeyEvent.VK_OPEN_BRACKET; + return KeyEvent.VK_OPEN_BRACKET; // Note: Requires Shift+[ case "RIGHT_BRACE": - return KeyEvent.VK_CLOSE_BRACKET; + return KeyEvent.VK_CLOSE_BRACKET; // Note: Requires Shift+]
203-203: Incorrect mapping for PLUS symbol.PLUS is mapped to VK_EQUALS, but the plus symbol (+) typically requires Shift+Equals on most keyboards.
Apply this diff to document the modifier requirement:
case "PLUS": - return KeyEvent.VK_EQUALS; + return KeyEvent.VK_EQUALS; // Note: Actual '+' requires Shift+Equalswindows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java (1)
43-43: Replace reflection with safer explicit mapping.Using reflection to access KeyEvent fields without proper validation could be exploited and makes the code harder to maintain.
Apply this diff to use the existing KeyboardUtils method instead of reflection:
-// Convert the function key to its corresponding KeyEvent constant -int keyCode = KeyEvent.class.getField("VK_" + key).getInt(null); +// Convert the function key to its corresponding KeyEvent constant +int keyCode = KeyboardUtils.getSpecificKeyCode(key);The KeyboardUtils already has support for F1-F12 keys in the
getSpecificKeyCodemethod.windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
45-47: Past feedback addressed: added 30 ms stabilization delayThanks for adding a 30 ms delay to stabilize key press registration; this directly addresses the earlier review suggestion.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithBasicKey.java (1)
24-33: Consistency of modifier keys across actions looks goodThe allowedValues for key-type-1 and key-type-2 match other actions (including BackSpace, Esc, Delete, CapsLock, and arrow keys), addressing the “common modifier keys across actions” feedback.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java (1)
24-31: Common modifier keys across actions: looks consistentThe allowedValues sets are consistent with other actions and include BackSpace, Esc, Delete, CapsLock, and arrow keys as requested earlier.
🧹 Nitpick comments (13)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java (1)
306-312: Consider preserving interrupted state in sleep method.The sleep method swallows InterruptedException without preserving the thread's interrupted state, which could mask important interruption signals.
Apply this diff to preserve the interrupted state:
public static void sleep(int delayInMilliseconds) { try { Thread.sleep(delayInMilliseconds); } catch (InterruptedException interruptedException) { - // ignore the exception + // Restore the interrupted status + Thread.currentThread().interrupt(); } }windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/ScreenshotUtils.java (2)
51-53: Check file deletion result.The temporary file deletion doesn't check if the operation was successful. Consider logging a warning if deletion fails.
Apply this diff to handle deletion result:
// Clean up temporary file if (screenshotFile.exists()) { - screenshotFile.delete(); + if (!screenshotFile.delete()) { + logger.info("Failed to delete temporary screenshot file: " + screenshotFile.getAbsolutePath()); + } }
171-173: Check file deletion result.Similar to the previous comment, the temporary file deletion should check the result.
Apply this diff to handle deletion result:
// Clean up temporary file if (screenshotFile.exists()) { - screenshotFile.delete(); + if (!screenshotFile.delete()) { + logger.info("Failed to delete temporary screenshot file: " + screenshotFile.getAbsolutePath()); + } }windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithGivenKey.java (1)
52-52: Replace Thread.sleep with KeyboardUtils.sleep for consistency.Use the utility method for consistency with other actions in the codebase.
Apply this diff:
// Small delay -Thread.sleep(100); +KeyboardUtils.sleep(100);windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java (1)
24-27: Align allowed modifier keys across actions and docs; clarify Fn supportThe allowedValues set looks consistent with other actions, but it omits Fn while the PR objective mentions Fn. AWT Robot cannot synthesize hardware-level Fn reliably on most machines. Please:
- Confirm that Fn is intentionally unsupported.
- Align all action descriptions/docs (and Marketplace listing) to reflect the exact supported list across actions.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplication.java (2)
39-44: Tiny grammar fix in prompt stringMinor readability nit: add a space after the period in "the image.Look".
- "the image.Look for the text in any form - it could be in buttons, labels, text fields, menus, " + + "the image. Look for the text in any form - it could be in buttons, labels, text fields, menus, " +
63-69: Optional: Externalize model selection instead of hardcoding "gpt-4o"Consider moving the model name to a property (e.g., testsigma-sdk.properties or an env var) so it can be changed without code changes and can align with tenant-specific AI configs.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/ClickOnTextWithWait.java (1)
186-219: Improve robustness of AI coordinate parsingCurrent parsing only accepts strict "YES,x,y". In practice, AI may return extra tokens (e.g., "YES, 123, 456." or "YES, x: 123, y: 456"). Consider extracting the first two integers to improve resilience. If you want, I can provide a regex-based parser.
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/WaitUntilTextPresentInScreen.java (5)
111-113: Nit: fix log message grammar for seconds.The log says "second" regardless of value.
- logger.info("Text not found yet. Waiting " + (POLLING_INTERVAL_MS / 1000) - + " second before next attempt. " + + logger.info("Text not found yet. Waiting " + (POLLING_INTERVAL_MS / 1000) + + " seconds before next attempt. " +
75-80: Hard-coded AI model may be brittle; parameterize or use default.Ensure "gpt-4o" is supported in your AI backend; consider making it configurable via system property or environment variable to allow upgrades without code changes.
- aiRequest.setModel("gpt-4o"); + aiRequest.setModel(System.getProperty("testsigma.ai.model", "gpt-4o"));Please verify that this approach aligns with the Testsigma SDK API expectations. Alternatively, rely on the SDK default model if available.
145-156: Method signature advertises checked exception but always throws RuntimeException.The method declares
throws Exceptionand mentions it in Javadoc, but it catches and wraps intoRuntimeException. Clean up the signature and Javadoc.- /** - * Saves the screenshot to a temporary file - * @param screenshot The captured screenshot - * @param fileName The base filename - * @return The temporary file - * @throws Exception if file creation fails - */ - private File saveScreenshotToFile(BufferedImage screenshot, String fileName) throws Exception { + /** + * Saves the screenshot to a temporary file + * @param screenshot The captured screenshot + * @param fileName The base filename + * @return The temporary file + */ + private File saveScreenshotToFile(BufferedImage screenshot, String fileName) {
69-85: Privacy/PII: Full-screen capture is sent to AI and uploaded.Capturing the entire screen and sending it to an external AI and S3 can leak sensitive information. Consider restricting capture to the target app window or allowing masking/regions via parameters.
Would you like me to propose a variant that captures only the active window or supports user-defined crop regions?
21-27: NLP text: consider clearer action text for users.Current text is functional but could be clearer and aligned with other steps (e.g., “Wait until text '' is present within seconds”).
-@Action(actionText = "Wait until text test-data is present in screen with timeout test-data2 seconds", +@Action(actionText = "Wait until text 'test-data' is present on screen within test-data2 seconds", description = "This action waits until the specified text is present on the screen using AI capabilities. " + - "It polls every 1 second until the text is found or timeout is reached. " + + "It polls every 1 second until the text is found or timeout is reached. " + "This works only for local executions",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
windows_advanced_actions/pom.xml(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/ClickOnTextWithWait.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressFunctionKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithBasicKeyCombination.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithGivenKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeys.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressTwoModifierKeysWithBasicKey.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/VerifyTextInApplication.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/WaitUntilTextPresentInScreen.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/KeyboardUtils.java(1 hunks)windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/utils/ScreenshotUtils.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- windows_advanced_actions/pom.xml
- windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/EnterDataUsingKeyboard.java
🔇 Additional comments (1)
windows_advanced_actions/src/main/java/com/testsigma/addons/windowsAdvanced/PressModifierKeyWithBasicKeyCombination.java (1)
1-86: LGTM! Well-structured keyboard action implementation.The implementation correctly uses the KeyboardUtils for key code mapping and ScreenshotUtils for screenshot capture. The error handling is comprehensive, and the key press/release sequence with delays is appropriate for reliable keyboard simulation.
Approve this addon as PUBLIC
Addon Name : Windows Advanced actions
Addon account: [email protected]
Jarvis Link : https://jarvis.testsigma.com/ui/tenants/2817/addons
Added below nlps to the windows advanced application
Summary by CodeRabbit