[
https://issues.apache.org/jira/browse/TIKA-4704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071441#comment-18071441
]
ASF GitHub Bot commented on TIKA-4704:
--------------------------------------
Copilot commented on code in PR #2743:
URL: https://github.com/apache/tika/pull/2743#discussion_r3039753868
##########
tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/PassbackFilterTest.java:
##########
@@ -81,6 +81,7 @@ public void testPassbackFilter(@TempDir Path tmpDir) throws
Exception {
.emitData()
.getMetadataList()
.get(0);
+ pipesClient.close();
Review Comment:
`pipesClient.close()` is called manually after `process(...)`, so if
`process(...)` throws (or any earlier line throws), the client (and its
per-client temp resources) won't be closed. Prefer scoping the client to a
try-with-resources in the test (and ideally avoid the mutable instance field)
so cleanup is guaranteed even on failures.
##########
tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/EmbeddedLimitsTest.java:
##########
@@ -165,23 +164,24 @@ public void testMaxDepthWithException(@TempDir Path tmp)
throws Exception {
limits.setThrowOnMaxDepth(true);
parseContext.set(EmbeddedLimits.class, limits);
- PipesClient pipesClient = init(tmp, TEST_DOC_WITH_EMBEDDED);
-
- PipesResult pipesResult = pipesClient.process(
- new FetchEmitTuple(TEST_DOC_WITH_EMBEDDED,
- new FetchKey(FETCHER_NAME, TEST_DOC_WITH_EMBEDDED),
- new EmitKey(), new Metadata(), parseContext,
- FetchEmitTuple.ON_PARSE_EXCEPTION.EMIT));
-
- // When throwOnMaxDepth=true and limit is exceeded, an exception is
thrown
- // but caught and recorded. Result is still "success" but with
exception.
- // The key behavior: parsing stops early, container metadata is
returned
- assertTrue(pipesResult.isSuccess(), "Parse should complete (with
exception recorded)");
- assertEquals(1, pipesResult.emitData().getMetadataList().size(),
- "Should have only container when maxDepth=0 with exception");
- // The status should indicate an exception was encountered
- assertEquals(PipesResult.RESULT_STATUS.PARSE_SUCCESS_WITH_EXCEPTION,
pipesResult.status(),
- "Should have parse exception status when throwOnMaxDepth=true
and limit exceeded");
+ try (PipesClient pipesClient = init(tmp, TEST_DOC_WITH_EMBEDDED))
+ {
Review Comment:
The `try` block uses a non-standard brace placement (`try (...)` followed by
`{` on the next line). This is inconsistent with the surrounding tests in this
module, which use K&R style (`try (...) {`) and can trigger formatting/lint
churn. Please put the opening brace on the same line as the `try` statement.
```suggestion
try (PipesClient pipesClient = init(tmp, TEST_DOC_WITH_EMBEDDED)) {
```
> 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)
