Skip to content

Convert Wicket groovy tests to java #9867

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

Merged
merged 9 commits into from
Nov 22, 2023

Conversation

happyuser23
Copy link
Contributor

Related to #7195

@happyuser23 happyuser23 requested a review from a team November 14, 2023 17:45
Copy link

linux-foundation-easycla bot commented Nov 14, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@@ -0,0 +1,119 @@
/*
Copy link
Contributor

@laurit laurit Nov 17, 2023

Choose a reason for hiding this comment

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

our convention is to place java tests in the same package as instrumentation code, groovy tests are usually in the root package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -23,4 +23,7 @@ dependencies {

testInstrumentation(project(":instrumentation:servlet:servlet-3.0:javaagent"))
testInstrumentation(project(":instrumentation:servlet:servlet-javax-common:javaagent"))

// Wicket 9 requires Java 11
latestDepTestLibrary("org.apache.wicket:wicket:8.+")
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to change the minimum required java version for latest dep tests instead of limiting the tested version range

val latestDepTest = findProperty("testLatestDeps") as Boolean
// spring 6 requires java 17
if (latestDepTest) {
otelJava {
minJavaVersionSupported.set(JavaVersion.VERSION_17)
}
}

Removing the latestDepTestLibrary line would also be fine, we don't run latest dep tests with jdk8 so this won't fail in the CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

testLatestDeps3 was failing.
Anyway, I updated as suggested to test with minimum required java version.

@laurit laurit merged commit 7c3a412 into open-telemetry:main Nov 22, 2023
@happyuser23 happyuser23 deleted the convert-wicket-test branch December 5, 2023 17:22
github-actions bot pushed a commit that referenced this pull request Mar 12, 2024
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