[
https://issues.apache.org/jira/browse/TIKA-4704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071281#comment-18071281
]
ASF GitHub Bot commented on TIKA-4704:
--------------------------------------
Copilot commented on code in PR #2735:
URL: https://github.com/apache/tika/pull/2735#discussion_r3037111465
##########
tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerResourceTest.java:
##########
@@ -602,4 +603,9 @@ public void testShallowExtraction() throws Exception {
assertEquals(200, response.getStatus());
// Just verify it succeeds - actual depth limiting behavior depends on
document structure
}
+
+ @AfterAll
+ public void tearDown() throws IOException {
+ Files.delete(unpackTempDir);
Review Comment:
`unpackTempDir` is created via `Files.createTempDirectory(...)`, so it will
typically contain files after the unpack tests run.
`Files.delete(unpackTempDir)` will fail with `DirectoryNotEmptyException` in
that case. Use a recursive directory delete (e.g.,
`FileUtils.deleteDirectory(unpackTempDir.toFile())` or a
`Files.walk(...).sorted(reverseOrder())` delete) and prefer
`deleteIfExists`/null guarding to avoid masking test failures with cleanup
exceptions.
```suggestion
if (unpackTempDir != null) {
try (java.util.stream.Stream<Path> paths =
Files.walk(unpackTempDir)) {
paths.sorted(java.util.Comparator.reverseOrder())
.forEach(path -> {
try {
Files.deleteIfExists(path);
} catch (IOException e) {
throw new RuntimeException(e);
}
});
} catch (RuntimeException e) {
if (e.getCause() instanceof IOException) {
throw (IOException) e.getCause();
}
throw e;
}
}
```
##########
tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerResourceTest.java:
##########
@@ -602,4 +603,9 @@ public void testShallowExtraction() throws Exception {
assertEquals(200, response.getStatus());
// Just verify it succeeds - actual depth limiting behavior depends on
document structure
}
+
+ @AfterAll
+ public void tearDown() throws IOException {
Review Comment:
This `tearDown()` method overrides `CXFTestBase#tearDown()` (which is also
annotated with `@AfterAll`) but doesn’t call `super.tearDown()`. That prevents
the base cleanup from running (server stop/destroy, PipesParser close,
config/temp cleanup), which can leave resources open and make subsequent tests
flaky. Consider either renaming this method (e.g., `cleanUpUnpackTempDir`) so
it doesn’t override, or add `@Override`, widen the signature to `throws
Exception`, and call `super.tearDown()` before deleting the temp directory.
```suggestion
@Override
@AfterAll
public void tearDown() throws Exception {
super.tearDown();
```
> 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)
