[jira] [Commented] (TIKA-4704) Avoid remaining temp files

[ASF GitHub Bot (Jira)]

    [ 
https://issues.apache.org/jira/browse/TIKA-4704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071284#comment-18071284
 ] 

ASF GitHub Bot commented on TIKA-4704:
--------------------------------------

Copilot commented on code in PR #2735:
URL: https://github.com/apache/tika/pull/2735#discussion_r3037153117


##########
tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerResourceTest.java:
##########
@@ -602,4 +604,11 @@ public void testShallowExtraction() throws Exception {
         assertEquals(200, response.getStatus());
         // Just verify it succeeds - actual depth limiting behavior depends on 
document structure
     }
+    
+    @Override
+    @AfterAll
+    public void tearDown() throws Exception {
+        super.tearDown();
+        FileUtils.deleteDirectory(unpackTempDir.toFile());

Review Comment:
   `unpackTempDir` is deleted unconditionally here. If `@BeforeAll` fails 
before `unpackTempDir` is assigned (e.g., config load error), this `@AfterAll` 
can throw a `NullPointerException` and mask the original failure. Consider 
guarding with a null/existence check before deleting the directory (similar to 
the pattern used in `TikaPipesTest`).
   ```suggestion
           if (unpackTempDir != null && Files.exists(unpackTempDir)) {
               FileUtils.deleteDirectory(unpackTempDir.toFile());
           }
   ```



##########
tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerResourceWithConfigTest.java:
##########
@@ -260,4 +262,10 @@ public void testPDFPerPageRenderGray() throws Exception {
             assertTrue(averageColor.getBlue() < 100 && averageColor.getBlue() 
> 90);
         }
     }
+    
+    @AfterAll
+    public void tearDown() throws IOException {

Review Comment:
   This method declares `throws IOException`, but it calls `super.tearDown()` 
which is declared `throws Exception` in `CXFTestBase`. That won’t compile 
unless you either broaden this method’s throws clause to `Exception` (or 
`Throwable`-compatible) or catch/handle the checked `Exception` from 
`super.tearDown()`.
   ```suggestion
       public void tearDown() throws Exception {
   ```



##########
tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerResourceWithConfigTest.java:
##########
@@ -260,4 +262,10 @@ public void testPDFPerPageRenderGray() throws Exception {
             assertTrue(averageColor.getBlue() < 100 && averageColor.getBlue() 
> 90);
         }
     }
+    
+    @AfterAll
+    public void tearDown() throws IOException {
+        super.tearDown();
+        FileUtils.deleteDirectory(unpackTempDir.toFile());

Review Comment:
   Same as in `UnpackerResourceTest`: deleting `unpackTempDir` unconditionally 
in `@AfterAll` can throw (NPE / missing dir) if setup fails early, which can 
hide the real test failure. Add a null/existence guard (and possibly a 
best-effort delete) around this cleanup.
   ```suggestion
           if (unpackTempDir != null && Files.exists(unpackTempDir)) {
               FileUtils.deleteQuietly(unpackTempDir.toFile());
           }
   ```





> 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)