Copilot commented on code in PR #2743:
URL: https://github.com/apache/tika/pull/2743#discussion_r3039575501
##########
tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/EmbeddedLimitsTest.java:
##########
@@ -225,20 +225,20 @@ public void testMaxDepthAllowsFirstLevel(@TempDir Path
tmp) throws Exception {
limits.setMaxDepth(2);
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.SKIP));
-
- assertTrue(pipesResult.isSuccess(), "Parse should succeed");
- // With maxDepth=2, first-level embedded should be parsed
- // mock-embedded.xml has 4 embedded documents
- int metadataCount = pipesResult.emitData().getMetadataList().size();
- assertTrue(metadataCount >= 2,
- "Should have at least 2 metadata objects with maxDepth=2, got:
" + metadataCount);
+ try (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.SKIP));
+
+ assertTrue(pipesResult.isSuccess(), "Parse should succeed");
+ // With maxDepth=2, first-level embedded should be parsed
+ // mock-embedded.xml has 4 embedded documents
Review Comment:
The comment here says `mock-embedded.xml has 4 embedded documents`, but
earlier in this same file (near the `TEST_DOC_WITH_EMBEDDED` constant) the
comment says it has 2 embedded documents. These contradictory expectations make
the tests confusing; please correct/remove one of the comments so they reflect
the same embedded count.
```suggestion
```
##########
tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/MetadataWriteLimiterTest.java:
##########
@@ -88,31 +88,32 @@ public void testWriteLimiterFromConfig(@TempDir Path tmp)
throws Exception {
*/
@Test
public void testWriteLimiterOverrideViaParseContext(@TempDir Path tmp)
throws Exception {
- PipesClient pipesClient = initWithWriteLimiter(tmp, TEST_DOC);
-
+ Metadata metadata;
// Create a ParseContext with an override that allows
X-TIKA:parse_time_millis
// The default config's includeFields (dc:creator, Content-Type,
X-TIKA:content)
// does NOT include X-TIKA:parse_time_millis, but this override does.
Review Comment:
There are duplicated comment blocks describing the ParseContext override
(one before the try-with-resources and the same text again inside the try).
This is redundant and makes the test harder to read; keep the comment in one
place (preferably only once near the ParseContext creation).
```suggestion
```
##########
tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/MetadataWriteLimiterTest.java:
##########
@@ -59,16 +59,16 @@ private PipesClient initWithWriteLimiter(Path tmp, String
testFileName) throws E
*/
@Test
public void testWriteLimiterFromConfig(@TempDir Path tmp) throws Exception
{
- PipesClient pipesClient = initWithWriteLimiter(tmp, TEST_DOC);
-
- PipesResult pipesResult = pipesClient.process(
- new FetchEmitTuple(TEST_DOC, new FetchKey(FETCHER_NAME,
TEST_DOC),
- new EmitKey(), new Metadata(), new ParseContext(),
FetchEmitTuple.ON_PARSE_EXCEPTION.SKIP));
-
- assertNotNull(pipesResult.emitData().getMetadataList());
- assertEquals(1, pipesResult.emitData().getMetadataList().size());
-
- Metadata metadata = pipesResult.emitData().getMetadataList().get(0);
+ Metadata metadata;
+ try (PipesClient pipesClient = initWithWriteLimiter(tmp, TEST_DOC)) {
+ PipesResult pipesResult = pipesClient.process(
+ new FetchEmitTuple(TEST_DOC, new FetchKey(FETCHER_NAME,
TEST_DOC),
+ new EmitKey(), new Metadata(), new ParseContext(),
FetchEmitTuple.ON_PARSE_EXCEPTION.SKIP));
+ assertNotNull(pipesResult.emitData().getMetadataList());
+ assertEquals(1, pipesResult.emitData().getMetadataList().size());
+ metadata = pipesResult.emitData().getMetadataList().get(0);
+ }
+
Review Comment:
Blank line contains trailing whitespace after the try-with-resources block.
This can cause Spotless/formatting checks to fail; please remove trailing
spaces (or run Spotless).
```suggestion
```
--
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]
