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.

Sign up for GitHub

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

Jump to bottom

Use jdk17 for quarkus maven plugin and extension maven plugin #42739

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use jdk17 for quarkus maven plugin and extension maven plugin #42739

wants to merge 1 commit into from

Conversation

manofthepeace
Copy link
Contributor

@manofthepeace manofthepeace commented Aug 23, 2024 •
edited
Loading

This needs a few things in order to work.

It needs a recent maven to build as it needs (simplified)

[INFO] +- org.apache.maven:maven-core:jar:3.9.8:provided
[INFO] |  +- org.eclipse.sisu:org.eclipse.sisu.inject:jar:0.9.0.M3:provided
[INFO] |  +- com.google.inject:guice:jar:5.1.0:provided

guice 5.1.0 is needed for JDK17 support. Sisu brings a old guice so using the one provided by maven-core make the thing work.

This: quarkusio/quarkus-quickstarts#1444 was also needed otherwise the CI step "Quickstarts Compilation - JDK 17" would fail for the same reason (guice).

CI is green on my fork.

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/kubernetes area/maven labels Aug 23, 2024
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Aug 23, 2024 •
edited by gsmet
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 2a3c730.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/infinispan-cache/deployment

✖ io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls - History

  • expected: "thread1" but was: "thread2" - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: 

expected: "thread1"
 but was: "thread2"
	at io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls(InfinispanCacheTest.java:283)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:499)
	at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:413)

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/smallrye-graphql

✖ io.quarkus.it.smallrye.graphql.MicrometerMetricsTest.shouldCreateMetricsFromAsyncQueryAndAsyncSourceField - History

  • 1 expectation failed. Response body doesn't match expectation. Expected: a string containing "mp_graphql_seconds_count{name=\"asyncSuperMetricFoo\",source=\"false\",type=\"QUERY\"}" Actual: # TYPE mp_graphql_seconds summary \# HELP mp_graphql_seconds - java.lang.AssertionError
java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.
Expected: a string containing "mp_graphql_seconds_count{name=\"asyncSuperMetricFoo\",source=\"false\",type=\"QUERY\"}"
  Actual: # TYPE mp_graphql_seconds summary
# HELP mp_graphql_seconds  
mp_graphql_seconds_count{name="clearMetrics",source="false",type="MUTATION"} 1.0
mp_graphql_seconds_sum{name="clearMetrics",source="false",type="MUTATION"} 5.56399E-4

gsmet
gsmet requested changes Aug 24, 2024
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

We need to make sure this doesn't break Maven 3.8.6 (yes, I know but that's the reason why we didn't do it for now - we have to support people building Quarkus apps/extensions with Maven 3.8.6 for a while).

@manofthepeace
Copy link
Contributor Author

@gsmet Should I abandon this? or daft for a long while. We could add the needed stuff as compile deps instead of provided but not sure its worth it?

@gsmet
Copy link
Member

gsmet commented Aug 26, 2024

Make if draft for now, I would say.

@manofthepeace manofthepeace marked this pull request as draft August 26, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/kubernetes area/maven triage/flaky-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@manofthepeace @gsmet