Repository navigation
fix: adding aot compiliation to quarkus tests - #48891
Conversation
c64982b to
1bcf104
Compare
|
It looks like rebalancing isn't needed. With a few more runs it looks like the total time for the slow/fast ci runs is typically 4-5 minutes faster. That puts us just a little under 30 minutes for each, which I recall was roughly the goal with splitting the test run all those months ago... Seeing the commits going back to 2022 trying to address the perfomance of the quarkus tests makes me think we probably aren't done yet. After we get this, and the change by @michalvavrik, merged is it worth looking at trying to optimize reaugmentation, or should we just wait until a few more of the long running individual test methods that are applicable for the new test framework and can be moved over as time goes by. cc @vmuzikar |
|
I don't have much experience with AOT, in order to review properly, I'd need to sit down on this. I can do that this evening at the earliest, but you don't need to wait with a review. If others review before, just merge. I don't expect there is an issue to find. |
| if (Boolean.getBoolean("kc.quarkus.tests.aot")) { | ||
| String version = String.valueOf(runtimeVersion.version().get(0)); | ||
| if (Integer.parseInt(version) < 25) { | ||
| throw new AssertionError("AOT requested, but the java version is less than 25"); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
| LOG.infof("Creating pre-initialized database %sfor reuse", useAot ? "and aot cache ":""); | ||
|
|
||
| try { | ||
| CLIResult result = invoke(SCRIPT_CMD, List.of("start-dev", "-Dinit_db_only=true", "-Dkc.config.built=true")); |
There was a problem hiding this comment.
now we will need to use launch mode exit_before_bootstrap
closes: keycloak#48846 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
| 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 |
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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.
…ycloakDistribution.java Co-authored-by: Michal Vavřík <43821672+michalvavrik@users.noreply.github.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com>
…ycloakDistribution.java Co-authored-by: Pedro Ruivo <pruivo@users.noreply.github.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com>
I applied the change - can't remember why I was using a string initially, but obviously didn't need to. |
| // 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 |
There was a problem hiding this comment.
Can you create a follow-up issue so we don't forget to remove this workaround later?
* fix: adding aot compiliation to quarkus tests closes: keycloak#48846 Signed-off-by: Steve Hawkins <shawkins@redhat.com> * Update quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java Co-authored-by: Michal Vavřík <43821672+michalvavrik@users.noreply.github.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com> * Update quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java Co-authored-by: Pedro Ruivo <pruivo@users.noreply.github.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com> --------- Signed-off-by: Steve Hawkins <shawkins@redhat.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com> Co-authored-by: Michal Vavřík <43821672+michalvavrik@users.noreply.github.com> Co-authored-by: Pedro Ruivo <pruivo@users.noreply.github.com>
closes: #48846
Adds aot compilation via a system property.
Also a small rebalancing of fast / slow tests.