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)) {
```
--
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]
