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.

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

Introduce env variable QUARKUS_NATIVE_IT_SKIP_VERIFY_IMAGE_METRICS #37820

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Dec 18, 2023

Allow users to skip tests verifying native image build metrics (such as
binary size, classes registered for reflection, etc.) by setting env
variable QUARKUS_NATIVE_IT_SKIP_VERIFY_IMAGE_METRICS to true.

Quarkus uses the "jvmci" version from the base-JDK used to build
GraalVM. When building GraalVM master with JDK 21, however, we are using
LabsJDK 21.0.1+12-jvmci-23.1-b26, so Quarkus thinks the GraalVM version
is 23.1 despite it acutally being 24.0. Since upstream GraalVM doesn't
plan on releasing GraalVM 24.0 based on JDK 21 it's not expected to get
a jvmci version bump in LabsJDK 21.

This patch allows us to keep testing GraalVM 24.0-dev with JDK 21 without
getting false negatives.

This comment has been minimized.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

I just added a minor comment

* Allow users to skip this kind of tests by setting env variable QUARKUS_NATIVE_IT_SKIP_VERIFY_IMAGE_METRICS to true
*/
String skipVerifyImageMetrics = System.getenv("QUARKUS_NATIVE_IT_SKIP_VERIFY_IMAGE_METRICS");
boolean skip = skipVerifyImageMetrics != null && skipVerifyImageMetrics.equalsIgnoreCase("true");
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpicking: you can use Boolean.valueOf here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, using Boolean.parseBoolean though since there is no need for the variable to be a Boolean instead of a boolean.

Copy link
Contributor

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

+1 on the suggestion to use Boolean.valueOf(System.getenv("QUARKUS_NATIVE_IT_SKIP_VERIFY_IMAGE_METRICS"))

Allow users to skip tests verifying native image build metrics (such as
binary size, classes registered for reflection, etc.)  by setting env
variable QUARKUS_NATIVE_IT_SKIP_VERIFY_IMAGE_METRICS to true.

Quarkus uses the "jvmci" version from the base-JDK used to build
GraalVM. When building GraalVM master with JDK 21, however, we are using
LabsJDK 21.0.1+12-jvmci-23.1-b26, so Quarkus thinks the GraalVM version
is 23.1 despite it acutally being 24.0. Since upstream GraalVM doesn't
plan on releasing GraalVM 24.0 based on JDK 21 it's not expected to get
a jvmci version bump in LabsJDK 21.

This patch allows us to keep testing GraalVM 24.0-dev with JDK 21 without
getting false negatives.
@zakkak zakkak force-pushed the 2023-12-18-opt-out-from-metrics-it branch from d92738c to 21f055a Compare December 19, 2023 12:54
@zakkak zakkak added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 19, 2023
Copy link

quarkus-bot bot commented Dec 19, 2023

Failing Jobs - Building 21f055a

Status Name Step Failures Logs Raw logs Build scan
Native Tests - Data1 Update Docker Client User Agent ⚠️ Check → Logs Raw logs 🚧
Native Tests - HTTP Update Docker Client User Agent ⚠️ Check → Logs Raw logs 🚧

You can also consult the Develocity build scans.

@zakkak
Copy link
Contributor Author

zakkak commented Dec 19, 2023

Failures are due to:

/home/runner/.docker/config.json: No such file or directory

The ImageMetricsITCase seems to work as expected.

@zakkak zakkak merged commit 4d07a91 into quarkusio:main Dec 19, 2023
47 of 49 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 19, 2023
@zakkak zakkak deleted the 2023-12-18-opt-out-from-metrics-it branch December 19, 2023 16:55
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants