8000 fix: adding aot compiliation to quarkus tests by shawkins · Pull Request #48891 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

fix: adding aot compiliation to quarkus tests #48891

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

Merged
vmuzikar merged 3 commits into from
May 13, 2026
Merged

fix: adding aot compiliation to quarkus tests #48891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd56361
fix: adding aot compiliation to quarkus tests
shawkins May 11, 2026
3f9d82b
Update quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKe…
shawkins May 12, 2026
2599caf
Update quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKe…
shawkins May 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ jobs:
run: |
declare -A PARAMS
PARAMS["win"]="-Dkc.quarkus.tests.groups=win"
PARAMS["zip-slow"]="-Dkc.quarkus.tests.groups=slow"
PARAMS["zip-fast"]='-Dkc.quarkus.tests.groups=!slow'
PARAMS["zip-slow"]="-Dkc.quarkus.tests.groups=slow -Dkc.quarkus.tests.aot=true"
PARAMS["zip-fast"]='-Dkc.quarkus.tests.groups=!slow -Dkc.quarkus.tests.aot=true'
PARAMS["storage"]="-Ptest-database"
PARAMS["smoke"]="-Dkc.quarkus.tests.groups=smoke"

Expand Down
28 changes: 28 additions & 0 deletions quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/HelpCommandDistTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.stream.Stream;

import org.keycloak.it.junit5.extension.CLIResult;
import org.keycloak.it.junit5.extension.DistributionTest;
Expand All @@ -42,6 +43,11 @@
import io.quarkus.test.junit.main.Launch;
import org.apache.commons.io.FileUtils;
import org.approvaltests.Approvals;
import org.approvaltests.core.ApprovalFailureReporter;
import org.approvaltests.reporters.intellij.IntelliJReporter;
import org.approvaltests.reporters.linux.LinuxDiffReporter;
import org.approvaltests.reporters.macosx.MacDiffReporter;
import org.approvaltests.reporters.windows.WindowsDiffReporter;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -223,8 +229,30 @@ private void assertHelp(CLIResult cliResult) {
.replace("data\\log\\", "data/log/")
.replace("\r\n", "\n");
}

if (output.startsWith("Appending additional Java properties to JAVA_OPTS")) {
output = output.substring(output.indexOf("\n") + 1);
}

