-
Notifications
You must be signed in to change notification settings - Fork 79
Adds two new switches to the maven test goal #720
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
base: master
Are you sure you want to change the base?
Conversation
QnD attempt to add native compilation to the tree, run with `-Dnative`. Note: Does not work with the out of the box plugin but requires graalvm/native-build-tools#720 applied to the graalvm native plugin and that the embedded junit version (in https://github.com/graalvm/native-build-tools/blob/master/gradle/libs.versions.toml) matches the version in jdbi (currently 5.13.0-M2). native images build but don't work. This is only a "proof of concept, compiles" sketch.
QnD attempt to add native compilation to the tree, run with `-Dnative`. Note: Does not work with the out of the box plugin but requires graalvm/native-build-tools#720 applied to the graalvm native plugin and that the embedded junit version (in https://github.com/graalvm/native-build-tools/blob/master/gradle/libs.versions.toml) matches the version in jdbi (currently 5.13.0-M2). native images build but don't work. This is only a "proof of concept, compiles" sketch.
pretty ping? |
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 needs to be documented
9f055c9
to
98e1058
Compare
Added documentation, rebased to the current main branch. |
@hgschmie thanks for your patience. Can you please also provide test that shows how these flags work? You can add a new profile on the existing samples, where you will enable your flags, and add new test in the corresponding functional test. Check functional tests here: https://github.com/graalvm/native-build-tools/tree/master/native-maven-plugin/src/functionalTest/groovy/org/graalvm/buildtools/maven Also, please add a changelog entry for this. |
Ok, will do tonight. |
c6c813c
to
2984155
Compare
Added functional tests and a changelog entry |
@dnestoro ready to be reviewed. |
|
||
Learn more about Native Image build configuration https://www.graalvm.org/reference-manual/native-image/overview/BuildConfiguration/[on the website]. | ||
|
||
==== Build, but not execute native tests |
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.
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.
Documentation review done! Thank you!
I have created a ticket to look at this file as a whole later down the line to go through streamlining this guide
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.
It would be useful to provide such guidance before asking for documentation. I am ok changing it, but I would like to know what the desired outcome is, otherwise it feels a bit like I am aiming at a moving target. I tried to follow the style in the maven-plugin.adoc file where there is a short, reference style explanation for each switch.
I shrank the text in the end-to-end guide down to a short paragraph and moved the bulk to the maven.adoc file in a new section "Controlling test execution".
.../functionalTest/groovy/org/graalvm/buildtools/maven/MavenTestExecutionFunctionalTests.groovy
Show resolved
Hide resolved
.../functionalTest/groovy/org/graalvm/buildtools/maven/MavenTestExecutionFunctionalTests.groovy
Outdated
Show resolved
Hide resolved
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.
Please address comments I left
|
||
Learn more about Native Image build configuration https://www.graalvm.org/reference-manual/native-image/overview/BuildConfiguration/[on the website]. | ||
|
||
==== Build, but not execute native tests |
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.
Documentation review done! Thank you!
I have created a ticket to look at this file as a whole later down the line to go through streamlining this guide
2984155
to
0f186fa
Compare
addressed the doc comments, moved docs around to reduce the size of the end-to-end guide text. |
anything more needed from me? I feel that I addressed all issues and this is good to be merged. This has been open for a very long time. |
Bit at a loss here. I think I addressed all the issues but you seem to have gone silent on me. @dnestoro @alvarosanchez @ban-mi ? |
@alvarosanchez @vjovanov can you advise whether this can be merged? @hgschmie seems to have addressed all requests by @dnestoro so we can probably merge it now? |
There are failing tests:
|
I will look into the failing test tonight; they pass for me locally and the CI did not work before. |
- skipTestExecution (boolean), default is false If true, create the native image for the tests but do not execute it. This allows the creation of the native images for testing even if they don't fully execute / tests fail. Otherwise, the build would stop at the first test failure. - failNoTests (boolean), default is true If true, fail building if no tests were found. In multi-module builds, there are often modules that have no tests (documentation) or tests are enabled/disabled by profiles (slow tests, e2e tests etc.). This switch allows for a concise configuraiton of the native plugin and then skipping modules where no tests were run without failing the build.
0f186fa
to
db5e3d9
Compare
@alvarosanchez @wirthi I fixed the failing test, however someone needs to approve the workflows so that the CI runs. |
If true, create the native image for the tests but do not execute it. This allows the creation of the native images for testing even if they don't fully execute / tests fail. Otherwise, the build would stop at the first test failure.
If true, fail building if no tests were found. In multi-module builds, there are often modules that have no tests (documentation) or tests are enabled/disabled by profiles (slow tests, e2e tests etc.). This switch allows for a concise configuraiton of the native plugin and then skipping modules where no tests were run without failing the build.