Skip to content

Don't use HttpServerTest.controller in java tests #12347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 30, 2024
Merged

Don't use HttpServerTest.controller in java tests #12347

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
978ae8e
Don't use HttpServerTest.controller in java tests
laurit Sep 27, 2024
67adfc1
finatra tests
laurit Sep 27, 2024
9a8dc72
replace more usages of HttpServerTest.controller
laurit Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...cala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestAsyncWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import io.opentelemetry.instrumentation.testing.junit.http.{
ServerEndpoint
}
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._
import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier

import java.util.function.Supplier
import scala.concurrent.{Await, ExecutionContextExecutor, Future}

object AkkaHttpTestAsyncWebServer {
Expand All @@ -31,7 +31,7 @@ object AkkaHttpTestAsyncWebServer {
val endpoint = ServerEndpoint.forPath(uri.path.toString())
AbstractHttpServerTest.controller(
endpoint,
new Supplier[HttpResponse] {
new ThrowingSupplier[HttpResponse, Exception] {
def get(): HttpResponse = {
val resp = HttpResponse(status =
endpoint.getStatus
Expand Down
8 changes: 4 additions & 4 deletions .../opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestServerSourceWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import akka.stream.ActorMaterializer
import akka.stream.scaladsl.Sink
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._
import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier

import java.util.function.Supplier
import scala.concurrent.Await

object AkkaHttpTestServerSourceWebServer {
Expand All @@ -33,7 +33,7 @@ object AkkaHttpTestServerSourceWebServer {
},
path(INDEXED_CHILD.rawPath()) {
parameterMap { map =>
val supplier = new Supplier[String] {
val supplier = new ThrowingSupplier[String, Exception] {
def get(): String = {
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
Expand Down Expand Up @@ -114,8 +114,8 @@ object AkkaHttpTestServerSourceWebServer {
}
}

def supplier(string: String): Supplier[String] = {
new Supplier[String] {
def supplier(string: String): ThrowingSupplier[String, Exception] = {
new ThrowingSupplier[String, Exception] {
def get(): String = {
string
}
Expand Down
4 changes: 2 additions & 2 deletions ...scala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestSyncWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import io.opentelemetry.instrumentation.testing.junit.http.{
ServerEndpoint
}
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._
import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier

import java.util.function.Supplier
import scala.concurrent.Await

object AkkaHttpTestSyncWebServer {
Expand All @@ -30,7 +30,7 @@ object AkkaHttpTestSyncWebServer {
val endpoint = ServerEndpoint.forPath(uri.path.toString())
AbstractHttpServerTest.controller(
endpoint,
new Supplier[HttpResponse] {
new ThrowingSupplier[HttpResponse, Exception] {
def get(): HttpResponse = {
val resp = HttpResponse(status = endpoint.getStatus)
endpoint match {
Expand Down
8 changes: 4 additions & 4 deletions ...est/scala/io/opentelemetry/javaagent/instrumentation/akkahttp/AkkaHttpTestWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import akka.http.scaladsl.server.Directives._
import akka.stream.ActorMaterializer
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._
import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier

import java.util.function.Supplier
import scala.concurrent.Await

object AkkaHttpTestWebServer {
Expand All @@ -32,7 +32,7 @@ object AkkaHttpTestWebServer {
},
path(INDEXED_CHILD.rawPath()) {
parameterMap { map =>
val supplier = new Supplier[String] {
val supplier = new ThrowingSupplier[String, Exception] {
def get(): String = {
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
Expand Down Expand Up @@ -107,8 +107,8 @@ object AkkaHttpTestWebServer {
}
}

def supplier(string: String): Supplier[String] = {
new Supplier[String] {
def supplier(string: String): ThrowingSupplier[String, Exception] = {
new ThrowingSupplier[String, Exception] {
def get(): String = {
string
}
Expand Down
8 changes: 7 additions & 1 deletion instrumentation/grails-3.0/javaagent/src/test/groovy/test/TestController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ package test

import grails.artefact.Controller
import grails.web.Action
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint
import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable

import static io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest.controller
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.CAPTURE_HEADERS
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION
Expand Down Expand Up @@ -82,4 +84,8 @@ class TestController implements Controller {
render INDEXED_CHILD.body
}
}

private static void controller(ServerEndpoint endpoint, ThrowingRunnable action) {
AbstractHttpServerTest.controller(endpoint, action)
}
}
42 changes: 20 additions & 22 deletions .../opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestAsyncWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,26 @@ object PekkoHttpTestAsyncWebServer {
val endpoint = ServerEndpoint.forPath(uri.path.toString())
AbstractHttpServerTest.controller(
endpoint,
new Supplier[HttpResponse] {
def get(): HttpResponse = {
val resp = HttpResponse(status =
endpoint.getStatus
) // .withHeaders(headers.Type)resp.contentType = "text/plain"
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case INDEXED_CHILD =>
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
uri.query().get(name).orNull
})
resp.withEntity("")
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT =>
resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ =>
HttpResponse(status = NOT_FOUND.getStatus)
.withEntity(NOT_FOUND.getBody)
}
() => {
val resp = HttpResponse(status =
endpoint.getStatus
) // .withHeaders(headers.Type)resp.contentType = "text/plain"
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case INDEXED_CHILD =>
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
uri.query().get(name).orNull
})
resp.withEntity("")
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT =>
resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ =>
HttpResponse(status = NOT_FOUND.getStatus)
.withEntity(NOT_FOUND.getBody)
}
}
)
Expand Down
12 changes: 5 additions & 7 deletions ...lemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestServerSourceWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0

import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._
import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier
import org.apache.pekko.actor.ActorSystem
import org.apache.pekko.http.scaladsl.Http
import org.apache.pekko.http.scaladsl.Http.ServerBinding
Expand All @@ -15,7 +16,6 @@ import org.apache.pekko.http.scaladsl.server.Directives._
import org.apache.pekko.stream.ActorMaterializer
import org.apache.pekko.stream.scaladsl.Sink

import java.util.function.Supplier
import scala.concurrent.Await

object PekkoHttpTestServerSourceWebServer {
Expand All @@ -33,7 +33,7 @@ object PekkoHttpTestServerSourceWebServer {
},
path(INDEXED_CHILD.rawPath()) {
parameterMap { map =>
val supplier = new Supplier[String] {
val supplier = new ThrowingSupplier[String, Exception] {
def get(): String = {
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
Expand Down Expand Up @@ -114,11 +114,9 @@ object PekkoHttpTestServerSourceWebServer {
}
}

def supplier(string: String): Supplier[String] = {
new Supplier[String] {
def get(): String = {
string
}
def supplier(string: String): ThrowingSupplier[String, Exception] = { () =>
{
string
}
}
}
38 changes: 18 additions & 20 deletions ...o/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestSyncWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,24 @@ object PekkoHttpTestSyncWebServer {
val endpoint = ServerEndpoint.forPath(uri.path.toString())
AbstractHttpServerTest.controller(
endpoint,
new Supplier[HttpResponse] {
def get(): HttpResponse = {
val resp = HttpResponse(status = endpoint.getStatus)
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case INDEXED_CHILD =>
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
uri.query().get(name).orNull
})
resp.withEntity("")
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT =>
resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ =>
HttpResponse(status = NOT_FOUND.getStatus)
.withEntity(NOT_FOUND.getBody)
}
() => {
val resp = HttpResponse(status = endpoint.getStatus)
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case INDEXED_CHILD =>
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
uri.query().get(name).orNull
})
resp.withEntity("")
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT =>
resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ =>
HttpResponse(status = NOT_FOUND.getStatus)
.withEntity(NOT_FOUND.getBody)
}
}
)
Expand Down
12 changes: 5 additions & 7 deletions ...la/io/opentelemetry/javaagent/instrumentation/pekkohttp/v1_0/PekkoHttpTestWebServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ package io.opentelemetry.javaagent.instrumentation.pekkohttp.v1_0

import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint._
import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier
import org.apache.pekko.actor.ActorSystem
import org.apache.pekko.http.scaladsl.Http
import org.apache.pekko.http.scaladsl.Http.ServerBinding
import org.apache.pekko.http.scaladsl.model.StatusCodes.Found
import org.apache.pekko.http.scaladsl.server.Directives._
import org.apache.pekko.stream.ActorMaterializer

import java.util.function.Supplier
import scala.concurrent.Await

object PekkoHttpTestWebServer {
Expand All @@ -32,7 +32,7 @@ object PekkoHttpTestWebServer {
},
path(INDEXED_CHILD.rawPath()) {
parameterMap { map =>
val supplier = new Supplier[String] {
val supplier = new ThrowingSupplier[String, Exception] {
def get(): String = {
INDEXED_CHILD.collectSpanAttributes(new UrlParameterProvider {
override def getParameter(name: String): String =
Expand Down Expand Up @@ -107,11 +107,9 @@ object PekkoHttpTestWebServer {
}
}

def supplier(string: String): Supplier[String] = {
new Supplier[String] {
def get(): String = {
string
}
def supplier(string: String): ThrowingSupplier[String, Exception] = { () =>
{
string
}
}
}
1 change: 1 addition & 0 deletions .../test/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/JettyServlet2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ protected void configure(HttpServerTestOptions options) {
options.setTestCaptureHttpHeaders(false);
options.setHasResponseSpan(e -> e.equals(REDIRECT) || e.equals(ERROR));
options.setHasResponseCustomizer(e -> true);
options.setExpectedException(new IllegalStateException(EXCEPTION.getBody()));
}

@Override
Expand Down
9 changes: 5 additions & 4 deletions ...t/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/TestServlet2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.REDIRECT;
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS;

import io.opentelemetry.instrumentation.test.base.HttpServerTest;
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest;
import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
import java.io.IOException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -24,10 +25,10 @@ private TestServlet2() {}

public static class Sync extends HttpServlet {
@Override
protected void service(HttpServletRequest req, HttpServletResponse resp) {
protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
req.getRequestDispatcher(null);
ServerEndpoint endpoint = ServerEndpoint.forPath(req.getServletPath());
HttpServerTest.controller(
AbstractHttpServerTest.controller(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AbstractHttpServerTest.controller(
controller(

endpoint,
() -> {
resp.setContentType("text/plain");
Expand All @@ -42,7 +43,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) {
} else if (ERROR.equals(endpoint)) {
resp.sendError(endpoint.getStatus(), endpoint.getBody());
} else if (EXCEPTION.equals(endpoint)) {
throw new Exception(endpoint.getBody());
throw new IllegalStateException(endpoint.getBody());
} else if (INDEXED_CHILD.equals(endpoint)) {
INDEXED_CHILD.collectSpanAttributes(req::getParameter);
resp.setStatus(endpoint.getStatus());
Expand Down
1 change: 1 addition & 0 deletions ...st/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/AbstractServlet3Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ protected void configure(HttpServerTestOptions options) {
options.setTestCaptureRequestParameters(true);
options.setHasResponseCustomizer(e -> true);
options.setHasResponseSpan(this::hasResponseSpan);
options.setExpectedException(new IllegalStateException(EXCEPTION.getBody()));
}

protected boolean hasResponseSpan(ServerEndpoint endpoint) {
Expand Down
3 changes: 1 addition & 2 deletions ...java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/jetty/JettyServlet3Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.io.Writer;
import java.net.InetSocketAddress;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.ErrorHandler;
Expand All @@ -47,7 +46,7 @@ public static boolean isBefore94() {
protected void configure(HttpServerTestOptions options) {
super.configure(options);
options.setTestNotFound(false);
options.setExpectedException(new ServletException(EXCEPTION.getBody()));
options.setExpectedException(new IllegalStateException(EXCEPTION.getBody()));
options.setContextPath("/jetty-context");
options.setVerifyServerSpanEndTime(!isAsyncTest());
}
Expand Down
3 changes: 1 addition & 2 deletions ...o/opentelemetry/javaagent/instrumentation/servlet/v3_0/jetty/JettyServletHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.HashSet;
import java.util.Set;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.ErrorHandler;
Expand All @@ -42,7 +41,7 @@ protected void configure(HttpServerTestOptions options) {
super.configure(options);
options.setContextPath("");
options.setTestNotFound(false);
options.setExpectedException(new ServletException(EXCEPTION.getBody()));
options.setExpectedException(new IllegalStateException(EXCEPTION.getBody()));
options.setHttpAttributes(
serverEndpoint -> {
Set<AttributeKey<?>> attributes =
Expand Down
Loading
Loading