Copilot commented on code in PR #2672: URL: https://github.com/apache/tika/pull/2672#discussion_r2889591071
########## tika-e2e-tests/tika-server/src/test/java/org/apache/tika/server/e2e/TikaServerHttp2Test.java: ########## @@ -0,0 +1,200 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tika.server.e2e; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.net.ServerSocket; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; +import java.time.Instant; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * End-to-end test verifying that tika-server-standard supports HTTP/2 (h2c cleartext). + * + * Starts the real fat-jar, sends a request using Java's HttpClient configured for HTTP/2, + * and asserts the response was served over HTTP/2. This validates the runtime classpath + * has the Jetty http2-server jar and CXF negotiates h2c correctly. + * + * Run with: mvn test -pl tika-e2e-tests/tika-server -Pe2e + * + * Inspired by Lawrence Moorehead's original contribution (elemdisc/tika PR#1, TIKA-4679). + */ +@Tag("E2ETest") +public class TikaServerHttp2Test { + + private static final Logger log = LoggerFactory.getLogger(TikaServerHttp2Test.class); + private static final long SERVER_STARTUP_TIMEOUT_MS = 90_000; + private static final String STATUS_PATH = "/status"; + + private Process serverProcess; + private int port; + private String endPoint; + + @BeforeEach + void startServer() throws Exception { + port = findFreePort(); + endPoint = "http://localhost:"; + port; + + String jarPath = System.getProperty("tika.server.jar"); + if (jarPath == null) { + // fall back to conventional location relative to this module + Path moduleDir = Paths.get("").toAbsolutePath(); + Path repoRoot = moduleDir; + while (repoRoot != null && !repoRoot.resolve("tika-server").toFile().isDirectory()) { + repoRoot = repoRoot.getParent(); + } + if (repoRoot == null) { + throw new IllegalStateException("Cannot locate tika root. Pass -Dtika.server.jar=/path/to/tika-server-standard.jar"); + } + jarPath = repoRoot.resolve("tika-server/tika-server-standard/target") + .toAbsolutePath() + .toString() + "/tika-server-standard-" + + System.getProperty("tika.version", "4.0.0-SNAPSHOT") + ".jar"; + } + + Path jar = Paths.get(jarPath); + Assumptions.assumeTrue(Files.exists(jar), + "Fat-jar not found at " + jarPath + "; skipping HTTP/2 e2e test. " + + "Build with: mvn package -pl tika-server/tika-server-standard -DskipTests"); + + log.info("Starting tika-server-standard from: {}", jarPath); + ProcessBuilder pb = new ProcessBuilder( + "java", "-jar", jarPath, + "-p", String.valueOf(port), + "-h", "localhost" + ); + pb.redirectErrorStream(true); + serverProcess = pb.start(); + + // Drain output in background so the process doesn't block + Thread drainThread = new Thread(() -> { + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(serverProcess.getInputStream(), UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + log.debug("tika-server: {}", line); + } + } catch (Exception e) { + log.debug("Server output stream closed", e); + } + }); + drainThread.setDaemon(true); + drainThread.start(); + + awaitServerStartup(); + } + Review Comment: The `stopServer()` method calls `serverProcess.waitFor()` without a timeout. If the server process doesn't terminate after `destroy()` (SIGTERM), this will block the test indefinitely. The existing `IntegrationTestBase.tearDown()` uses a safer pattern: `waitFor(5, TimeUnit.SECONDS)` followed by `destroyForcibly()` and then `waitFor(30, TimeUnit.SECONDS)`. Consider adopting the same approach here to prevent hanging CI builds. ########## tika-e2e-tests/tika-server/src/test/java/org/apache/tika/server/e2e/TikaServerHttp2Test.java: ########## @@ -0,0 +1,200 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tika.server.e2e; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.net.ServerSocket; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; +import java.time.Instant; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * End-to-end test verifying that tika-server-standard supports HTTP/2 (h2c cleartext). + * + * Starts the real fat-jar, sends a request using Java's HttpClient configured for HTTP/2, + * and asserts the response was served over HTTP/2. This validates the runtime classpath + * has the Jetty http2-server jar and CXF negotiates h2c correctly. + * + * Run with: mvn test -pl tika-e2e-tests/tika-server -Pe2e + * + * Inspired by Lawrence Moorehead's original contribution (elemdisc/tika PR#1, TIKA-4679). + */ +@Tag("E2ETest") +public class TikaServerHttp2Test { + + private static final Logger log = LoggerFactory.getLogger(TikaServerHttp2Test.class); + private static final long SERVER_STARTUP_TIMEOUT_MS = 90_000; + private static final String STATUS_PATH = "/status"; + + private Process serverProcess; + private int port; + private String endPoint; + + @BeforeEach + void startServer() throws Exception { + port = findFreePort(); + endPoint = "http://localhost:"; + port; + + String jarPath = System.getProperty("tika.server.jar"); + if (jarPath == null) { + // fall back to conventional location relative to this module + Path moduleDir = Paths.get("").toAbsolutePath(); + Path repoRoot = moduleDir; Review Comment: The variable `moduleDir` is assigned on line 71 but never used — `repoRoot` is immediately initialized to the same value on line 72. You can remove `moduleDir` and initialize `repoRoot` directly: `Path repoRoot = Paths.get("").toAbsolutePath();`. ```suggestion Path repoRoot = Paths.get("").toAbsolutePath(); ``` -- 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]
