Skip to content

Commit

Permalink
Convert HttpClientTest to JUnit (#3652)
Browse files Browse the repository at this point in the history
* Migrate HttpClientTest to junit to allow both Java or spock tests.

* More

* Update

* Finish

* Cleanup

* Better stack

* Java 15

* Merge

* Fix name

* Cleanup

* ? extends

* Moar
  • Loading branch information
Anuraag Agrawal authored Jul 27, 2021
1 parent 262feb1 commit 47be4a1
Show file tree
Hide file tree
Showing 48 changed files with 1,962 additions and 1,110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import akka.http.javadsl.model.headers.RawHeader
import akka.stream.ActorMaterializer
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.test.base.SingleConnection
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest
import io.opentelemetry.instrumentation.testing.junit.http.SingleConnection
import java.util.concurrent.TimeUnit
import spock.lang.Shared

Expand Down Expand Up @@ -46,7 +47,7 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest<HttpRequest> impl
}

@Override
void sendRequestWithCallback(HttpRequest request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
void sendRequestWithCallback(HttpRequest request, String method, URI uri, Map<String, String> headers, AbstractHttpClientTest.RequestResult requestResult) {
Http.get(system).singleRequest(request, materializer).whenComplete {response, throwable ->
if (throwable == null) {
response.discardEntityBytes(materializer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import java.util.concurrent.CancellationException
import org.apache.http.HttpResponse
Expand Down Expand Up @@ -46,7 +47,7 @@ class ApacheHttpAsyncClientTest extends HttpClientTest<HttpUriRequest> implement
}

@Override
void sendRequestWithCallback(HttpUriRequest request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
void sendRequestWithCallback(HttpUriRequest request, String method, URI uri, Map<String, String> headers, AbstractHttpClientTest.RequestResult requestResult) {
client.execute(request, new FutureCallback<HttpResponse>() {
@Override
void completed(HttpResponse httpResponse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import java.util.function.Consumer
import org.apache.http.HttpHost
Expand Down Expand Up @@ -59,7 +60,7 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
}

// compilation fails with @Override annotation on this method (groovy quirk?)
void sendRequestWithCallback(T request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
void sendRequestWithCallback(T request, String method, URI uri, Map<String, String> headers, AbstractHttpClientTest.RequestResult requestResult) {
try {
executeRequestWithCallback(request, uri) {
it.entity?.content?.close() // Make sure the connection is closed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import java.util.concurrent.TimeUnit
import java.util.function.Consumer
Expand Down Expand Up @@ -64,7 +65,7 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
}

// compilation fails with @Override annotation on this method (groovy quirk?)
void sendRequestWithCallback(T request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
void sendRequestWithCallback(T request, String method, URI uri, Map<String, String> headers, AbstractHttpClientTest.RequestResult requestResult) {
try {
executeRequestWithCallback(request, uri) {
it.close() // Make sure the connection is closed.
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;

import com.linecorp.armeria.client.WebClientBuilder;
import io.opentelemetry.instrumentation.armeria.v1_3.AbstractArmeriaHttpClientTest;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
import org.junit.jupiter.api.extension.RegisterExtension;

class ArmeriaHttpClientTest extends AbstractArmeriaHttpClientTest {

@RegisterExtension
static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();

@Override
protected WebClientBuilder configureClient(WebClientBuilder clientBuilder) {
return clientBuilder;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.armeria.v1_3;

import com.linecorp.armeria.client.WebClientBuilder;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
import org.junit.jupiter.api.extension.RegisterExtension;

class ArmeriaHttpClientTest extends AbstractArmeriaHttpClientTest {

@RegisterExtension
static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forLibrary();

@Override
protected WebClientBuilder configureClient(WebClientBuilder clientBuilder) {
return clientBuilder.decorator(
ArmeriaTracing.create(testing.getOpenTelemetry()).newClientDecorator());
}

// library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet
@Override
protected boolean testWithClientParent() {
return false;
}

// Agent users have automatic propagation through executor instrumentation, but library users
// should do manually using Armeria patterns.
@Override
protected boolean testCallbackWithParent() {
return false;
}

@Override
protected boolean testErrorWithCallback() {
return false;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.armeria.v1_3;

import com.linecorp.armeria.client.ClientFactory;
import com.linecorp.armeria.client.WebClient;
import com.linecorp.armeria.client.WebClientBuilder;
import com.linecorp.armeria.common.HttpMethod;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.RequestHeaders;
import com.linecorp.armeria.common.util.Exceptions;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.URI;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CompletionException;
import org.junit.jupiter.api.BeforeEach;

public abstract class AbstractArmeriaHttpClientTest extends AbstractHttpClientTest<HttpRequest> {

protected abstract WebClientBuilder configureClient(WebClientBuilder clientBuilder);

private WebClient client;

@BeforeEach
void setupClient() {
client =
configureClient(
WebClient.builder()
.factory(ClientFactory.builder().connectTimeout(connectTimeout()).build()))
.build();
}

@Override
protected final HttpRequest buildRequest(String method, URI uri, Map<String, String> headers) {
return HttpRequest.of(
RequestHeaders.builder(HttpMethod.valueOf(method), uri.toString())
.set(headers.entrySet())
.build());
}

@Override
protected final int sendRequest(
HttpRequest request, String method, URI uri, Map<String, String> headers) {
try {
return client.execute(request).aggregate().join().status().code();
} catch (CompletionException e) {
return Exceptions.throwUnsafely(e.getCause());
}
}

@Override
protected final void sendRequestWithCallback(
HttpRequest request,
String method,
URI uri,
Map<String, String> headers,
RequestResult requestResult) {
client
.execute(request)
.aggregate()
.whenComplete(
(response, throwable) ->
requestResult.complete(() -> response.status().code(), throwable));
}

// Not supported yet: https://github.com/line/armeria/issues/2489
@Override
protected final boolean testRedirects() {
return false;
}

@Override
protected final boolean testReusedRequest() {
// armeria requests can't be reused
return false;
}

@Override
protected Set<AttributeKey<?>> httpAttributes(URI uri) {
Set<AttributeKey<?>> extra = new HashSet<>();
extra.add(SemanticAttributes.HTTP_HOST);
extra.add(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH);
extra.add(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH);
extra.add(SemanticAttributes.HTTP_SCHEME);
extra.add(SemanticAttributes.HTTP_TARGET);
extra.addAll(super.httpAttributes(uri));
return extra;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import com.ning.http.client.Response
import com.ning.http.client.uri.Uri
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.test.base.SingleConnection
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest
import io.opentelemetry.instrumentation.testing.junit.http.SingleConnection
import spock.lang.AutoCleanup
import spock.lang.Shared

Expand All @@ -39,7 +40,7 @@ class AsyncHttpClientTest extends HttpClientTest<Request> implements AgentTestTr
}

@Override
void sendRequestWithCallback(Request request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
void sendRequestWithCallback(Request request, String method, URI uri, Map<String, String> headers, AbstractHttpClientTest.RequestResult requestResult) {
// TODO(anuraaga): Do we also need to test ListenableFuture callback?
client.executeRequest(request, new AsyncCompletionHandler<Void>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpClientTest
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import org.asynchttpclient.AsyncCompletionHandler
import org.asynchttpclient.Dsl
Expand Down Expand Up @@ -40,7 +41,7 @@ class AsyncHttpClientTest extends HttpClientTest<Request> implements AgentTestTr
}

@Override
void sendRequestWithCallback(Request request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
void sendRequestWithCallback(Request request, String method, URI uri, Map<String, String> headers, AbstractHttpClientTest.RequestResult requestResult) {
client.executeRequest(request, new AsyncCompletionHandler<Void>() {
@Override
Void onCompleted(Response response) throws Exception {
Expand Down
Loading

0 comments on commit 47be4a1

Please sign in to comment.