Skip to content

Conversation

@laurit
Copy link
Contributor

@laurit laurit commented Sep 25, 2025

An alternative for #14731
This PR largely copies from #14731 The main difference is that #14731 puts test container management into SmokeTestInstrumentationExtension while this PR puts it into an abstract base class. Hopefully by not coupling SmokeTestInstrumentationExtension with our smoke tests it will be usable outside this project and provide users a better way for asserting the telemetry produced by the agent. For example we could use it in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/examples/distro/smoke-tests/src/test/java/com/example/javaagent/smoketest/SmokeTest.java or in vendor distros.
This PR retains the @AppServer annotation from groovy and #14731 I like that it lets you easily run the some tests for one server version. If we'd go with @ParameterizedTest then you'd always need to alter the test parameters to just keep the one version you wish to run. Also with @ParameterizedTest we might need to merge all test cases into one method to void starting the same server multiple times. The benefit of using @ParameterizedTest would be that it is an established pattern and the code would be much more compact. Could also try some dynamic test magic.

@laurit laurit requested a review from a team as a code owner September 25, 2025 12:52
@laurit laurit requested a review from zeitlinger September 25, 2025 12:52
Copy link
Member

@zeitlinger zeitlinger left a comment

Choose a reason for hiding this comment

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

I like it - it's similar to a version that I had at one point - but your version doesn't have the rough edges that had with the superclass approach.

@trask trask merged commit 38c2b69 into open-telemetry:main Sep 25, 2025
89 checks passed
@laurit laurit deleted the smoke-tests-groovy branch September 25, 2025 15:55
mznet pushed a commit to mznet/opentelemetry-java-instrumentation that referenced this pull request Sep 26, 2025
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