Skip to content

convert groovy smoke tests to java, part 1 #14648

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Sep 16, 2025
Merged

convert groovy smoke tests to java, part 1 #14648

merged 16 commits into from
Sep 16, 2025

Conversation

@zeitlinger
Copy link
Member

No description provided.

@zeitlinger zeitlinger self-assigned this Sep 12, 2025
laurit
laurit reviewed Sep 15, 2025
View reviewed changes
smoke-tests/src/test/java/io/opentelemetry/smoketest/AgentDebugLoggingTest.java Outdated
Comment on lines 29 to 30
startTarget(8);
stopTarget();
Copy link
Contributor

Choose a reason for hiding this comment

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

try {
  startTarget(8);
} finally {
  stopTarget();
}

would be java equivalent of the original groovy code. I'd probably move the stopTarget to a cleanup method

Copy link
Member Author

Choose a reason for hiding this comment

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

that would work it this test but fail here, because cleanup is not done in between a parameterized test.

I added "withTarget" - which works with parameterized tests.

laurit
laurit reviewed Sep 15, 2025
View reviewed changes
smoke-tests/src/test/java/io/opentelemetry/smoketest/JavaSmokeTest.java
import org.junit.jupiter.api.BeforeEach;
import org.testcontainers.containers.output.OutputFrame;

public abstract class JavaSmokeTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

not really a fan of using the Java prefix, guess we can rename it back once all tests are converted

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, waiting for ideas here - cound also rename the groovy classes to have a "Legacy" prefix

laurit
laurit reviewed Sep 15, 2025
View reviewed changes
smoke-tests/src/test/java/io/opentelemetry/smoketest/JavaTelemetryRetriever.java
import java.util.function.Supplier;
import java.util.stream.Collectors;

public class JavaTelemetryRetriever {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this class is equivalent to the original groovy version? Did you consider deleting the groovy version and just keeping this one? Imo it is fine to keep both for now if it makes it easier for you.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just wanted to avoid having to learn how groovy java interop works...

@zeitlinger zeitlinger force-pushed the convert-groovy-smoke-tests-to-java branch from 5fd2a36 to 5e8f1d2 Compare September 15, 2025 08:05
@zeitlinger
Copy link
Member Author

@laurit I guess this has to do with the shade I added - but I don't understand it:

For GatewayRouteMappingTest:

Transformed classes match global libraries ignore matcher: [org.springframework.boot.autoconfigure.gson.GsonAutoConfiguration$StandardGsonBuilderCustomizer$$Lambda]
java.lang.AssertionError: Transformed classes match global libraries ignore matcher: [org.springframework.boot.autoconfigure.gson.GsonAutoConfiguration$StandardGsonBuilderCustomizer$$Lambda]
	at io.opentelemetry.instrumentation.testing.AgentTestRunner.afterTestClass(AgentTestRunner.java:85)
	at io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension.afterAll(InstrumentationExtension.java:65)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

@zeitlinger zeitlinger marked this pull request as ready for review September 15, 2025 16:13
@zeitlinger zeitlinger requested a review from a team as a code owner September 15, 2025 16:13
@zeitlinger
Copy link
Member Author

Thanks @laurit !

@zeitlinger zeitlinger changed the title convert groovy smoke tests to java convert groovy smoke tests to java, part 1 Sep 15, 2025
jaydeluca
jaydeluca reviewed Sep 15, 2025
View reviewed changes
smoke-tests/src/test/java/io/opentelemetry/smoketest/AgentDebugLoggingTest.java Outdated
protected String getTargetImage(String jdk) {
return "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-spring-boot:jdk"
+ jdk
+ "-20211213.1570880324";
Copy link
Member

Choose a reason for hiding this comment

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

what do you think about taking this opportunity to pull in a newer one of these?

Suggested change
+ "-20211213.1570880324";
+ "-20250915.17728045097";

Copy link
Member Author

Choose a reason for hiding this comment

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

This should better be a renovate rule then - I'd suggest to do that as a follow-up.

An idea would be to have the same tag for all images to make it easier to create a rule.

The tag is set here:

opentelemetry-java-instrumentation/.github/workflows/reusable-publish-smoke-test-images.yml

Line 61 in 98072b9

run: echo "TAG=$(date '+%Y%m%d').$GITHUB_RUN_ID" >> $GITHUB_ENV

If we change it to GITHUB_SHA then it would be the same for all artfacts.

@trask wdyt?

smoke-tests/src/test/java/io/opentelemetry/smoketest/QuarkusSmokeTest.java Outdated
protected String getTargetImage(String jdk) {
return "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-quarkus:jdk"
+ jdk
+ "-20241105.11678591860";
Copy link
Member

Choose a reason for hiding this comment

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

latest

Suggested change
+ "-20241105.11678591860";
+ "-20250915.17728045126";

smoke-tests/src/test/java/io/opentelemetry/smoketest/LogsSmokeTest.java
protected String getTargetImage(String jdk) {
return "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-spring-boot:jdk"
+ jdk
+ "-20211213.1570880324";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
+ "-20211213.1570880324";
+ "-20250915.17728045097";

Copy link
Contributor

Choose a reason for hiding this comment

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

the new image doesn't seem to work

smoke-tests/src/test/java/io/opentelemetry/smoketest/SecurityManagerSmokeTest.java Outdated
protected String getTargetImage(String jdk) {
return "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-security-manager:jdk"
+ jdk
+ "-20241021.11448062560";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
+ "-20241021.11448062560";
+ "-20250915.17728045123";

@laurit laurit enabled auto-merge (squash) September 16, 2025 07:22
@laurit laurit merged commit 011201c into open-telemetry:main Sep 16, 2025
89 checks passed
@zeitlinger zeitlinger deleted the convert-groovy-smoke-tests-to-java branch September 16, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaydeluca jaydeluca jaydeluca left review comments

@laurit laurit laurit approved these changes

Assignees

@zeitlinger zeitlinger

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zeitlinger @laurit @jaydeluca