Write http server tests in java #5501
Conversation
...rc/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
Show resolved
Hide resolved
| }); | ||
| } | ||
|
|
||
| testing.waitAndAssertTraces(assertions); |
There was a problem hiding this comment.
This behaves a bit strangely. When an assertion fails it will take 10s until awaitility times out.
instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy
Outdated
Show resolved
Hide resolved
instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ServerTest.groovy
Outdated
Show resolved
Hide resolved
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy
Outdated
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
Show resolved
Hide resolved
...c/main/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy
Show resolved
Hide resolved
instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy
Outdated
Show resolved
Hide resolved
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
...rc/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
Outdated
Show resolved
Hide resolved
| return span; | ||
| } | ||
|
|
||
| private static void assertException( |
There was a problem hiding this comment.
Can you use SpanDataAssert.hasException?
There was a problem hiding this comment.
I didn't use SpanDataAssert.hasException because it requires an instance of Throwable but here we only have exception class and error message. As actually we use only a few exception classes and all of them have a suitable constructor I could try constructing the exception instance reflectively.
There was a problem hiding this comment.
we use only a few exception classes and all of them have a suitable constructor I could try constructing the exception instance reflectively
I think this approach makes the test harder to read, @anuraaga what do you think about something like SpanDataAssert.hasExceptionSatisying for the most flexibility here?
@laurit I'm ok merging with any approach here and we can revisit
There was a problem hiding this comment.
I think that could be useful, but in this case, can't the places passing an exception class and String error message pass a Throwable instead by calling new Class(message)?
There was a problem hiding this comment.
I think it should be done in a separate pr as this pr is already fairly large.
...rc/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
Outdated
Show resolved
Hide resolved
| String contextPath = ""; | ||
| Class<? extends Throwable> expectedExceptionClass = Exception.class; | ||
|
|
||
| Function<ServerEndpoint, Boolean> hasHandlerSpan = unused -> false; |
There was a problem hiding this comment.
Why not use Predicate for this?
* Write http server tests in java * typo * Apply suggestions from code review Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com> * add comments * address review comments * use Predicate Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Add an option to write http server tests in java as junit tests and convert grails3 test to java. Being able to write tests in java will hopefully allow us to upgrade to groovy 4 (currently grails tests don't run with groovy 4) which hopefully will let us run some tests on jdk17.