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();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
