[
https://issues.apache.org/jira/browse/TIKA-4704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071434#comment-18071434
]
ASF GitHub Bot commented on TIKA-4704:
--------------------------------------
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
```
> 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)
