-
Notifications
You must be signed in to change notification settings - Fork 16
[CUS-9851] Added a class to verify excel cells and updated success msg. #292
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
📝 WalkthroughWalkthroughThis PR adds a new utility class Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VerifyAction as VerifyIfCellValuesInExcel
participant Loader as File Loader
participant Util as ExcelCellUtils
participant XL1 as Excel File 1
participant XL2 as Excel File 2
User->>VerifyAction: execute(excel-path-1, excel-path-2, row, col, sheet)
rect rgb(200, 220, 240)
Note over VerifyAction: Load Excel Files
VerifyAction->>Loader: load(path-1)
Loader->>XL1: open/download
XL1-->>VerifyAction: XSSFWorkbook 1
VerifyAction->>Loader: load(path-2)
Loader->>XL2: open/download
XL2-->>VerifyAction: XSSFWorkbook 2
end
rect rgb(220, 240, 220)
Note over VerifyAction: Validate & Extract
VerifyAction->>VerifyAction: validate sheet index
VerifyAction->>Util: getCellValueAsString(sheet-1, row, col)
Util-->>VerifyAction: value-1
VerifyAction->>Util: getCellValueAsString(sheet-2, row, col)
Util-->>VerifyAction: value-2
end
rect rgb(240, 230, 200)
Note over VerifyAction: Compare & Result
alt Values Match
VerifyAction->>VerifyAction: return SUCCESS
else Values Differ
VerifyAction->>VerifyAction: return FAILED
end
end
VerifyAction-->>User: Result (SUCCESS/FAILED)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheet.java (1)
62-72: PotentialNullPointerExceptionif file download fails.If
excelutil.copyFileFromDownloads()throws an exception,downloadedExcelFileremainsnull, but execution continues to line 72 wherenew FileInputStream(downloadedExcelFile)will throw an NPE. Add an early return after handling the exception.🔎 Proposed fix
} catch (Exception el) { String errorMessage = ExceptionUtils.getStackTrace(el); result = com.testsigma.sdk.Result.FAILED; setErrorMessage(errorMessage); logger.warn(errorMessage); + return result; }
🧹 Nitpick comments (7)
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteNewExcelFile.java (1)
54-66: Potential resource leak:FileInputStreamandFileOutputStreamnot properly managed.If an exception occurs between lines 55-65, the
inputStreamwon't be closed. Consider using try-with-resources for both streams to ensure proper cleanup.🔎 Suggested refactor using try-with-resources
- FileInputStream inputStream = new FileInputStream(downloadedExcelFile); - Workbook workbook = new XSSFWorkbook(inputStream); + try (FileInputStream inputStream = new FileInputStream(downloadedExcelFile); + Workbook workbook = new XSSFWorkbook(inputStream)) { Sheet sheet = workbook.getSheetAt(0); int newRowNumber = sheet.getLastRowNum() + 1; Row newRow = sheet.createRow(newRowNumber); Cell cell = newRow.createCell(0); cell.setCellValue(dataValues); - FileOutputStream outputStream = new FileOutputStream(downloadedExcelFile); - workbook.write(outputStream); - workbook.close(); - outputStream.close(); + try (FileOutputStream outputStream = new FileOutputStream(downloadedExcelFile)) { + workbook.write(outputStream); + } runTimeData.setValue(Integer.toString(newRowNumber)); runTimeData.setKey(testData.getValue().toString()); String successMsg = "Successfully wrote value '" + dataValues + "' to new row " + newRowNumber + " in Sheet index: 0.<br>File path: " + downloadedExcelFile.getAbsolutePath(); logger.info(successMsg.replace("<br>", " ")); setSuccessMessage(successMsg); + }excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheet.java (1)
54-56: Minor: Double semicolons.Lines 54-56 have trailing double semicolons (
;;). While harmless, they should be cleaned up.🔎 Proposed fix
- int rowIndex = Integer.parseInt(testData1.getValue().toString());; - int columnIndex = Integer.parseInt(testData2.getValue().toString());; - int sheetIndex = Integer.parseInt(testData4.getValue().toString());; + int rowIndex = Integer.parseInt(testData1.getValue().toString()); + int columnIndex = Integer.parseInt(testData2.getValue().toString()); + int sheetIndex = Integer.parseInt(testData4.getValue().toString());excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java (2)
56-58: Minor: Double semicolons.Lines 56-58 have trailing double semicolons (
;;). Consider removing the extras.🔎 Proposed fix
- int rowIndex = Integer.parseInt(testData1.getValue().toString());; - int columnIndex = Integer.parseInt(testData2.getValue().toString());; - int sheetIndex = Integer.parseInt(testData4.getValue().toString());; + int rowIndex = Integer.parseInt(testData1.getValue().toString()); + int columnIndex = Integer.parseInt(testData2.getValue().toString()); + int sheetIndex = Integer.parseInt(testData4.getValue().toString());
117-130: Consider cleaning up temporary files.The downloaded temporary files are never deleted. For long-running processes or frequent executions, this could accumulate temporary files on disk. Consider using
tempFile.deleteOnExit()after creation.🔎 Proposed fix
File tempFile = File.createTempFile("downloaded-", fileName); + tempFile.deleteOnExit(); try (InputStream in = url.openStream();excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifyIfCellValuesInExcel.java (1)
62-63: Consider cleaning up temporary downloaded files.When files are downloaded via
getExcelFile(), temporary files are created but never deleted. This could lead to disk space issues over time. Consider cleaning them up after comparison is complete.🔎 Proposed approach
Track downloaded files and clean them up in a
finallyblock:File excelFile1 = null; File excelFile2 = null; boolean isFile1Temp = filePath1.startsWith("http://") || filePath1.startsWith("https://"); boolean isFile2Temp = filePath2.startsWith("http://") || filePath2.startsWith("https://"); try { excelFile1 = getExcelFile(filePath1); excelFile2 = getExcelFile(filePath2); // ... rest of the logic } finally { // Clean up temp files if (isFile1Temp && excelFile1 != null && excelFile1.exists()) { excelFile1.delete(); } if (isFile2Temp && excelFile2 != null && excelFile2.exists()) { excelFile2.delete(); } }Alternatively, use
tempFile.deleteOnExit()in thedownloadFile()method.excelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.java (2)
46-47: Consider using a date formatter for better readability.
cell.getDateCellValue().toString()produces a format likeSat Dec 26 00:00:00 EST 2025. Consider usingSimpleDateFormatorDateTimeFormatterfor a more user-friendly format.🔎 Example improvement
import java.text.SimpleDateFormat; // In the NUMERIC case: if (DateUtil.isCellDateFormatted(cell)) { SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); return sdf.format(cell.getDateCellValue()); }
82-117: Consider reducing code duplication.
getCellValueOrEmptyduplicates most logic fromgetCellValueAsString. You could refactor to share common logic, for example by extracting a private helper method that takes a "placeholder strategy" or by having one method call the other with placeholder replacement.🔎 Possible approach
public static String getCellValueOrEmpty(Cell cell) { String value = getCellValueAsString(cell); switch (value) { case "[EMPTY CELL]": case "[BLANK]": case "[EMPTY ROW]": return ""; case "[UNKNOWN]": return "N/A"; default: return value; } }This is optional since the current implementation is clear and works correctly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
excelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/VerifyIfCellValuesInExcel.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheet.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteNewExcelFile.java
🔇 Additional comments (9)
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteNewExcelFile.java (1)
71-74: LGTM!The detailed success message provides useful feedback including the written value, row number, sheet index, and file path. The HTML-to-space replacement for logging is appropriate.
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheet.java (1)
88-92: LGTM!The structured success message is well-formatted and provides comprehensive details about the write operation.
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java (2)
21-22: LGTM!The annotation update adds a space before "(supports upload section)" for better readability.
97-101: LGTM!The success message follows the same structured format as the other Excel action classes.
excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifyIfCellValuesInExcel.java (3)
98-114: LGTM!The comparison logic correctly uses
.equals()for string comparison. The placeholder strings fromExcelCellUtils([EMPTY ROW],[EMPTY CELL],[BLANK]) allow meaningful comparisons even for edge cases.
131-146: LGTM!The method properly handles both local files and URLs, with appropriate file existence validation for local paths.
148-165: LGTM!The download implementation correctly uses try-with-resources and a reasonable buffer size.
excelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.java (2)
21-29: LGTM!The method correctly handles null rows and delegates cell handling to the specialized method.
58-67: LGTM!The fallback chain for FORMULA cells is sensible: try evaluated string value, then numeric value, then the raw formula. This handles most formula scenarios gracefully.
please review this addon and publish as PUBLIC
Addon name : ExcelActions_cloud
Addon accont: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira: https://testsigma.atlassian.net/browse/CUS-9851
fix
added class to verify excel cell values.
updated classes to display proper success message
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.