try {
// on failure with java 25, the intellijreporter seems to work, but really just throws
// an exception later - https://github.com/approvals/ApprovalTests.Java/issues/800
Comment on lines +238 to +239
@vmuzikar vmuzikar May 13, 2026
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you create a follow-up issue so we don't forget to remove this workaround later?

@shawkins shawkins May 13, 2026
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#48976

if (IntelliJReporter.findJetBrainsIdes().equals("")) {
Stream.of(WindowsDiffReporter.INSTANCE, LinuxDiffReporter.INSTANCE, MacDiffReporter.INSTANCE).forEach(reporter -> {
var reporters = reporter.getReporters();
for (int i = 0; i < reporters.length; i++) {
if (reporters[i] instanceof IntelliJReporter) {
reporters[i] = new ApprovalFailureReporter() {

@Override
public boolean report(String received, String approved) {
return false;
}
};
}
}
});
}
Approvals.verify(output);
} catch (Error cause) {
if ("true".equals(System.getenv(REPLACE_EXPECTED))) {
Expand Down
66 changes: 50 additions & 16 deletions quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.UncheckedIOException;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -67,7 +68,6 @@
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;

import io.quarkus.deployment.util.FileUtil;
import io.quarkus.deployment.util.IoUtil;
import io.quarkus.fs.util.ZipUtils;
import io.restassured.RestAssured;
import org.apache.commons.io.FileUtils;
Expand All @@ -78,7 +78,9 @@
import org.jboss.shrinkwrap.api.exporter.ZipExporter;
import org.jboss.shrinkwrap.api.spec.JavaArchive;

import static org.keycloak.quarkus.runtime.Environment.KC_CONFIG_BUILT;
import static org.keycloak.quarkus.runtime.Environment.LAUNCH_MODE;
import static org.keycloak.quarkus.runtime.Environment.LAUNCH_MODE_EXIT_BEFORE_BOOTSTRAP;
import static org.keycloak.quarkus.runtime.Environment.isWindows;

public final class RawKeycloakDistribution implements KeycloakDistribution {
Expand Down Expand Up @@ -114,6 +116,13 @@ public RawKeycloakDistribution(boolean debug, boolean manualStop, boolean enable
this.removeBuildOptionsAfterBuild = removeBuildOptionsAfterBuild;
this.requestPort = requestPort;
this.distPath = prepareDistribution();
if (reCreate) {
try {
preInitH2(distPath);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
this.outputConsumer = new DefaultOutputConsumer();
}

Expand Down Expand Up @@ -167,6 +176,8 @@ private CLIResult invoke(String script, List<String> arguments) throws IOExcepti
builder.environment().put("DEBUG_SUSPEND", "y");
}*/

addAOTEnvVars();

builder.environment().putAll(envVars);

Process proc = builder.start();
Expand Down Expand Up @@ -545,7 +556,6 @@ private Path prepareDistribution() {
ZipUtils.unzip(distFile.toPath(), distRootPath);
FileUtils.copyDirectory(dPath.resolve("conf").toFile(), dPath.resolve("conf-bak").toFile());
FileUtils.copyDirectory(dPath.resolve("lib").resolve("quarkus").toFile(), dPath.resolve("quarkus-bak").toFile());
preInitH2(dPath);
postInit(dPath);
}

Expand All @@ -564,29 +574,42 @@ private Path prepareDistribution() {
}

private void preInitH2(Path dPath) throws IOException {
LOG.info("Creating pre-initialized database for reuse");
ProcessHandle.current().info().command().ifPresent(command -> {
ProcessBuilder pb = new ProcessBuilder(List.of(command, "-D" + LAUNCH_MODE + "=" + Environment.LAUNCH_MODE_EXIT_BEFORE_BOOTSTRAP,
"-Dkc.home.dir=" + dPath.toFile().getAbsolutePath(), "-Dkc.config.built=true",
"-Dkc.db=dev-file", "-jar", dPath.resolve("lib").resolve("quarkus-run.jar").toFile().getAbsolutePath(), "start-dev"));
try {
Process dbInit = pb.start();
if (!dbInit.waitFor(30, TimeUnit.SECONDS)) {
throw new RuntimeException("Pre-initialized db process did not exit");
Runtime.Version runtimeVersion = Runtime.version();
boolean useAot = false;
if (Boolean.getBoolean("kc.quarkus.tests.aot")) {
if (Runtime.version().feature() < 25) {
throw new AssertionError("AOT requested, but the java version is less than 25");
@pruivo pruivo May 12, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to be this strict? Can we just ignore the kc.quarkus.tests.aot value instead?

Another suggestion I have is to always enable if the JDK version >= 25.

@shawkins shawkins May 12, 2026
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's an overhead in creating the cache such that for running individual tests, or test methods locally, I don't want it to happen automatically - mentioned on #48846 (comment)

}
if (dbInit.exitValue() != 0) {
throw new RuntimeException("Could not create pre-initialized db, exit code %s: %s".formatted(dbInit.exitValue(), new String(IoUtil.readBytes(dbInit.getErrorStream()))));
useAot = true;

if (envVars.containsKey("JAVA_OPTS_APPEND")) {
throw new AssertionError("the raw dist marked as recreate should not have JAVA_OPTS_APPEND set");
}
File aotFile = getAotFile(dPath);
// TODO: we know for certain that the test java is 25+, so we'll set it here
envVars.put("JAVA", command);
envVars.put("JAVA_OPTS_APPEND", "-Xlog:aot -XX:AOTCacheOutput=\"%s\"".formatted(aotFile.getAbsolutePath()));
}

LOG.infof("Creating pre-initialized database %sfor reuse", useAot ? "and aot cache ":"");

try {
CLIResult result = invoke(SCRIPT_CMD, List.of("start-dev", "-D%s=%s".formatted(LAUNCH_MODE, LAUNCH_MODE_EXIT_BEFORE_BOOTSTRAP), "-D%s=true".formatted(KC_CONFIG_BUILT)));
if (result.exitCode() != 0) {
throw new RuntimeException("Could not create pre-initialized db, exit code %s: %s".formatted(result.exitCode(), new String(result.getErrorOutput())));
}
} catch (IOException e) {
throw new RuntimeException(e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
throw new UncheckedIOException(e);
}
});
FileUtils.copyDirectory(dPath.resolve("data").resolve("h2").toFile(), dPath.resolve("h2-bak").toFile());
}

private File getAotFile(Path dPath) {
return dPath.resolve("app.aot").toFile();
}

private void postInit(Path dPath) {
if (System.getProperty("product") != null) {
// JDBC drivers might be excluded if running as a product build
Expand Down Expand Up @@ -641,12 +664,23 @@ private void startServer(List<String> arguments) throws Exception {
if (debug) {
builder.environment().put("DEBUG_SUSPEND", "y");
}

addAOTEnvVars();

builder.environment().putAll(envVars);

keycloak = builder.start();
}

private void addAOTEnvVars() {
File aotFile = getAotFile(getDistPath());
if (aotFile.exists() && !envVars.containsKey("JAVA_OPTS_APPEND")) {
// TODO: we know for certain that the test java is 25+, so we'll set it here
@michalvavrik michalvavrik May 12, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For me this TODO and the same one on line 591 are not actionable as I don't understand them. What is "it"? If it is lines below, why is it "TODO"

@shawkins shawkins May 12, 2026
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When we call out to kc.sh it will look for the JAVA, JAVA_HOME, or just use java on the path. In that last case it can be picking up a java with a different version than the one used to run the test.
@michalvavrik michalvavrik May 12, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

understood, thanks

envVars.put("JAVA", ProcessHandle.current().info().command().orElseThrow());
envVars.put("JAVA_OPTS_APPEND", "-XX:AOTCache=\"%s\"".formatted(aotFile.getAbsolutePath()));
}
}

@Override
public void setManualStop(boolean manualStop) {
this.manualStop = manualStop;
Expand Down
Loading
0