-
Notifications
You must be signed in to change notification settings - Fork 16
feat/CUS-10043-Optimised the already present code and migrated to IN region #300
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
feat/CUS-10043-Optimised the already present code and migrated to IN region #300
Conversation
📝 WalkthroughWalkthroughThe pull request updates project dependencies in the Maven configuration with major version upgrades for Selenium (3.x to 4.x) and Appium (7.x to 9.x) libraries, adds Apache Commons Lang utility library, and refactors the sorting verification logic in Sorting.java to simplify the control flow with improved error handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @validate_order/pom.xml:
- Line 16: The pom property testsigma.sdk.version is set to a non-published
pre-release value; update the <testsigma.sdk.version> property to a valid
released version (e.g., 1.0.1 or later) so Maven can resolve the dependency;
locate the testsigma.sdk.version entry in the POM and replace "1.0.0.rc1" with
the chosen stable release string, then run a mvn dependency:resolve or build to
verify resolution.
In @validate_order/src/main/java/com/testsigma/addons/web/Sorting.java:
- Around line 40-41: The current code calls operator.getValue().toString() which
can NPE if getValue() is null; update the Sorting.java code around the operator
usage to guard against null by capturing the value into a variable (e.g., Object
val = operator.getValue()) and compute the order as either an empty string or
the lower-cased toString (e.g., String order = val == null ? "" :
val.toString().toLowerCase()), then log using logger.info("Sorting order
selected: " + order); this prevents NullPointerException while preserving the
logged output.
🧹 Nitpick comments (2)
validate_order/pom.xml (1)
43-45: TestNG version 6.14.3 is outdated.Consider updating to a newer TestNG version (7.x) for compatibility with Java 11+ and to receive bug fixes and improvements.
validate_order/src/main/java/com/testsigma/addons/web/Sorting.java (1)
92-98: Potential NPE ifgetText()returns null.While uncommon,
WebElement.getText()can return null for certain element states. Calling.trim()on null will throw a NullPointerException.Defensive null handling
private List<String> extractText(List<WebElement> elements) { List<String> data = new ArrayList<>(); for (WebElement element : elements) { - data.add(element.getText().trim()); + String text = element.getText(); + data.add(text != null ? text.trim() : ""); } return data; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
validate_order/pom.xmlvalidate_order/src/main/java/com/testsigma/addons/web/Sorting.java
🔇 Additional comments (7)
validate_order/src/main/java/com/testsigma/addons/web/Sorting.java (5)
55-56: Lexicographic sorting may not work for numeric columns.
Collections.sort(expectedList)performs lexicographic string comparison, so numeric data will sort incorrectly (e.g., "10" < "2" < "9"). If the column contains numeric data, this validation will produce false negatives.Is this addon intended only for text columns, or should it also handle numeric data? If numeric support is needed, consider detecting numeric patterns and using a numeric comparator.
20-24: LGTM!The reformatted annotations improve readability and clearly document the action's purpose and allowed test data values.
Also applies to: 27-30
45-49: LGTM!Good addition of the empty elements check with appropriate error messaging and logging before returning
FAILED.
76-88: LGTM!The structured exception handling properly distinguishes between assertion failures and unexpected exceptions, providing clear error messages for each case.
37-37: No action required — the exception declaration is compatible.The
execute()method signature correctly declaresthrows NoSuchElementException. SinceNoSuchElementExceptionis an unchecked exception (extendsRuntimeException), it can be declared in an overriding method without violating Java's method override rules. Only checked exceptions are constrained by the Liskov Substitution Principle; unchecked exceptions may be thrown or declared freely. Testsigma's documentation and examples confirm this pattern is correct and will compile without errors.Likely an incorrect or invalid review comment.
validate_order/pom.xml (2)
57-57: Version 9.4.0 is a valid published release.Appium java-client 9.4.0 is available on Maven Central (published January 21, 2025). No action needed.
51-51: No changes needed. Selenium version 4.33.0 is a valid release published to Maven Central (May 25, 2025) and will not cause build failures.Likely an incorrect or invalid review comment.
| <maven.compiler.source>11</maven.compiler.source> | ||
| <maven.compiler.target>11</maven.compiler.target> | ||
| <testsigma.sdk.version>1.2.2_cloud</testsigma.sdk.version> | ||
| <testsigma.sdk.version>1.0.0.rc1</testsigma.sdk.version> |
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.
🧩 Analysis chain
🌐 Web query:
testsigma SDK java version 1.0.0.rc1 stable release
💡 Result:
I couldn't find a stable release named "1.0.0.rc1" for the Testsigma Java SDK. The official GitHub repo for testsigma-java-sdk shows no published releases/tags, and Maven listings show published stable versions such as 1.0.1 (and later cloud versions). [1][2]
Sources:
- testsigma-java-sdk GitHub repository (no releases/tags published). [1]
- Maven / mvnrepository entry for com.testsigma:testsigma-java-sdk (example stable version 1.0.1). [2]
Replace testsigma.sdk.version with a valid published release.
Version 1.0.0.rc1 does not appear to exist as a published release on Maven repositories. Use a stable version such as 1.0.1 or later.
🤖 Prompt for AI Agents
In @validate_order/pom.xml at line 16, The pom property testsigma.sdk.version is
set to a non-published pre-release value; update the <testsigma.sdk.version>
property to a valid released version (e.g., 1.0.1 or later) so Maven can resolve
the dependency; locate the testsigma.sdk.version entry in the POM and replace
"1.0.0.rc1" with the chosen stable release string, then run a mvn
dependency:resolve or build to verify resolution.
| String order = operator.getValue().toString().toLowerCase(); | ||
| logger.info("Sorting order selected: " + order); |
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.
Potential NPE if operator.getValue() returns null.
If the test data value is not provided or is null, calling toString() will throw a NullPointerException. Consider adding a null check or using String.valueOf().
Proposed fix
- String order = operator.getValue().toString().toLowerCase();
+ Object value = operator.getValue();
+ if (value == null) {
+ setErrorMessage("Sorting order (ascending/descending) must be provided.");
+ return Result.FAILED;
+ }
+ String order = value.toString().toLowerCase();🤖 Prompt for AI Agents
In @validate_order/src/main/java/com/testsigma/addons/web/Sorting.java around
lines 40 - 41, The current code calls operator.getValue().toString() which can
NPE if getValue() is null; update the Sorting.java code around the operator
usage to guard against null by capturing the value into a variable (e.g., Object
val = operator.getValue()) and compute the order as either an empty string or
the lower-cased toString (e.g., String order = val == null ? "" :
val.toString().toLowerCase()), then log using logger.info("Sorting order
selected: " + order); this prevents NullPointerException while preserving the
logged output.
Publish this addon as public
Addon Name: Validate Order
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/30397/addons
Jira : https://testsigma.atlassian.net/browse/CUS-10043
Optimised the already present code
Publish this addon as public (IN Region)
Addon Name: Validate Order
Jarvis Link: https://jarvis-in.testsigma.com/ui/tenants/3/addons
Jira : https://testsigma.atlassian.net/browse/CUS-10043
Migrated to IN region
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.