-
Notifications
You must be signed in to change notification settings - Fork 16
[CUS-7924] ADDED Verify if testdata1 option testdata2 NLP in Windows lite Application #236
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
Conversation
WalkthroughIntroduces a Maven-based verify_string_actions addon project with POM configuration, SDK properties, and four comparison action classes supporting numeric and string operators across two Windows action variants (standard and advanced). Changes
Sequence DiagramsequenceDiagram
actor User
participant Action as Comparison Action
participant Validator as Input Validator
participant NumComp as Numeric Comparison
participant StrComp as String Comparison
participant Result as Result Builder
User->>Action: Execute with testData1, testData2, operator
Action->>Validator: Validate inputs for null
alt Inputs Valid
Validator-->>Action: Proceed
Action->>NumComp: Try parseInt on both values
alt NumberFormatException
NumComp-->>Action: Parsing failed
Action->>StrComp: Perform string comparison
StrComp-->>Action: Comparison result
else Success
NumComp-->>Action: Numeric comparison result
end
Action->>Result: Set success/failure message
Result-->>User: Return comparison result
else Inputs Invalid
Validator-->>Action: Error detected
Action->>Result: Set error message
Result-->>User: Return failure with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes The changes span six files with moderate complexity: new project structure (POM), SDK properties, and four similar-yet-distinct action classes with comparable logic patterns. While there is repetition across the classes, each requires separate verification of operator support consistency, null handling, and numeric/string comparison semantics. The heterogeneous placement across two action hierarchies (WindowLite vs. WindowsAdvanced) demands attention to ensure behavior alignment. Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Nitpick comments (5)
verify_string_actions/pom.xml (3)
20-20: Bump Lombok to latest stable for JDK/IDE compatibility.1.18.30 → 1.18.42 recommended. Improves generated docs and processor behavior.
Apply:
- <lombok.version>1.18.30</lombok.version> + <lombok.version>1.18.42</lombok.version>Based on learnings.
49-58: Scope selenium/appium appropriately to avoid fat shaded jars (unless strictly needed at runtime).Shading selenium-java and io.appium:java-client will bloat the addon and risk conflicts. If the platform provides these, use provided scope or exclude from shade.
Options:
- Mark as provided:
<dependency> <groupId>org.seleniumhq.selenium</groupId> <artifactId>selenium-java</artifactId> <version>4.33.0</version> + <scope>provided</scope> </dependency> <dependency> <groupId>io.appium</groupId> <artifactId>java-client</artifactId> <version>9.4.0</version> + <scope>provided</scope> </dependency>
- Or keep compile scope but configure shade to minimize and exclude test frameworks and large drivers. See next comment. Based on learnings (selenium 4.33.0 is current).
69-81: Tune shade plugin to minimize and exclude test frameworks.Reduce jar size and avoid leaking junit/testng into runtime.
Example:
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> <version>3.2.4</version> <executions> <execution> <phase>package</phase> <goals> <goal>shade</goal> </goals> + <configuration> + <minimizeJar>true</minimizeJar> + <filters> + <filter> + <artifact>org.junit.jupiter:junit-jupiter-api</artifact> + <excludes> + <exclude>**</exclude> + </excludes> + </filter> + <filter> + <artifact>org.testng:testng</artifact> + <excludes> + <exclude>**</exclude> + </excludes> + </filter> + </filters> + </configuration> </execution> </executions> </plugin>Also applies to: 83-95
verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyIfStringsMatchTheOperator.java (1)
11-11: Remove unused TestNG import.org.testng.annotations.Test isn’t used.
verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyIfStringsMatchTheOperator.java (1)
6-7: Remove unused import.WindowsAction is not used.
-import com.testsigma.sdk.WindowsAction; import com.testsigma.sdk.WindowsAdvancedAction;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
verify_string_actions/pom.xml(1 hunks)verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyIfStringsMatchTheOperator.java(1 hunks)verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyTestDataInIfcondition.java(1 hunks)verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyIfStringsMatchTheOperator.java(1 hunks)verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyTestDataInIfcondition.java(1 hunks)verify_string_actions/src/main/resources/testsigma-sdk.properties(1 hunks)
🔇 Additional comments (1)
verify_string_actions/pom.xml (1)
43-46: TestNG 6.14.3 is very old; move to 7.x and mark test scope.6.14.3 has known issues; also don’t ship it in the runtime/shaded jar.
Apply:
- <dependency> - <groupId>org.testng</groupId> - <artifactId>testng</artifactId> - <version>6.14.3</version> - </dependency> + <dependency> + <groupId>org.testng</groupId> + <artifactId>testng</artifactId> + <version>7.10.0</version> <!-- or latest 7.x --> + <scope>test</scope> + </dependency>Please verify exact latest 7.x in your repo/CI before bumping. Based on learnings.
| <groupId>com.fasterxml.jackson.core</groupId> | ||
| <artifactId>jackson-annotations</artifactId> | ||
| <version>2.13.0</version> | ||
| </dependency> |
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.
Upgrade Jackson annotations (old, vulnerable 2.13.0).
Pin to a current 2.17.x to avoid known CVEs and align with modern Jackson.
Apply:
- <dependency>
- <groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-annotations</artifactId>
- <version>2.13.0</version>
- </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-annotations</artifactId>
+ <version>2.17.2</version>
+ </dependency>📝 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.
| <groupId>com.fasterxml.jackson.core</groupId> | |
| <artifactId>jackson-annotations</artifactId> | |
| <version>2.13.0</version> | |
| </dependency> | |
| <dependency> | |
| <groupId>com.fasterxml.jackson.core</groupId> | |
| <artifactId>jackson-annotations</artifactId> | |
| <version>2.17.2</version> | |
| </dependency> |
🤖 Prompt for AI Agents
In verify_string_actions/pom.xml around lines 60 to 63, the jackson-annotations
dependency is pinned to vulnerable 2.13.0; update the <version> to a current
2.17.x release (e.g., 2.17.y) to address known CVEs, ensure consistency with any
Jackson BOM or other Jackson artifacts in the project (or update the BOM to
2.17.x instead), then run a full build and unit tests to verify compatibility
and resolve any API changes.
| @TestData(reference = "operator", | ||
| allowedValues = {"==", ">", "<=", ">=", "!=", "less than", "contains"}) | ||
| private com.testsigma.sdk.TestData operator; | ||
|
|
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.
Add missing '<' to allowed operator values.
You handle "<" in code but it’s not present in allowedValues, which will block it in the UI.
Apply:
- @TestData(reference = "operator",
- allowedValues = {"==", ">", "<=", ">=", "!=", "less than", "contains"})
+ @TestData(reference = "operator",
+ allowedValues = {"==", "<", ">", "<=", ">=", "!=", "less than", "contains"})📝 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.
| @TestData(reference = "operator", | |
| allowedValues = {"==", ">", "<=", ">=", "!=", "less than", "contains"}) | |
| private com.testsigma.sdk.TestData operator; | |
| @TestData(reference = "operator", | |
| allowedValues = {"==", "<", ">", "<=", ">=", "!=", "less than", "contains"}) | |
| private com.testsigma.sdk.TestData operator; |
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyIfStringsMatchTheOperator.java
around lines 26 to 29, the allowedValues for the operator TestData is missing
the "<" token even though the code handles it; update the allowedValues array to
include "<" (e.g., add "<" alongside the existing entries) so the UI will accept
the less-than operator.
| protected Result execute() throws NoSuchElementException { | ||
| logger.info("=== Verify If Strings Match The Operator: Starting Execution ==="); | ||
| com.testsigma.sdk.Result result = com.testsigma.sdk.Result.SUCCESS; | ||
|
|
||
| if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) { | ||
| setErrorMessage("One or more input values are null"); | ||
| return com.testsigma.sdk.Result.FAILED; | ||
| } | ||
| String str1 = testData1.getValue().toString(); | ||
| String str2 = testData2.getValue().toString(); | ||
| String op = operator.getValue().toString(); | ||
|
|
||
| // try int comparison first | ||
| try { | ||
| int num1 = Integer.parseInt(str1); | ||
| int num2 = Integer.parseInt(str2); | ||
| boolean comparisonResult = compareNumbers(num1, num2, op); | ||
|
|
||
| } catch (NumberFormatException e) { | ||
| // If parsing fails, fall back to string comparison | ||
| boolean comparisonResult = compareStrings(str1, str2, op); | ||
| if (!comparisonResult) { | ||
| result = com.testsigma.sdk.Result.FAILED; | ||
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | ||
| } else { | ||
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | ||
| } | ||
| } | ||
| return null; | ||
| } |
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.
execute() must return a Result; numeric branch currently does nothing.
Return null causes NPEs upstream; also the int branch ignores comparisonResult.
Apply:
@Override
protected Result execute() throws NoSuchElementException {
logger.info("=== Verify If Strings Match The Operator: Starting Execution ===");
- com.testsigma.sdk.Result result = com.testsigma.sdk.Result.SUCCESS;
-
if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) {
setErrorMessage("One or more input values are null");
- return com.testsigma.sdk.Result.FAILED;
+ return Result.FAILED;
}
String str1 = testData1.getValue().toString();
String str2 = testData2.getValue().toString();
String op = operator.getValue().toString();
- // try int comparison first
+ // try numeric comparison first
try {
- int num1 = Integer.parseInt(str1);
- int num2 = Integer.parseInt(str2);
- boolean comparisonResult = compareNumbers(num1, num2, op);
-
+ double num1 = Double.parseDouble(str1);
+ double num2 = Double.parseDouble(str2);
+ boolean comparisonResult = compareNumbers(num1, num2, op);
+ if (!comparisonResult) {
+ setErrorMessage("Number comparison failed: " + str1 + " " + op + " " + str2);
+ return Result.FAILED;
+ } else {
+ setSuccessMessage("Number comparison succeeded: " + str1 + " " + op + " " + str2);
+ return Result.SUCCESS;
+ }
} catch (NumberFormatException e) {
// If parsing fails, fall back to string comparison
boolean comparisonResult = compareStrings(str1, str2, op);
if (!comparisonResult) {
- result = com.testsigma.sdk.Result.FAILED;
setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2);
- } else {
- setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2);
- }
+ return Result.FAILED;
+ } else {
+ setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2);
+ return Result.SUCCESS;
+ }
}
- 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.
| protected Result execute() throws NoSuchElementException { | |
| logger.info("=== Verify If Strings Match The Operator: Starting Execution ==="); | |
| com.testsigma.sdk.Result result = com.testsigma.sdk.Result.SUCCESS; | |
| if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) { | |
| setErrorMessage("One or more input values are null"); | |
| return com.testsigma.sdk.Result.FAILED; | |
| } | |
| String str1 = testData1.getValue().toString(); | |
| String str2 = testData2.getValue().toString(); | |
| String op = operator.getValue().toString(); | |
| // try int comparison first | |
| try { | |
| int num1 = Integer.parseInt(str1); | |
| int num2 = Integer.parseInt(str2); | |
| boolean comparisonResult = compareNumbers(num1, num2, op); | |
| } catch (NumberFormatException e) { | |
| // If parsing fails, fall back to string comparison | |
| boolean comparisonResult = compareStrings(str1, str2, op); | |
| if (!comparisonResult) { | |
| result = com.testsigma.sdk.Result.FAILED; | |
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | |
| } else { | |
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | |
| } | |
| } | |
| return null; | |
| } | |
| @Override | |
| protected Result execute() throws NoSuchElementException { | |
| logger.info("=== Verify If Strings Match The Operator: Starting Execution ==="); | |
| if (testData1.getValue() == null | |
| || testData2.getValue() == null | |
| || operator.getValue() == null) { | |
| setErrorMessage("One or more input values are null"); | |
| return Result.FAILED; | |
| } | |
| String str1 = testData1.getValue().toString(); | |
| String str2 = testData2.getValue().toString(); | |
| String op = operator.getValue().toString(); | |
| // try numeric comparison first | |
| try { | |
| double num1 = Double.parseDouble(str1); | |
| double num2 = Double.parseDouble(str2); | |
| boolean comparisonResult = compareNumbers(num1, num2, op); | |
| if (!comparisonResult) { | |
| setErrorMessage("Number comparison failed: " + str1 + " " + op + " " + str2); | |
| return Result.FAILED; | |
| } else { | |
| setSuccessMessage("Number comparison succeeded: " + str1 + " " + op + " " + str2); | |
| return Result.SUCCESS; | |
| } | |
| } catch (NumberFormatException e) { | |
| // If parsing fails, fall back to string comparison | |
| boolean comparisonResult = compareStrings(str1, str2, op); | |
| if (!comparisonResult) { | |
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | |
| return Result.FAILED; | |
| } else { | |
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | |
| return Result.SUCCESS; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyIfStringsMatchTheOperator.java
around lines 31 to 60, the execute() method currently returns null and the
numeric comparison branch parses integers but ignores the comparisonResult;
update the numeric branch to check comparisonResult, set result to FAILED or
SUCCESS accordingly and call setErrorMessage or setSuccessMessage with a clear
message like the string branch does, then ensure the method returns the result
(not null) at the end; also remove the unused local com.testsigma.sdk.Result
result = SUCCESS or use it consistently so the method always returns the correct
com.testsigma.sdk.Result value.
| private boolean compareNumbers(int num1, int num2, String op) { | ||
| switch (op) { | ||
| case "==": | ||
| return num1 == num2; | ||
| case "<": | ||
| case "less than": | ||
| return num1 < num2; | ||
| case ">": | ||
| return num1 > num2; | ||
| case "<=": | ||
| return num1 <= num2; | ||
| case ">=": | ||
| return num1 >= num2; | ||
| case "!=": | ||
| return num1 != num2; | ||
| case "contains": | ||
| // For numbers, convert to string and check if one contains the other | ||
| return String.valueOf(num1).contains(String.valueOf(num2)); | ||
| default: | ||
| setErrorMessage("Invalid operator: " + op); | ||
| return false; | ||
| } | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Support decimals in numeric comparisons.
Switch compareNumbers to doubles to avoid truncation.
Apply:
- private boolean compareNumbers(int num1, int num2, String op) {
+ private boolean compareNumbers(double num1, double num2, String op) {
switch (op) {
case "==":
return num1 == num2;
case "<":
case "less than":
return num1 < num2;
case ">":
return num1 > num2;
case "<=":
return num1 <= num2;
case ">=":
return num1 >= num2;
case "!=":
return num1 != num2;
case "contains":
// For numbers, convert to string and check if one contains the other
return String.valueOf(num1).contains(String.valueOf(num2));
default:
setErrorMessage("Invalid operator: " + op);
return false;
}
}📝 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 boolean compareNumbers(int num1, int num2, String op) { | |
| switch (op) { | |
| case "==": | |
| return num1 == num2; | |
| case "<": | |
| case "less than": | |
| return num1 < num2; | |
| case ">": | |
| return num1 > num2; | |
| case "<=": | |
| return num1 <= num2; | |
| case ">=": | |
| return num1 >= num2; | |
| case "!=": | |
| return num1 != num2; | |
| case "contains": | |
| // For numbers, convert to string and check if one contains the other | |
| return String.valueOf(num1).contains(String.valueOf(num2)); | |
| default: | |
| setErrorMessage("Invalid operator: " + op); | |
| return false; | |
| } | |
| } | |
| private boolean compareNumbers(double num1, double num2, String op) { | |
| switch (op) { | |
| case "==": | |
| return num1 == num2; | |
| case "<": | |
| case "less than": | |
| return num1 < num2; | |
| case ">": | |
| return num1 > num2; | |
| case "<=": | |
| return num1 <= num2; | |
| case ">=": | |
| return num1 >= num2; | |
| case "!=": | |
| return num1 != num2; | |
| case "contains": | |
| // For numbers, convert to string and check if one contains the other | |
| return String.valueOf(num1).contains(String.valueOf(num2)); | |
| default: | |
| setErrorMessage("Invalid operator: " + op); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyIfStringsMatchTheOperator.java
around lines 61 to 83, the compareNumbers method currently uses ints which
truncates decimals; change it to use doubles (e.g., compareDoubles) and update
the method signature and all call sites to pass parsed doubles or parse inside
the method. Parse inputs to double with proper try/catch for
NumberFormatException and call setErrorMessage(...) and return false on parse
failure. Replace integer operators with double comparisons (==, <, >, <=, >=,
!=) and for "contains" convert the doubles to string (preserving decimal
representation) before checking contains. Ensure no integer-specific operations
remain and adjust return types/signatures accordingly.
| if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) { | ||
| setErrorMessage("One or more input values are null"); | ||
| return com.testsigma.sdk.Result.FAILED; | ||
| } | ||
| String str1 = testData1.getValue().toString(); | ||
| String str2 = testData2.getValue().toString(); | ||
| String op = operator.getValue().toString(); | ||
|
|
||
| // try int comparison first | ||
| try { | ||
| int num1 = Integer.parseInt(str1); | ||
| int num2 = Integer.parseInt(str2); | ||
| boolean comparisonResult = compareNumbers(num1, num2, op); | ||
|
|
||
| } catch (NumberFormatException e) { | ||
| // If parsing fails, fall back to string comparison | ||
| boolean comparisonResult = compareStrings(str1, str2, op); | ||
| if (!comparisonResult) { | ||
| result = com.testsigma.sdk.Result.FAILED; | ||
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | ||
| } else { | ||
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | ||
| } | ||
| } | ||
| return null; |
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.
Fix result handling in execute()
The action never returns a Result: the numeric branch ignores comparisonResult, and the method ends with return null. That breaks the IF condition (null result) and lets failed numeric comparisons pass silently. We need to evaluate the comparison and return SUCCESS/FAILED with the right messages.
@@
- com.testsigma.sdk.Result result = com.testsigma.sdk.Result.SUCCESS;
-
- if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) {
+ if (testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) {
setErrorMessage("One or more input values are null");
return com.testsigma.sdk.Result.FAILED;
}
String str1 = testData1.getValue().toString();
String str2 = testData2.getValue().toString();
String op = operator.getValue().toString();
// try int comparison first
try {
int num1 = Integer.parseInt(str1);
int num2 = Integer.parseInt(str2);
boolean comparisonResult = compareNumbers(num1, num2, op);
-
+ if (!comparisonResult) {
+ if (getErrorMessage() == null) {
+ setErrorMessage("Numeric comparison failed: " + num1 + " " + op + " " + num2);
+ }
+ return com.testsigma.sdk.Result.FAILED;
+ }
+ setSuccessMessage("Numeric comparison succeeded: " + num1 + " " + op + " " + num2);
+ return com.testsigma.sdk.Result.SUCCESS;
} catch (NumberFormatException e) {
// If parsing fails, fall back to string comparison
boolean comparisonResult = compareStrings(str1, str2, op);
if (!comparisonResult) {
- result = com.testsigma.sdk.Result.FAILED;
- setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2);
- } else {
- setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2);
+ if (getErrorMessage() == null) {
+ setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2);
+ }
+ return com.testsigma.sdk.Result.FAILED;
}
+ setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2);
+ return com.testsigma.sdk.Result.SUCCESS;
}
- return null;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowLite/VerifyTestDataInIfcondition.java
around lines 37-61, the execute() method never returns a proper
com.testsigma.sdk.Result and the numeric branch ignores comparisonResult;
initialize a Result variable before the try, evaluate comparisonResult in the
try block (set result to SUCCESS or FAILED and call
setSuccessMessage/setErrorMessage with the numeric comparison details), keep the
catch block behavior for string comparisons but also set that same result
variable there, then return the result variable at the end instead of null.
| private boolean compareNumbers(int num1, int num2, String op) { | ||
| switch (op) { | ||
| case "==": | ||
| return num1 == num2; | ||
| case "<": | ||
| case "less than": | ||
| return num1 < num2; | ||
| case ">": | ||
| return num1 > num2; | ||
| case "<=": | ||
| return num1 <= num2; | ||
| case ">=": | ||
| return num1 >= num2; | ||
| case "!=": | ||
| return num1 != num2; | ||
| case "contains": | ||
| // For numbers, convert to string and check if one contains the other | ||
| return String.valueOf(num1).contains(String.valueOf(num2)); | ||
| default: |
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.
🛠️ Refactor suggestion | 🟠 Major
Use doubles for numeric comparisons.
Avoid int truncation.
- private boolean compareNumbers(int num1, int num2, String op) {
+ private boolean compareNumbers(double num1, double num2, String op) {Keep body as in lite fix.
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyIfStringsMatchTheOperator.java
around lines 61 to 79, the numeric comparison currently uses int which can
truncate decimals; change the method to use double (or parse inputs to double)
for comparisons and string containment: update the method signature/variables
from int to double, perform comparisons with double operators, and for the
"contains" case convert doubles to String (e.g., Double.toString) before
checking contains; keep the rest of the method body logic the same as the lite
fix.
| @TestData(reference = "operator", | ||
| allowedValues = {"==", ">", "<=", ">=", "!=", "less than", "contains"}) | ||
| private com.testsigma.sdk.TestData operator; | ||
|
|
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.
Add missing '<' to allowed operator values.
Enable the operator that your code already supports.
- @TestData(reference = "operator",
- allowedValues = {"==", ">", "<=", ">=", "!=", "less than", "contains"})
+ @TestData(reference = "operator",
+ allowedValues = {"==", "<", ">", "<=", ">=", "!=", "less than", "contains"})📝 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.
| @TestData(reference = "operator", | |
| allowedValues = {"==", ">", "<=", ">=", "!=", "less than", "contains"}) | |
| private com.testsigma.sdk.TestData operator; | |
| @TestData(reference = "operator", | |
| allowedValues = {"==", "<", ">", "<=", ">=", "!=", "less than", "contains"}) |
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyTestDataInIfcondition.java
around lines 24 to 27, the allowedValues for the 'operator' TestData is missing
the "<" operator even though the code supports it; update the allowedValues
array to include "<" (e.g., add "<" into the list alongside the other operators)
so the operator is enabled and selectable.
| @Override | ||
| protected Result execute() throws NoSuchElementException { | ||
| logger.info("=== Verify If Strings Match The Operator: Starting Execution ==="); | ||
| Result result = Result.SUCCESS; | ||
|
|
||
| if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) { | ||
| setErrorMessage("One or more input values are null"); | ||
| return Result.FAILED; | ||
| } | ||
| String str1 = testData1.getValue().toString(); | ||
| String str2 = testData2.getValue().toString(); | ||
| String op = operator.getValue().toString(); | ||
|
|
||
| // try int comparison first | ||
| try { | ||
| int num1 = Integer.parseInt(str1); | ||
| int num2 = Integer.parseInt(str2); | ||
| boolean comparisonResult = compareNumbers(num1, num2, op); | ||
|
|
||
| } catch (NumberFormatException e) { | ||
| // If parsing fails, fall back to string comparison | ||
| boolean comparisonResult = compareStrings(str1, str2, op); | ||
| if (!comparisonResult) { | ||
| result = Result.FAILED; | ||
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | ||
| } else { | ||
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | ||
| } | ||
| } | ||
| return null; | ||
| } |
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.
Return a Result; handle numeric comparison outcome.
Return null is incorrect; numeric path ignores result.
@Override
protected Result execute() throws NoSuchElementException {
logger.info("=== Verify If Strings Match The Operator: Starting Execution ===");
- Result result = Result.SUCCESS;
-
if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) {
setErrorMessage("One or more input values are null");
return Result.FAILED;
}
String str1 = testData1.getValue().toString();
String str2 = testData2.getValue().toString();
String op = operator.getValue().toString();
- // try int comparison first
+ // try numeric comparison first
try {
- int num1 = Integer.parseInt(str1);
- int num2 = Integer.parseInt(str2);
- boolean comparisonResult = compareNumbers(num1, num2, op);
-
+ double num1 = Double.parseDouble(str1);
+ double num2 = Double.parseDouble(str2);
+ boolean comparisonResult = compareNumbers(num1, num2, op);
+ if (!comparisonResult) {
+ setErrorMessage("Number comparison failed: " + str1 + " " + op + " " + str2);
+ return Result.FAILED;
+ } else {
+ setSuccessMessage("Number comparison succeeded: " + str1 + " " + op + " " + str2);
+ return Result.SUCCESS;
+ }
} catch (NumberFormatException e) {
// If parsing fails, fall back to string comparison
boolean comparisonResult = compareStrings(str1, str2, op);
if (!comparisonResult) {
- result = Result.FAILED;
setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2);
- } else {
- setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2);
- }
+ return Result.FAILED;
+ } else {
+ setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2);
+ return Result.SUCCESS;
+ }
}
- 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.
| @Override | |
| protected Result execute() throws NoSuchElementException { | |
| logger.info("=== Verify If Strings Match The Operator: Starting Execution ==="); | |
| Result result = Result.SUCCESS; | |
| if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) { | |
| setErrorMessage("One or more input values are null"); | |
| return Result.FAILED; | |
| } | |
| String str1 = testData1.getValue().toString(); | |
| String str2 = testData2.getValue().toString(); | |
| String op = operator.getValue().toString(); | |
| // try int comparison first | |
| try { | |
| int num1 = Integer.parseInt(str1); | |
| int num2 = Integer.parseInt(str2); | |
| boolean comparisonResult = compareNumbers(num1, num2, op); | |
| } catch (NumberFormatException e) { | |
| // If parsing fails, fall back to string comparison | |
| boolean comparisonResult = compareStrings(str1, str2, op); | |
| if (!comparisonResult) { | |
| result = Result.FAILED; | |
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | |
| } else { | |
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | |
| } | |
| } | |
| return null; | |
| } | |
| @Override | |
| protected Result execute() throws NoSuchElementException { | |
| logger.info("=== Verify If Strings Match The Operator: Starting Execution ==="); | |
| if(testData1.getValue() == null || testData2.getValue() == null || operator.getValue() == null) { | |
| setErrorMessage("One or more input values are null"); | |
| return Result.FAILED; | |
| } | |
| String str1 = testData1.getValue().toString(); | |
| String str2 = testData2.getValue().toString(); | |
| String op = operator.getValue().toString(); | |
| // try numeric comparison first | |
| try { | |
| double num1 = Double.parseDouble(str1); | |
| double num2 = Double.parseDouble(str2); | |
| boolean comparisonResult = compareNumbers(num1, num2, op); | |
| if (!comparisonResult) { | |
| setErrorMessage("Number comparison failed: " + str1 + " " + op + " " + str2); | |
| return Result.FAILED; | |
| } else { | |
| setSuccessMessage("Number comparison succeeded: " + str1 + " " + op + " " + str2); | |
| return Result.SUCCESS; | |
| } | |
| } catch (NumberFormatException e) { | |
| // If parsing fails, fall back to string comparison | |
| boolean comparisonResult = compareStrings(str1, str2, op); | |
| if (!comparisonResult) { | |
| setErrorMessage("String comparison failed: " + str1 + " " + op + " " + str2); | |
| return Result.FAILED; | |
| } else { | |
| setSuccessMessage("String comparison succeeded: " + str1 + " " + op + " " + str2); | |
| return Result.SUCCESS; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyTestDataInIfcondition.java
around lines 28 to 58, the method currently returns null and the
numeric-comparison path computes comparisonResult but never uses it; fix by
using the boolean from compareNumbers to set result and messages just like the
string path does (on false set Result.FAILED and setErrorMessage, on true set
Result.SUCCESS and setSuccessMessage), ensure comparisonResult is declared in a
scope you can evaluate, and replace the final return null with return result.
| private boolean compareNumbers(int num1, int num2, String op) { | ||
| switch (op) { | ||
| case "==": | ||
| return num1 == num2; | ||
| case "<": | ||
| case "less than": | ||
| return num1 < num2; | ||
| case ">": | ||
| return num1 > num2; | ||
| case "<=": | ||
| return num1 <= num2; | ||
| case ">=": | ||
| return num1 >= num2; | ||
| case "!=": | ||
| return num1 != num2; | ||
| case "contains": | ||
| // For numbers, convert to string and check if one contains the other | ||
| return String.valueOf(num1).contains(String.valueOf(num2)); | ||
| default: | ||
| setErrorMessage("Invalid operator: " + op); | ||
| return false; | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Use doubles for numeric comparisons.
Keep operator semantics identical to other classes.
- private boolean compareNumbers(int num1, int num2, String op) {
+ private boolean compareNumbers(double num1, double num2, String op) {Retain the rest; convert to String for "contains" if needed.
📝 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 boolean compareNumbers(int num1, int num2, String op) { | |
| switch (op) { | |
| case "==": | |
| return num1 == num2; | |
| case "<": | |
| case "less than": | |
| return num1 < num2; | |
| case ">": | |
| return num1 > num2; | |
| case "<=": | |
| return num1 <= num2; | |
| case ">=": | |
| return num1 >= num2; | |
| case "!=": | |
| return num1 != num2; | |
| case "contains": | |
| // For numbers, convert to string and check if one contains the other | |
| return String.valueOf(num1).contains(String.valueOf(num2)); | |
| default: | |
| setErrorMessage("Invalid operator: " + op); | |
| return false; | |
| } | |
| private boolean compareNumbers(double num1, double num2, String op) { | |
| switch (op) { | |
| case "==": | |
| return num1 == num2; | |
| case "<": | |
| case "less than": | |
| return num1 < num2; | |
| case ">": | |
| return num1 > num2; | |
| case "<=": | |
| return num1 <= num2; | |
| case ">=": | |
| return num1 >= num2; | |
| case "!=": | |
| return num1 != num2; | |
| case "contains": | |
| // For numbers, convert to string and check if one contains the other | |
| return String.valueOf(num1).contains(String.valueOf(num2)); | |
| default: | |
| setErrorMessage("Invalid operator: " + op); | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In
verify_string_actions/src/main/java/com/testsigma/addons/WindowsAdvanced/VerifyTestDataInIfcondition.java
around lines 59-80, the numeric comparison method should use doubles not ints:
change the method to accept double (or parse inputs to double) and perform
comparisons using double semantics for "==", "<", ">", "<=", ">=", "!=" (use
direct double comparisons or Double.compare to match other classes' behavior),
keep the "contains" case by converting the numbers to String and checking
contains, and ensure callers are updated to pass/parse doubles and that
setErrorMessage/default behavior remains unchanged.
| @@ -0,0 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNDBkMjhiNS0xNmUwLThlNmYtOWQ0ZS05MjYxMGNiZTcyYzciLCJ1bmlxdWVJZCI6IjU1MTUiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiNDMifQ.hs6fts2pQASHimQdNiwyAo3JA3bPUpVXqlGHT8P-PE8qTWUxq_4PfyufLmR7RfdNFMibDOa80-zMX-_Gg5mvpQ No newline at end of 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.
Remove committed API key immediately; rotate the credential.
A live token is hardcoded in a resource that will be shaded into the jar. This is a severe secret leak.
Actions:
- Revoke/rotate the key in Testsigma.
- Remove this property from the repo and the built artifact. Load the key from a secure secret store or environment at runtime instead.
- If a properties file is required, commit only a template (e.g., testsigma-sdk.properties.example) with a placeholder.
Apply one of:
- testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9...
+ # testsigma-sdk.api.key=${TESTSIGMA_API_KEY}and configure secure injection, or delete the file entirely from resources. Ensure you purge the secret from git history if the repo is public.
📝 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.
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNDBkMjhiNS0xNmUwLThlNmYtOWQ0ZS05MjYxMGNiZTcyYzciLCJ1bmlxdWVJZCI6IjU1MTUiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiNDMifQ.hs6fts2pQASHimQdNiwyAo3JA3bPUpVXqlGHT8P-PE8qTWUxq_4PfyufLmR7RfdNFMibDOa80-zMX-_Gg5mvpQ | |
| # testsigma-sdk.api.key=${TESTSIGMA_API_KEY} |
🤖 Prompt for AI Agents
In verify_string_actions/src/main/resources/testsigma-sdk.properties lines 1-1
there is a committed live Testsigma API key which must be revoked and removed:
immediately rotate/revoke the exposed key in Testsigma, delete this properties
file from the repository and any build inputs so it is not shaded into the jar,
replace it in source with a template file (e.g.,
testsigma-sdk.properties.example) containing a placeholder value, change the
application to read the real key from a secure source at runtime (environment
variable, secrets manager, or CI secret injection) and update
documentation/README to show how to provide the secret; finally purge the secret
from git history (use git filter-repo or BFG) if the repo is public or shared.
Please review this add-on and approve as public
addon name: verify_string_actions
Addon account : https://jarvis.testsigma.com/ui/tenants/2817/addons
Ticket : https://testsigma.atlassian.net/browse/CUS-7924
Summary by CodeRabbit
Release Notes
New Features
Chores