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

Tests config for serialization of arrays #279

Merged
merged 4 commits into from
Sep 17, 2024
Merged

Tests config for serialization of arrays #279

merged 4 commits into from
Sep 17, 2024

Conversation

@Karm Karm requested a review from zakkak September 9, 2024 15:25
@Karm Karm self-assigned this Sep 9, 2024
@Karm Karm requested a review from jerboaa September 9, 2024 15:29
@Karm
Copy link
Owner Author

Karm commented Sep 9, 2024

 > but was: <Exception in thread "main" com.oracle.svm.core.jdk.UnsupportedFeatureError: SerializationConstructorAccessor class not found for declaringClass: [Z (targetConstructorClass: java.lang.Object). Usually adding [Z to serialization-config.json fixes the problem.
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.util.VMError.unsupportedFeature(VMError.java:121)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.reflect.serialize.SerializationSupport.getSerializationConstructorAccessor(SerializationSupport.java:164)
	at [email protected]/jdk.internal.reflect.SerializationConstructorAccessorGenerator.generateSerializationConstructor(SerializationConstructorAccessorGenerator.java:51)
	at [email protected]/jdk.internal.reflect.ReflectionFactory.generateConstructor(ReflectionFactory.java:384)
	at [email protected]/jdk.internal.reflect.ReflectionFactory.newConstructorForSerialization(ReflectionFactory.java:288)
	at [email protected]/sun.reflect.ReflectionFactory.newConstructorForSerialization(ReflectionFactory.java:100)
	at for_serialization.Main.main(Main.java:147)
	at [email protected]/java.lang.invoke.LambdaForm$DMH/sa346b79c.invokeStaticInit(LambdaForm$DMH)
>
[INFO] 
Error:  Tests run: 9, Failures: 1, Errors: 0, Skipped: 0

Expected. The fix is not in the released JDK 22 Mandrel

Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

Expected. The fix is not in the released JDK 22 Mandrel

And it's not going to get backported to it, so we probably need to disable it in that configuration.

Co-authored-by: Foivos <[email protected]>
@Karm
Copy link
Owner Author

Karm commented Sep 13, 2024

Expected. The fix is not in the released JDK 22 Mandrel

And it's not going to get backported to it, so we probably need to disable it in that configuration.

I see, indeed. Lemme update it...

@Karm Karm requested a review from zakkak September 16, 2024 12:46
@Karm
Copy link
Owner Author

Karm commented Sep 16, 2024

@zakkak How about this?
0732e2b

if it's not fixed by then, we can bump the versions so as not not fail CI unnecessarily...

@Karm
Copy link
Owner Author

Karm commented Sep 16, 2024

Eh..

Failing now on this PR:

JDK 22.0.1+8, vendor version: Mandrel-24.0.1.0-Final

Exclusions in the test:

    @Test
    @Tag("builder-image")
    @IfMandrelVersion(min = "23.1.5", maxJDK = "21.999", inContainer = true)
    public void forSerializationContainer21Test(TestInfo testInfo) throws IOException, InterruptedException {
        forSerialization(testInfo, Apps.FOR_SERIALIZATION_BUILDER_IMAGE);
    }

    @Test
    @IfMandrelVersion(min = "23.1.5", maxJDK = "21.999")
    public void forSerialization21Test(TestInfo testInfo) throws IOException, InterruptedException {
        forSerialization(testInfo, Apps.FOR_SERIALIZATION);
    }

    @Test
    @Tag("builder-image")
    @IfMandrelVersion(min = "23.1.5", minJDK = "23", inContainer = true)
    public void forSerializationContainer23Test(TestInfo testInfo) throws IOException, InterruptedException {
        forSerialization(testInfo, Apps.FOR_SERIALIZATION_BUILDER_IMAGE);
    }

    @Test
    @IfMandrelVersion(min = "23.1.5", minJDK = "23")
    public void forSerialization23Test(TestInfo testInfo) throws IOException, InterruptedException {
        forSerialization(testInfo, Apps.FOR_SERIALIZATION);
    }

I am probably blind... or it's parsing Mandrel 24 as JDK 24?

@zakkak
Copy link
Collaborator

zakkak commented Sep 16, 2024

or it's parsing Mandrel 24 as JDK 24?

The logs say

The test suite runs with Mandrel version 24.0.2 in container, JDK 22.0.2.

so it doesn't look like a parsing issue.

I think the issue is that you are passing "21.999" as the maxJDK which doesn't match the pattern in

private static final Pattern JDK_PATTERN = Pattern.compile("(?<jfeature>[0-9]+)(\\.(?<jinterim>[0-9]*)\\.(?<jupdate>[0-9]*))?");

@zakkak
Copy link
Collaborator

zakkak commented Sep 17, 2024

@Karm thanks for the fix. I opened #280 to keep track of the version parsing issue.

@zakkak zakkak merged commit 2956944 into master Sep 17, 2024
54 checks passed
@zakkak zakkak deleted the GR-52473 branch September 17, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants