Skip to content
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

Test OTLP exporters with different OkHttp versions #6045

Merged

Conversation

jack-berg
Copy link
Member

A potential solution to #6026.

Allows us to define a set of OkHttp versions to test the OTLP exporters against, besides the latest.

With this, we can tell users who are not satisfied with using the latest OkHttp version that they can downgrade the OkHttp dependency resolution to some other version in the tested set.

@@ -41,6 +39,45 @@ val testJavaVersion: String? by project

testing {
suites {
listOf(
"LATEST",
"4.11.0"
Copy link
Member Author

@jack-berg jack-berg Dec 5, 2023

Choose a reason for hiding this comment

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

We would enumerate the list of tested versions here. "LATEST" is a special keyword which just uses the dependency from :dependencyConfiguration.

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3f1b9ed) 91.18% compared to head (576fa89) 91.18%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6045   +/-   ##
=========================================
  Coverage     91.18%   91.18%           
  Complexity     5623     5623           
=========================================
  Files           618      618           
  Lines         16580    16580           
  Branches       1642     1642           
=========================================
  Hits          15118    15118           
  Misses         1013     1013           
  Partials        449      449           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jack-berg jack-berg marked this pull request as ready for review December 18, 2023 22:10
@jack-berg jack-berg requested a review from a team December 18, 2023 22:10
@jack-berg
Copy link
Member Author

Discussed in the 12/14/2023 SIG meeting and there was no disagreement on maintaining these types of tests. Marking as "ready to review".

@@ -41,6 +39,45 @@ val testJavaVersion: String? by project

testing {
suites {
listOf(
"LATEST",
Copy link
Member

Choose a reason for hiding this comment

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

does the latest version still get tested outside of this new suite also?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indirectly via NoGrpcJavaOtlpIntegrationTest.

But the code for testing the OkHttp based sender was previously in the default test source set. Those test classes have been relocated to testDefaultSender in this PR, which runs as part of these new test suites.

The latest will still always run locally (not just on CI), because of the line further down:

enabled = it.equals("LATEST") || "true".equals(System.getenv("CI"))

Copy link
Member

Choose a reason for hiding this comment

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

Those test classes have been relocated to testDefaultSender in this PR, which runs as part of these new test suites.

ah, that's what I missed, thx!

@jack-berg jack-berg merged commit de65a4b into open-telemetry:main Jan 2, 2024
18 checks passed
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.

2 participants