-
Notifications
You must be signed in to change notification settings - Fork 967
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
Conversation
2a17ca7
to
ea42209
Compare
@@ -0,0 +1,119 @@ | |||
/* |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.+") |
There was a problem hiding this comment.
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
Lines 32 to 39 in 7400025
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
There was a problem hiding this comment.
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.
…ion in testLatestDeps
Co-authored-by: Lauri Tulmin <[email protected]>
Related to #7195