Skip to content

Commit

Permalink
move assertions to before trace assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
@jaydeluca
jaydeluca committed Jul 24, 2024
1 parent ad3bae2 commit 6c4f1f8
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 30 deletions.
28 changes: 11 additions & 17 deletions ...est/java/io/opentelemetry/javaagent/instrumentation/jsp/JspInstrumentationBasicTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ protected void cleanUp() {
void testNonErroneousGet(
String testName, String jspFileName, String jspClassName, String jspClassNamePrefix) {
AggregatedHttpResponse res = client.get(jspFileName).aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -134,8 +135,6 @@ void testNonErroneousGet(
.withParent(trace.getSpan(0))
.withRoute(jspFileName)
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

static class NonErroneousArgs implements ArgumentsProvider {
Expand All @@ -151,9 +150,11 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
@Test
void testNonErroneousGetWithQueryString() {
String queryString = "HELLO";
AggregatedHttpResponse res = client.get("/getQuery.jsp?" + queryString).aggregate().join();
String route = "/" + getContextPath() + "/getQuery.jsp";

AggregatedHttpResponse res = client.get("/getQuery.jsp?" + queryString).aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
Expand Down Expand Up @@ -194,8 +195,6 @@ void testNonErroneousGetWithQueryString() {
.withParent(trace.getSpan(0))
.withRoute("/getQuery.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@Test
Expand All @@ -206,6 +205,7 @@ void testNonErroneousPost() {
.build();

AggregatedHttpResponse res = client.execute(headers, "name=world").aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -233,8 +233,6 @@ void testNonErroneousPost() {
.withParent(trace.getSpan(0))
.withRoute("/post.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@ParameterizedTest(name = "GET jsp with {0}")
Expand All @@ -246,6 +244,7 @@ void testErroneousRuntimeErrorsGet(
Class<?> exceptionClass,
boolean errorMessageOptional) {
AggregatedHttpResponse res = client.get(jspFileName).aggregate().join();
assertThat(res.status().code()).isEqualTo(500);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -276,8 +275,6 @@ void testErroneousRuntimeErrorsGet(
.withRoute(jspFileName)
.withErrorMessageOptional(errorMessageOptional)
.build())));

assertThat(res.status().code()).isEqualTo(500);
}

static class ErroneousRuntimeErrorsArgs implements ArgumentsProvider {
Expand All @@ -304,6 +301,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
@Test
void testNonErroneousIncludePlainHtmlGet() {
AggregatedHttpResponse res = client.get("/includes/includeHtml.jsp").aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -331,13 +329,12 @@ void testNonErroneousIncludePlainHtmlGet() {
.withParent(trace.getSpan(0))
.withRoute("/includes/includeHtml.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@Test
void testNonErroneousMultiGet() {
AggregatedHttpResponse res = client.get("/includes/includeMulti.jsp").aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -397,15 +394,14 @@ void testNonErroneousMultiGet() {
.withRoute("/common/javaLoopH2.jsp")
.withRequestUrlOverride("/includes/includeMulti.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@ParameterizedTest
@ArgumentsSource(CompileErrorsArgs.class)
void testCompileErrorShouldNotProduceRenderTracesAndSpans(
String jspFileName, String jspClassName, String jspClassNamePrefix) {
AggregatedHttpResponse res = client.get(jspFileName).aggregate().join();
assertThat(res.status().code()).isEqualTo(500);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -444,8 +440,6 @@ void testCompileErrorShouldNotProduceRenderTracesAndSpans(
equalTo(
stringKey("jsp.compiler"),
"org.apache.jasper.compiler.JDTCompiler"))));

assertThat(res.status().code()).isEqualTo(500);
}

static class CompileErrorsArgs implements ArgumentsProvider {
Expand All @@ -462,7 +456,9 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
@ValueSource(strings = {"/common/hello.html"})
void testDirectStaticFileReference(String staticFile) {
String route = "/" + getContextPath() + "/*";

AggregatedHttpResponse res = client.get(staticFile).aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand All @@ -488,7 +484,5 @@ void testDirectStaticFileReference(String staticFile) {
satisfies(
NetworkAttributes.NETWORK_PEER_PORT,
val -> val.isInstanceOf(Long.class)))));

assertThat(res.status().code()).isEqualTo(200);
}
}
21 changes: 8 additions & 13 deletions ...t/java/io/opentelemetry/javaagent/instrumentation/jsp/JspInstrumentationForwardTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ void testNonErroneousGetForwardTo(
String jspForwardDestClassName,
String jspForwardDestClassPrefix) {
AggregatedHttpResponse res = client.get(forwardFromFileName).aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -149,8 +150,6 @@ void testNonErroneousGetForwardTo(
.withParent(trace.getSpan(2))
.withRoute(forwardDestFileName)
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

static class NonErroneousGetForwardArgs implements ArgumentsProvider {
Expand Down Expand Up @@ -179,6 +178,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
@Test
void testNonErroneousGetForwardToPlainHtml() {
AggregatedHttpResponse res = client.get("/forwards/forwardToHtml.jsp").aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -206,14 +206,13 @@ void testNonErroneousGetForwardToPlainHtml() {
.withParent(trace.getSpan(0))
.withRoute("/forwards/forwardToHtml.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@Test
void testNonErroneousGetForwardedToJspWithMultipleIncludes() {
AggregatedHttpResponse res =
client.get("/forwards/forwardToIncludeMulti.jsp").aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -291,13 +290,12 @@ void testNonErroneousGetForwardedToJspWithMultipleIncludes() {
.withRequestUrlOverride("/includes/includeMulti.jsp")
.withForwardOrigin("/forwards/forwardToIncludeMulti.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@Test
void testNonErroneousGetForwardToAnotherForward() {
AggregatedHttpResponse res = client.get("/forwards/forwardToJspForward.jsp").aggregate().join();
assertThat(res.status().code()).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -355,14 +353,13 @@ void testNonErroneousGetForwardToAnotherForward() {
.withParent(trace.getSpan(4))
.withRoute("/common/loop.jsp")
.build())));

assertThat(res.status().code()).isEqualTo(200);
}

@Test
void testForwardToJspWithCompileErrorShouldNotProduceSecondRenderSpan() {
AggregatedHttpResponse res =
client.get("/forwards/forwardToCompileError.jsp").aggregate().join();
assertThat(res.status().code()).isEqualTo(500);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -402,15 +399,15 @@ void testForwardToJspWithCompileErrorShouldNotProduceSecondRenderSpan() {
.withClassName("compileError_jsp")
.withExceptionClass(JasperException.class)
.build())));

assertThat(res.status().code()).isEqualTo(500);
}

@Test
void testForwardToNonExistentJspShouldBe404() {
String route = "/" + getContextPath() + "/forwards/forwardToNonExistent.jsp";

AggregatedHttpResponse res =
client.get("/forwards/forwardToNonExistent.jsp").aggregate().join();
String route = "/" + getContextPath() + "/forwards/forwardToNonExistent.jsp";
assertThat(res.status().code()).isEqualTo(404);

testing.waitAndAssertTraces(
trace ->
Expand Down Expand Up @@ -453,7 +450,5 @@ void testForwardToNonExistentJspShouldBe404() {
.withRoute("/forwards/forwardToNonExistent.jsp")
.build()),
span -> span.hasName("ResponseFacade.sendError").hasParent(trace.getSpan(2))));

assertThat(res.status().code()).isEqualTo(404);
}
}

0 comments on commit 6c4f1f8

Please sign in to comment.