-
Notifications
You must be signed in to change notification settings - Fork 873
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.
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
Convert Aws sdk 2.2 Client core tests from groovy to java #12949
base: main
Are you sure you want to change the base?
Conversation
...g/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientCoreTest.java
Outdated
Show resolved
Hide resolved
...g/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientCoreTest.java
Outdated
Show resolved
Hide resolved
...sting/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.java
Outdated
Show resolved
Hide resolved
...sting/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.java
Outdated
Show resolved
Hide resolved
...sting/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientTest.java
Outdated
Show resolved
Hide resolved
equalTo(maybeStable(DB_OPERATION), operation)); | ||
} | ||
|
||
private static Stream<Arguments> provideArguments() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use something like
@SuppressWarnings("unchecked")
private static DynamoDbClient wrapClient(DynamoDbAsyncClient asyncClient) {
return (DynamoDbClient)
Proxy.newProxyInstance(
DynamoDbClient.class.getClassLoader(),
new Class<?>[] {DynamoDbClient.class},
(proxy, method, args) -> {
Method asyncMethod =
DynamoDbAsyncClient.class.getMethod(method.getName(), method.getParameterTypes());
CompletableFuture<?> future =
(CompletableFuture<?>) asyncMethod.invoke(asyncClient, args);
return future.get();
});
}
so that the parameters would not need to be duplicated between sync and async case. Async test could use Object response = call.apply(wrapClient(client));
Note that the response passed to validateOperationResponse
won't be a Future
any more with this.
Could generalize this to
@SuppressWarnings("unchecked")
protected static <T, U> T wrapClient(Class<T> syncClientClass, Class<U> asyncClientClass, U asyncClient) {
return (T)
Proxy.newProxyInstance(
AbstractAws2ClientCoreTest.class.getClassLoader(),
new Class<?>[] {syncClientClass},
(proxy, method, args) -> {
Method asyncMethod =
asyncClientClass.getMethod(method.getName(), method.getParameterTypes());
CompletableFuture<?> future =
(CompletableFuture<?>) asyncMethod.invoke(asyncClient, args);
return future.get();
});
}
from what I noticed only s3 getObject
has different parameters for sync and async.
Idk if the slightly reduced code size is worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are a wizard. thank you for this suggestion
Related to #7195