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

Add regression test for JDK 17 record annotations fix #4423

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

jerboaa
Copy link
Collaborator

@jerboaa jerboaa commented Mar 23, 2022

This adds a simple app using record annotations, for which a
native image gets generated. This is a regression test for
the fix described in issue #3984

@jerboaa
Copy link
Collaborator Author

jerboaa commented Mar 23, 2022

@loicottet @olpaw Could you please have a look at this? I'm not sure what the best practise for native image tests is. Thanks!

@zakkak
Copy link
Collaborator

zakkak commented Mar 23, 2022

@jerboaa I believe the test should better be placed under https://github.com/oracle/graal/tree/master/substratevm/src/com.oracle.svm.test and I don't think it needs a special gate tag, i.e., it's OK to run it with the test tag.

@olpaw
Copy link
Member

olpaw commented Mar 24, 2022

@jerboaa I believe the test should better be placed under https://github.com/oracle/graal/tree/master/substratevm/src/com.oracle.svm.test and I don't think it needs a special gate tag, i.e., it's OK to run it with the test tag.

@zakkak is right. If we would add an mx <command> for every issue we are fixing, things would get out of hand rather soon.

@jerboaa please add a jdk17 specific test project com.oracle.svm.test.jdk17 to substratevm/mx.substratevm/suite.pywith "javaCompliance": "17+" and add it to distribution "SVM_TESTS". Then add your test in there as a junit test case. No need to make any changes to mx_substratevm.py

@jerboaa
Copy link
Collaborator Author

jerboaa commented Mar 24, 2022

@olpaw @zakkak OK, thanks. Will do.

@jerboaa jerboaa force-pushed the record-annotations-test branch from 8c09ff5 to 1d4a6dc Compare March 24, 2022 11:10
@jerboaa jerboaa force-pushed the record-annotations-test branch 4 times, most recently from 1f3f802 to f75b23b Compare March 24, 2022 14:20
This adds a JDK 17+ test project and adds a regression test
for the fix described in issue oracle#3984
@jerboaa jerboaa force-pushed the record-annotations-test branch from f75b23b to bbef970 Compare March 24, 2022 15:11
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.

LGTM, thank you @jerboaa!

@jerboaa
Copy link
Collaborator Author

jerboaa commented Mar 25, 2022

If somebody could help getting this integrated, I'd appreciate it.

@jerboaa
Copy link
Collaborator Author

jerboaa commented Apr 4, 2022

Gentle, ping. Could we get this integrated, please? Thank you!

@loicottet
Copy link
Member

This flew under my radar, sorry. I have started the process to integrate this and it should be in soon.

@jerboaa
Copy link
Collaborator Author

jerboaa commented Apr 4, 2022

Thanks, @loicottet

@graalvmbot graalvmbot merged commit fa4448b into oracle:master Apr 6, 2022
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.

5 participants