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);
}
```
--
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]
