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
Run CI with GraalVM #2476
base: master
Are you sure you want to change the base?
Run CI with GraalVM #2476
Conversation
.github/workflows/graal-ci.yml
Outdated
|
||
- name: build | ||
env: | ||
MAVEN_CONFIG: "-Dbasepom.check.skip-enforcer=false -B" |
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.
the ci.yml has -B -fae
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.
Yes, I left this out intentionally, it is easier to track down errors when they are always at the end of the build log
1e5fc6a
to
c326930
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
|
||
<profile> | ||
<id>native</id> | ||
<activation> |
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.
all of the modules where this needs to be activated are packaging jars. can't we just use
<property>
<name>packaging</name>
<value>!pom</value>
</property>
and avoid more dotfiles?
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.
this actually does not work because maven only supports a single property for activation. sigh. maven is terrible.
</file> | ||
</activation> | ||
<build> | ||
<pluginManagement> |
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.
this should move outside the profile, into the pluginManagement section above.
<configuration> | ||
<!-- ... --> | ||
</configuration> | ||
<executions> |
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.
those should stay here, move into the <plugin>
below.
Have you actually gotten this to work? I get
|
No, I got stuck there for now. I will take another look at some point. |
Any progress on this? Thanks. |
Hi @Eng-Fouad , sorry I did not look any more yet. I did try the suggested junit configuration tweaks but still ran into the NoSuchMethodError above, and didn't really have the tools to understand what went wrong. We will get this working eventually. |
c326930
to
e12373b
Compare
I rebased the branch and adopted the latest version of the native plugin. Let's see how the build does. |
fd48951
to
8d00612
Compare
Ok, the good news is the newer Graal and plugin versions seem to make development a lot easier. |
Now I am stuck on getting
I thought the metadata repository would take care of making Mockito work:
|
It looks Metadata for Mockito does not include proxy-config.json:
Then use either |
So, by enabling the agent experimental predefined classes, the proxy starts working.
|
I filed: mockito/mockito#3183 |
b0c7281
to
f21d795
Compare
f21d795
to
cf97baf
Compare
cf97baf
to
f08ad5f
Compare
Some minor progress here - by reducing Mockito usage to only interfaces, and disabling codegen and using Proxy mocks instead, the "more than one class" problem goes away and instead I get some test failures instead. Still more digging needed though. |
f08ad5f
to
3851383
Compare
We now removed all uses of the bytebuddy agent and Mockito only runs with pure JVM proxies in mainline. Rebased on top of that, running the build again... |
I guess some reflection metadata is being lost, despite using the agent. |
According to GraalVM maven plugin docs:
Also:
So the sole purpose of the agent is to help generating the native metadata configurations (i.e. How about creating the native metadata configuration files manually? See this gist. In addition to the classes mentioned in the gist, we need to include all classes (in tests) that are being constructed/accessed by reflection. For example, the following beans need to be registered in jdbi/core/src/test/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapperTest.java Lines 42 to 47 in 5a33a0f
For reference, see how they maintain |
The main reason is that it is a pain in the ass to get right, for something that a human really "shouldn't" have to write, and there are many hundreds of test cases :) |
Thanks for the reference to the Netty config - theirs is very short, so I can see how it's not too big of a deal to maintain. Ours is probably going to be much longer, because we have a ton of random types both in main and test that we access via reflection. |
Probably it is better to create/maintain configs for main sources manually, and use the agent for test sources only. |
b70f674
to
6918e6a
Compare
Ok, I have a bit more information on the test failures. It looks like a constructor parameter for a test case |
I looked through the |
Interesting, I extracted the code in question into a sample project to have a reproducer, and it works there ??? |
Ok, so I think the problem is somehow introduced from the jdbi parent pom. When I inherit from our Jdbi build pom, the test fails. When I inline the native-image configuration and depend on basepom, skipping jdbi build configuration, the test passes. |
I think this is a native-image bug. |
752aa7c
to
9ae2ff5
Compare
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 1 New issue |
Ok, I added a workaround to the issue I filed above, which we cannot merge, but should get us moving forward to the next problem for now at least. And...
🤔 I'll have to come back to this later |
Is this relevant? the release notes of JUnit 5.10.0:
|
It could be! I don't get why |
No description provided.