Skip to content

convert dropwizard-views-0.7 tests from groovy to java#8808

Merged
laurit merged 8 commits into
open-telemetry:mainfrom
quinoant:ViewRenderTest-toJava
Jun 28, 2023
Merged

convert dropwizard-views-0.7 tests from groovy to java#8808
laurit merged 8 commits into
open-telemetry:mainfrom
quinoant:ViewRenderTest-toJava

Conversation

@quinoant

Copy link
Copy Markdown
Contributor

related to #7195

@quinoant quinoant requested a review from a team June 27, 2023 02:05
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 27, 2023

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

Comment on lines +6 to +9
package io.opentelemetry.javaagent.instrumentation.dropwizardviews; /*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package io.opentelemetry.javaagent.instrumentation.dropwizardviews; /*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.dropwizardviews;


@AfterEach
void cleanUp() {
testing.clearData();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows each of the iterations of the test to start with a clean slate rather than a trace with other spans that had been created by previous test interfering. Taking that part out causes every test after the first one to fail.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is so because you have not added @RegisterExtension annotation on InstrumentationExtension field?

() -> {
renderer.render(view, Locale.ENGLISH, outputStream);
});
assertTrue(outputStream.toString("UTF-8").contains("This is an example of a view"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use assertj assertThat(outputStream.toString("UTF-8")).contains("This is an example of a view")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great recommendation. I will apply that suggestion.

@quinoant

Copy link
Copy Markdown
Contributor Author

Thanks for the tips and the quick review!

@mateuszrzeszutek mateuszrzeszutek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thanks @quinoant !

@laurit laurit merged commit 47159e6 into open-telemetry:main Jun 28, 2023
@quinoant quinoant deleted the ViewRenderTest-toJava branch June 28, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants