[
https://issues.apache.org/jira/browse/TIKA-4704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18070129#comment-18070129
]
ASF GitHub Bot commented on TIKA-4704:
--------------------------------------
Copilot commented on code in PR #2728:
URL: https://github.com/apache/tika/pull/2728#discussion_r3018013286
##########
tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/CXFTestBase.java:
##########
@@ -201,7 +203,8 @@ public void setUp() throws Exception {
// Initialize PipesParsingHelper for pipes-based parsing
// Merge the fetcher config with basePath pointing to the temp
directory
- this.pipesConfigPath = mergeFetcherConfig(this.pipesConfigPath,
inputTempDirectory);
+ this.pipesConfigPath = mergeFetcherConfig(tmpPipesConfigPath,
inputTempDirectory);
+ Files.delete(tmpPipesConfigPath);
TikaJsonConfig tikaJsonConfig =
TikaJsonConfig.load(this.pipesConfigPath);
Review Comment:
`Files.delete(tmpPipesConfigPath)` is an unconditional hard delete in setup
and will (a) fail the entire test run if deletion fails (common on
Windows/locked files), and (b) won’t run if anything later in `setUp()` throws.
Consider making this best-effort (e.g., `deleteIfExists` inside a try/catch)
and/or moving cleanup into a `finally` block after `mergeFetcherConfig`
succeeds so intermediate temp files don’t leak on exceptions.
##########
tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/CXFTestBase.java:
##########
@@ -413,6 +422,7 @@ public void tearDown() throws Exception {
if (pipesConfigPath != null) {
try {
Files.deleteIfExists(pipesConfigPath);
+ LOG.info("Config file {} deleted", pipesConfigPath);
Review Comment:
The log line `LOG.info("Config file {} deleted", pipesConfigPath)` is
potentially inaccurate because `Files.deleteIfExists(...)` may return `false`
(file didn’t exist) without deleting anything. Consider logging only when the
return value is `true`, and/or using `debug` level to avoid noisy logs during
normal test runs.
```suggestion
boolean deleted = Files.deleteIfExists(pipesConfigPath);
if (deleted) {
LOG.info("Config file {} deleted", pipesConfigPath);
}
```
> Avoid remaining temp files
> --------------------------
>
> Key: TIKA-4704
> URL: https://issues.apache.org/jira/browse/TIKA-4704
> Project: Tika
> Issue Type: Task
> Affects Versions: 3.3.0
> Reporter: Tilman Hausherr
> Priority: Minor
> Fix For: 4.0.0, 3.3.1
>
> Attachments: screenshot-1.png
>
>
> This is my temp directory after a successful build of tika 3. We should try
> to lessen this.
> !screenshot-1.png!
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
