-
Notifications
You must be signed in to change notification settings - Fork 878
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 ratpack 1.7 tests from groovy to java #12980
Conversation
...library/src/test/java/io/opentelemetry/instrumentation/ratpack/v1_7/AbstractRatpackTest.java
Outdated
Show resolved
Hide resolved
...7/library/src/test/java/io/opentelemetry/instrumentation/ratpack/v1_7/client/BarService.java
Outdated
Show resolved
Hide resolved
@Test | ||
void testIgnoreHandlersBeforeOpenTelemetryServerHandler() { | ||
assertThat(app.getHttpClient().get("ignore").getBody().getText()).isEqualTo("ignored"); | ||
assertThat(testing.spans().stream().filter(span -> "GET /ignore".equals(span.getName()))) |
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.
In the groovy test we retry until this condition becomes true, here we assume that we already have the completed spans. Mayabe here we should also use awaitility to retry to ensure this test won't be flaky or rewrite to use testing.waitAndAssertTraces
that also retries.
|
||
@Test | ||
void testIgnoreHandlersBeforeOpenTelemetryServerHandler() { | ||
assertThat(app.getHttpClient().get("ignore").getBody().getText()).isEqualTo("ignored"); |
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.
original groovy test uses app.test()
. I think it should be fine to not use it. The difference between groovy and java version is that in groovy version app = new RatpackFunctionalTest(RatpackApp)
is run for each test and the app.test
calls close
on the app when the test completes. In the java version app is created once for all tests and close
is not called. We could close it in a @AfterAll
cleanup method.
span.hasName("another-span") | ||
.hasParent(trace.getSpan(0)) | ||
.hasAttributes(Attributes.empty()) | ||
.hasTotalRecordedEvents(1))); |
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 hasEventsSatisfyingExactly
Related to #7195