convert module jaxrs-client-2.0-testing test case from groovy to java #12903
Conversation
shalk
changed the title
.../java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java
Outdated
Show resolved
Hide resolved
| abstract ClientBuilder builder() | ||
|
|
||
| @Unroll | ||
| def "should properly convert HTTP status #statusCode to span error status"() { |
There was a problem hiding this comment.
Have you converted the method?
There was a problem hiding this comment.
I have removed this case, the client error case have been covered by AbstractHttpClientTest
There was a problem hiding this comment.
I fail to see which test in AbstractHttpClientTest would test client-error endpoint that this test uses.
There was a problem hiding this comment.
I fail to see which test in
AbstractHttpClientTestwould testclient-errorendpoint that this test uses.
I found HttpClientTestServer have the path /client-error
How about add a new case /client-error into AbstractHttpClientTest ?
There was a problem hiding this comment.
Imo adding it to the AbstractHttpClientTest and disabling it everywhere does not make sense. If it is added to AbstractHttpClientTest then attempt should be made to enable it on as many clients as possible. Probably best to do this in a separate PR.
There was a problem hiding this comment.
add a separate PR #12924
This reverts commit 044bc96.
.../java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java
Outdated
Show resolved
Hide resolved
...ng/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Jay DeLuca <[email protected]>
#7195