-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Add basic jlink smoke test for Temurin #4067
Add basic jlink smoke test for Temurin #4067
Conversation
Grinder that fails the test on Windows due to #4057 (signing issue):
Grinder that passes the test on Linux (doesn't have the signing issue):
Grinder that fails on mac for the same issue as Windows:
|
@andrew-m-leonard This should be a basic test that you could use for the build changes that we need. FYI. |
b0b1173
to
45671c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to any other reviewers: I got a bit nervous when the git view seemed to indicate you'd changed
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
to
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
But the originals have just been moved, so there are only additions to the imports here.
Approving on the basis that the tests results from the Grinders look like they're doing what is required :-)
If we haven't already got them we should possibly also ensure that Grinders run clean on the 24+23 builds (before the changes assiciated with the JEP went in) and a jdk11u to make sure they're ok with "good" builds on the range of versions we support.
Grinder pipeline for JDK 21, 17, 11, 23 - Linux, Windows, Mac: |
The windows result on JDK 17 for example looks like this:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Thanks. I'll leave this for @andrew-m-leonard to merge when he thinks it's a good time. E.g. when a fix for #4057 is available too. |
Yes I guess we shouldn't merge this until 4057 is resolved too otherwise the smoke tests will fail for 24 and no other testing will run (Although maybe that's a good thing for one EA cycle? ;-) I'll also leave it to @andrew-m-leonard to decide) |
Putting into draft to avoid accidental early merge |
Dual signing and post build signing fix:adoptium/ci-jenkins-pipelines#1157 |
Nice work! So it didn't require any changes to the signing setup pre-jlink after all. Just skip the signing step after. Nice.
Confirmed!
Do you have a Grinder run for mac as well? |
Seems a problem with Orka not provisioning currently... |
Created new fix that handles Mac libjli.dylib problem.
|
Well done! Where is the new fix that handles the above? |
45671c8
to
d844a53
Compare
Basically runs `jlink --add-modules java.base --output <output> --verbose`. This is to ensure that signing of builds don't break the jlink functionality. Especially when linking from the run-time image (JEP 493 in JDK 24+).
d844a53
to
55d74ae
Compare
#4083 now merged |
Thanks, Andrew! |
Basically runs
jlink --add-modules java.base --output <output> --verbose
. This is to ensure that signing of builds don't break the jlink functionality. Especially when linking from the run-time image (JEP 493 in JDK 24+).