fix(jetty-client:12.0.0.beta0): resolve test failures #601
Conversation
- Updated the version of 'org.eclipse.jetty:jetty-client' to 11.0.12 in the testImplementation scope and regenerated metadata.
|
@fniephaus please review this pull request when possible as we are planning to have a new release of reachability metadata (+Native Build Tools) soon. |
fniephaus
requested review from
mhalbritter and
dnestoro
and removed request for
fniephaus
There was a problem hiding this comment.
Looks okay. Thanks for your contribution! I just added few minor comments. Please address them and resolve current merge conflicts.
@mhalbritter please review this PR since you added initial metadata and tests for jetty-client
| graalvmNative { | ||
| binaries { | ||
| test { | ||
| buildArgs.add('--enable-preview') |
There was a problem hiding this comment.
Why did you use this build arg?
| "resources":{ | ||
| "includes":[{ | ||
| "condition":{"typeReachable":"org.eclipse.jetty.client.http.HttpReceiverOverHTTP"}, | ||
| "pattern":"\\QMETA-INF/services/org.eclipse.jetty.http.HttpFieldPreEncoder\\E" |
There was a problem hiding this comment.
This should be covered by the ServiceLoader integration GraalVM has. Can be removed.
| "pattern":"\\Qorg/eclipse/jetty/version/build.properties\\E" | ||
| }, { | ||
| "condition":{"typeReachable":"org.eclipse.jetty.client.AbstractConnectorHttpClientTransport"}, | ||
| "pattern":"java.base:\\Qjdk/internal/icu/impl/data/icudt67b/nfc.nrm\\E" |
There was a problem hiding this comment.
This looks suspicious. Try to remove it and see if tests still pass.
| String libraryVersion = tck.testedLibraryVersion.get() | ||
|
|
||
| dependencies { | ||
| testImplementation "org.eclipse.jetty:jetty-client:11.0.12" |
There was a problem hiding this comment.
Shouldn't this be $libraryVersion? Right now, the metadata should be for 12.0.0-beta0 but has been tested against 11.0.12?
There was a problem hiding this comment.
And I guess even the agent collected it from 11.0.12?
|
Hello, i've left some remarks. |
What does this PR do?
Checklist before merging