-
Notifications
You must be signed in to change notification settings - Fork 3k
CI - Isolate tests from .mvn/maven.config #46525
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
Conversation
My, I did a gazillion of tests locally and it's still going weird :/. Pushed another shot at it... |
Status for workflow
|
I'm so happy to see this PR, because I've been driving myself mad trying to figure out how |
I wonder if it would be more robust to copy the maven wrapper at some point so we don't have two copies of it to maintain? But that could be a future enhancement. These changes look sensible, but I also wonder if @aloubyansky might have opinions, since he always points out I've misunderstood resource filtering and am doing it wrong. |
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. Will also cherry-pick this commit to my branch and report back.
Follow-up of quarkusio#46525 and also related to quarkusio#48506 We need the Maven wrapper to be present in some modules to avoid having the nested Maven runs inherit from the root .mvn/maven.config. But it's better to copy it in the build when needed. Per gripe from @aloubyansky.
To workaround command line size limit on Windows, we have to use
.mvn/maven.config
which is sometimes propagated to our tests.These changes isolate some of our tests from the root
.mvn/maven.config
.The whole idea is to have a Maven wrapper in the test directory to avoid bubbling up to the root.
In Maven 4, we should be able to use this new feature just introduced by @cstamas to avoid the problem, but for now that's how it is.
This will hopefully fix the remaining CI issues (until more are discovered...).