[CUS-12130] updated add-on to store filename/path/extension#381
Open
ManojTestsigma wants to merge 1 commit intodevfrom
Open
[CUS-12130] updated add-on to store filename/path/extension#381ManojTestsigma wants to merge 1 commit intodevfrom
ManojTestsigma wants to merge 1 commit intodevfrom
Conversation
📝 WalkthroughWalkthroughVersion 1.0.3 release adds two new web actions for browser-downloaded files: Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Action
participant Browser as Browser Downloads
participant FileOp as File System
participant Runtime as Runtime Variable
participant Util as Utilities
Test->>Browser: Identify latest downloaded file
alt Return file path
Test->>Util: Copy file from downloads
Util->>FileOp: Create local temp copy
FileOp-->>Test: Local file path
Test->>Runtime: Store file-path
else Return file metadata
Test->>Browser: Open temp blank tab
Test->>Browser: Wait for download completion
Browser-->>Test: Download confirmed
Test->>FileOp: Parse filename from path
FileOp-->>Test: Extract name/extension
Test->>Runtime: Store file-name or file-extension
Test->>Browser: Close temp tab
end
sequenceDiagram
participant Test as Test Action
participant Browser as Browser Downloads
participant FileOp as File System
participant Util as Utilities
Test->>Test: Normalize expected file type
Test->>Test: Map extension to MIME type
Test->>Util: Create utilities instance
Test->>Util: Copy latest downloaded file
Util->>FileOp: Write to temp location
FileOp-->>Test: Temp file created
Test->>FileOp: Probe file MIME type
FileOp-->>Test: Detected MIME type
alt MIME matches expected
Test-->>Test: Set success message
else MIME mismatch
Test-->>Test: Set error message
end
Test->>FileOp: Delete temp file
Test->>Browser: Switch to original window
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 3
🧹 Nitpick comments (2)
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/VerifyIfTheLatestFileType.java (1)
61-63: IgnoredFile.delete()result and no cleanup on probe failure path.
localFile.delete()return value is ignored — if deletion fails (e.g. file locked), the temp file silently lingers. Also, sincecopyFileFromDownloadswrites a temp file, consider usingFiles.delete(path)to surface the failure in the log, or register the temp file for deletion on JVM exit (File.deleteOnExit()) as a safety net.🧹 Suggested tweak
- String actualFileType; - try { - actualFileType = Files.probeContentType(localFile.toPath()); - } finally { - localFile.delete(); - } + localFile.deleteOnExit(); + String actualFileType; + try { + actualFileType = Files.probeContentType(localFile.toPath()); + } finally { + if (!localFile.delete()) { + logger.info("Could not delete temp file: " + localFile.getAbsolutePath()); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/VerifyIfTheLatestFileType.java` around lines 61 - 63, The finally block currently calls localFile.delete() and ignores the boolean result, so failed deletes silently leave temp files; update the cleanup in the copyFileFromDownloads flow to attempt a surfaced delete (use Files.delete(Path) and catch/log IOException) and also register the temp file as a safety net with localFile.deleteOnExit() when it is created; ensure you reference and update the cleanup around localFile (and the finally block) to log failures with processLogger or similar and avoid swallowing the delete result.get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFileName.java (1)
66-78:file-extensionreturns empty string for dotfiles and extensionless files.
dotIndex > 0plus thedotIndex + 1substring can silently return an empty extension (e.g.archive.or.bashrc). That's fine for truly extensionless files, but for trailing-dot names it's also empty. Consider whether callers should instead get a clearer failure (or at least log a warning) when no extension can be derived, so downstream assertions don't pass on an empty value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFileName.java` around lines 66 - 78, The current logic in GetLatestDownloadedFileName uses dotIndex > 0 and substring(dotIndex + 1) which returns an empty string for names like "archive." or dotfiles like ".bashrc"; change the "file-extension" branch so it only returns an extension when dotIndex > 0 AND dotIndex < originalFileName.length() - 1, otherwise set storedValue to null (or another sentinel) and emit a warning/logging message so downstream assertions don't silently accept an empty extension; update the branch that assigns storedValue for contentType "file-extension" to use this stricter check (referencing variables contentType, storedValue, originalFileName, dotIndex) and add a warning via the existing logger or a new one if none exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFileName.java`:
- Around line 23-26: Update the stale `@Action` description on the
GetLatestDownloadedFileName class so it enumerates all supported content values:
"file-name", "file-name-with-extension", and "file-extension" in addition to
retrieving the latest downloaded file path; edit the description string in the
`@Action` annotation (the annotation with actionText = "Get latest downloaded
content and store in runtime variable variable-name") to explicitly mention
these supported content values and their meanings so the annotation accurately
reflects current behavior.
- Around line 56-87: The blank tab creation and switch
(JavascriptExecutor.executeScript, driver.getWindowHandles,
driver.switchTo().window) occur before the try/finally that closes the tab, so
if any of those calls throw the temp tab can leak; move the tab-open and switch
logic inside the existing try block (or expand the try to start before
executeScript) in GetLatestDownloadedFileName so that the finally always runs
and always executes driver.close() and
driver.switchTo().window(originalWindowHandle); ensure you still capture
originalWindowHandle before opening the new tab and keep
utilities.isFileDownloaded(), utilities.getDownloadedFileLocalPath(), and the
contentType switch unchanged.
In
`@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/VerifyIfTheLatestFileType.java`:
- Around line 45-46: The current MIME check in VerifyIfTheLatestFileType (the
switch cases setting expectedMimeType for "xlsx" and "docx") relies on
Files.probeContentType which is platform-dependent and will cause flaky
failures; update the logic to treat expected mime types as a set of allowed
values per extension (e.g., for "xlsx" allow
application/vnd.openxmlformats-officedocument.spreadsheetml.sheet and
application/zip, for "docx" allow
application/vnd.openxmlformats-officedocument.wordprocessingml.document and
application/zip, and for "csv" allow text/csv and text/plain), and change the
validation path in the method that calls Files.probeContentType to: 1) first try
a content-based detector such as Apache Tika (org.apache.tika:tika-core) if
available, 2) if not available or detector returns null/ambiguous, fall back to
matching the file extension against the allowed set, and 3) consider
probeContentType returning null or generic types (like application/zip or
text/plain) as valid when they appear in the allowed set; update the switch that
defines expectedMimeType to produce collections (allowedMimeTypes) and modify
the comparison code to check containment instead of strict equality.
---
Nitpick comments:
In
`@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFileName.java`:
- Around line 66-78: The current logic in GetLatestDownloadedFileName uses
dotIndex > 0 and substring(dotIndex + 1) which returns an empty string for names
like "archive." or dotfiles like ".bashrc"; change the "file-extension" branch
so it only returns an extension when dotIndex > 0 AND dotIndex <
originalFileName.length() - 1, otherwise set storedValue to null (or another
sentinel) and emit a warning/logging message so downstream assertions don't
silently accept an empty extension; update the branch that assigns storedValue
for contentType "file-extension" to use this stricter check (referencing
variables contentType, storedValue, originalFileName, dotIndex) and add a
warning via the existing logger or a new one if none exists.
In
`@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/VerifyIfTheLatestFileType.java`:
- Around line 61-63: The finally block currently calls localFile.delete() and
ignores the boolean result, so failed deletes silently leave temp files; update
the cleanup in the copyFileFromDownloads flow to attempt a surfaced delete (use
Files.delete(Path) and catch/log IOException) and also register the temp file as
a safety net with localFile.deleteOnExit() when it is created; ensure you
reference and update the cleanup around localFile (and the finally block) to log
failures with processLogger or similar and avoid swallowing the delete result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d254082-c9c3-49f3-84b4-578f158df22e
📒 Files selected for processing (3)
get_latest_downloaded_file_path/pom.xmlget_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFileName.javaget_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/VerifyIfTheLatestFileType.java
Comment on lines
+56
to
+87
| ((JavascriptExecutor) driver).executeScript("window.open('about:blank','_blank');"); | ||
| List<String> tabs = new ArrayList<>(driver.getWindowHandles()); | ||
| driver.switchTo().window(tabs.get(tabs.size() - 1)); | ||
| try { | ||
| if (!utilities.isFileDownloaded()) { | ||
| throw new RuntimeException("File is still downloading."); | ||
| } | ||
| String originalPath = utilities.getDownloadedFileLocalPath(); | ||
| logger.info("Original downloaded file path: " + originalPath); | ||
|
|
||
| int lastSep = Math.max(originalPath.lastIndexOf('/'), originalPath.lastIndexOf('\\')); | ||
| String originalFileName = lastSep >= 0 ? originalPath.substring(lastSep + 1) : originalPath; | ||
| int dotIndex = originalFileName.lastIndexOf('.'); | ||
|
|
||
| switch (contentType) { | ||
| case "file-name": | ||
| storedValue = dotIndex > 0 ? originalFileName.substring(0, dotIndex) : originalFileName; | ||
| break; | ||
| case "file-name-with-extension": | ||
| storedValue = originalFileName; | ||
| break; | ||
| case "file-extension": | ||
| storedValue = dotIndex > 0 ? originalFileName.substring(dotIndex + 1) : ""; | ||
| break; | ||
| default: | ||
| storedValue = originalFileName; | ||
| break; | ||
| } | ||
| } finally { | ||
| driver.close(); | ||
| driver.switchTo().window(originalWindowHandle); | ||
| } |
There was a problem hiding this comment.
Possible temporary-tab leak on early failure.
The new blank tab is opened on line 56 and the switch happens on line 58, but the try/finally that restores the original window handle and closes the tab only starts at line 59. If executeScript, getWindowHandles, or switchTo().window(...) throws, the temp tab is left open and the driver may be pointed at it, which cascades into the outer catch where no cleanup runs.
Consider moving the tab creation/switch inside the try block (or wrap the whole else-branch in a try/finally that restores originalWindowHandle and closes any extra tabs).
🛠️ Suggested structure
- ((JavascriptExecutor) driver).executeScript("window.open('about:blank','_blank');");
- List<String> tabs = new ArrayList<>(driver.getWindowHandles());
- driver.switchTo().window(tabs.get(tabs.size() - 1));
- try {
+ try {
+ ((JavascriptExecutor) driver).executeScript("window.open('about:blank','_blank');");
+ List<String> tabs = new ArrayList<>(driver.getWindowHandles());
+ driver.switchTo().window(tabs.get(tabs.size() - 1));
if (!utilities.isFileDownloaded()) {
throw new RuntimeException("File is still downloading.");
}
...
} finally {
- driver.close();
- driver.switchTo().window(originalWindowHandle);
+ if (!driver.getWindowHandle().equals(originalWindowHandle)) {
+ driver.close();
+ }
+ driver.switchTo().window(originalWindowHandle);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ((JavascriptExecutor) driver).executeScript("window.open('about:blank','_blank');"); | |
| List<String> tabs = new ArrayList<>(driver.getWindowHandles()); | |
| driver.switchTo().window(tabs.get(tabs.size() - 1)); | |
| try { | |
| if (!utilities.isFileDownloaded()) { | |
| throw new RuntimeException("File is still downloading."); | |
| } | |
| String originalPath = utilities.getDownloadedFileLocalPath(); | |
| logger.info("Original downloaded file path: " + originalPath); | |
| int lastSep = Math.max(originalPath.lastIndexOf('/'), originalPath.lastIndexOf('\\')); | |
| String originalFileName = lastSep >= 0 ? originalPath.substring(lastSep + 1) : originalPath; | |
| int dotIndex = originalFileName.lastIndexOf('.'); | |
| switch (contentType) { | |
| case "file-name": | |
| storedValue = dotIndex > 0 ? originalFileName.substring(0, dotIndex) : originalFileName; | |
| break; | |
| case "file-name-with-extension": | |
| storedValue = originalFileName; | |
| break; | |
| case "file-extension": | |
| storedValue = dotIndex > 0 ? originalFileName.substring(dotIndex + 1) : ""; | |
| break; | |
| default: | |
| storedValue = originalFileName; | |
| break; | |
| } | |
| } finally { | |
| driver.close(); | |
| driver.switchTo().window(originalWindowHandle); | |
| } | |
| try { | |
| ((JavascriptExecutor) driver).executeScript("window.open('about:blank','_blank');"); | |
| List<String> tabs = new ArrayList<>(driver.getWindowHandles()); | |
| driver.switchTo().window(tabs.get(tabs.size() - 1)); | |
| if (!utilities.isFileDownloaded()) { | |
| throw new RuntimeException("File is still downloading."); | |
| } | |
| String originalPath = utilities.getDownloadedFileLocalPath(); | |
| logger.info("Original downloaded file path: " + originalPath); | |
| int lastSep = Math.max(originalPath.lastIndexOf('/'), originalPath.lastIndexOf('\\')); | |
| String originalFileName = lastSep >= 0 ? originalPath.substring(lastSep + 1) : originalPath; | |
| int dotIndex = originalFileName.lastIndexOf('.'); | |
| switch (contentType) { | |
| case "file-name": | |
| storedValue = dotIndex > 0 ? originalFileName.substring(0, dotIndex) : originalFileName; | |
| break; | |
| case "file-name-with-extension": | |
| storedValue = originalFileName; | |
| break; | |
| case "file-extension": | |
| storedValue = dotIndex > 0 ? originalFileName.substring(dotIndex + 1) : ""; | |
| break; | |
| default: | |
| storedValue = originalFileName; | |
| break; | |
| } | |
| } finally { | |
| if (!driver.getWindowHandle().equals(originalWindowHandle)) { | |
| driver.close(); | |
| } | |
| driver.switchTo().window(originalWindowHandle); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFileName.java`
around lines 56 - 87, The blank tab creation and switch
(JavascriptExecutor.executeScript, driver.getWindowHandles,
driver.switchTo().window) occur before the try/finally that closes the tab, so
if any of those calls throw the temp tab can leak; move the tab-open and switch
logic inside the existing try block (or expand the try to start before
executeScript) in GetLatestDownloadedFileName so that the finally always runs
and always executes driver.close() and
driver.switchTo().window(originalWindowHandle); ensure you still capture
originalWindowHandle before opening the new tab and keep
utilities.isFileDownloaded(), utilities.getDownloadedFileLocalPath(), and the
contentType switch unchanged.
vigneshtestsigma
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
please review this addon and publish as PUBLIC
Addon name : get_latest_downloaded_file_path
Addon accont: https://jarvis.testsigma.com/ui/tenants/3072/addons
Jira: https://testsigma.atlassian.net/browse/CUS-12130
fix
added 2 NLP's
Summary by CodeRabbit
New Features
Chores