test latest deps cleanup #5269
test latest deps cleanup #5269
Conversation
trask
force-pushed
the
test-latest-deps-cleanup
branch
2 times, most recently
from
17118ce
to
164e523
Compare
trask
force-pushed
the
test-latest-deps-cleanup
branch
from
b94accb
to
5fc506d
Compare
trask
force-pushed
the
test-latest-deps-cleanup
branch
from
16ecd10
to
3265d0c
Compare
| // FIXME: We don't currently handle jetty continuations properly (at all). | ||
| abstract class JettyContinuationHandlerTest extends JettyHandlerTest { |
There was a problem hiding this comment.
Jetty Continuation was deprecated in jetty 9.4 and removed in jetty 10, so this test doesn't compile against jetty 10 (see latestDepTestLibrary bump in the gradle file).
Since this test doesn't run anyways (it's abstract) and based on the FIXME we don't support Continuation anyways, it seems ok to just delete it.
| testLibrary("org.elasticsearch.plugin:transport-netty3-client:5.0.0") | ||
| testLibrary("org.elasticsearch.client:transport:5.0.0") | ||
|
|
||
| latestDepTestLibrary("org.elasticsearch.plugin:transport-netty3-client:5.0.0") |
There was a problem hiding this comment.
tests were failing to pass on 5.2.+ so I bailed on it for this PR
There was a problem hiding this comment.
I'll revisit this in subsequent PR (since it doesn't have a trailing comment yet, it's still not "done" from perspective of #5227)
| @@ -43,8 +43,4 @@ dependencies { | |||
|
|
|||
| testLibrary("org.springframework.boot:spring-boot-autoconfigure:$springBootVersion") | |||
| testLibrary("org.springframework.boot:spring-boot-starter-tomcat:$springBootVersion") | |||
|
|
|||
| // spring boot 3 uses spring 6 which requires java 17 | |||
| latestDepTestLibrary("org.springframework.boot:spring-boot-autoconfigure:2.+") | |||
There was a problem hiding this comment.
This will still be a problem when Spring 3 is released
There was a problem hiding this comment.
is your preference to keep this and open a tracking issue now?
There was a problem hiding this comment.
I think yes, that will remove a time pressure from us.
There was a problem hiding this comment.
by "remove time pressure" do you mean it will give us more time to fix the issue and support spring boot 3 before it's GA'd? if this is a concern, maybe we should add some exceptions to #5258?
There was a problem hiding this comment.
No, I mean that if we keep this constraint, then when Spring Boot 3 is released we will NOT be forced to fix it ASAP because our build broke.
There was a problem hiding this comment.
I personally think that's a good thing, it's sort of an alarm bell that Spring Boot 3 has GA'd. It's easy enough to snooze the alarm by adding the suppression + logging the issue at that time.
There was a problem hiding this comment.
going to merge, I think removing it is most consistent from #5258 perspective, and I think the alarm bell when unsupported GAs are released is a good thing for us, but we can revisit if this feels problematic
Co-authored-by: Nikita Salnikov-Tarnovski <[email protected]>
* test latest deps cleanup * Revert currently irrelevant change * Update instrumentation/lettuce/lettuce-4.0/javaagent/build.gradle.kts Co-authored-by: Nikita Salnikov-Tarnovski <[email protected]> Co-authored-by: Nikita Salnikov-Tarnovski <[email protected]>
part of #5227
and applying some benefits from #5258
this PR adds trailing comments to about half of all
latestDepTestLibraryusages, categorizing them as// documented limitation- version limitation is in official docs// issue #X- issue tracking the limitation, issue should be tagged withtest latest deps// see X module- more recent versions supported by another instrumentation modulewill tackle more in subsequent PRs and will see if any additional categories are needed still