-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Make Maven ITs less fragile #35996
Make Maven ITs less fragile #35996
Conversation
d7922d9
to
5465b32
Compare
This comment has been minimized.
This comment has been minimized.
5465b32
to
35fbd80
Compare
This comment has been minimized.
This comment has been minimized.
@aloubyansky Looks like your feedback was requested, but not explicitly.. HTH |
Unfortunately, the last commit is problematic as I used valid ascii characters. I'm not completely sure what's going on with this test as I saw it failing in some of our regular builds, even without this change. I'll revert the ascii one and see how CI goes. |
35fbd80
to
20d31aa
Compare
This comment has been minimized.
This comment has been minimized.
@@ -474,7 +474,10 @@ public void testThatSourceChangesAreDetectedOnPomChange() throws Exception { | |||
await() | |||
.pollDelay(100, TimeUnit.MILLISECONDS) | |||
.atMost(1, TimeUnit.MINUTES) | |||
.until(() -> devModeClient.getHttpResponse("/app/hello").contains("hello " + uuid)); | |||
.until(() -> { | |||
System.out.println(devModeClient.getHttpResponse("/app/hello")); |
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.
Did you mean to keep it a part of the commit?
20d31aa
to
a4b6044
Compare
This comment has been minimized.
This comment has been minimized.
a4b6044
to
2081179
Compare
This comment has been minimized.
This comment has been minimized.
2081179
to
cb8dffd
Compare
This comment has been minimized.
This comment has been minimized.
e3333db
to
e6be3ff
Compare
This comment has been minimized.
This comment has been minimized.
Enforcing versions of the plugins make sure we don't start downloading them when running the tests which can lead to timeouts. Also it makes the build more stable and actually tests the version we are using when creating new projects.
When running mvn compile quarkus:dev, compile is not executed when restarting dev mode after the pom change. Not specifying compile makes the compile goal to be run.
With maven-compiler-plugin 2.11.0, this test is not working anymore on Windows: Error while storing the mojo status: Input length = 1
We need to see the details of the errors when something is going wrong.
Installing the lib jar a second time leads to an access denied exception on Windows, probably because the jar is still used by the dev mode instance.
e6be3ff
to
5bec8af
Compare
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
Enforcing versions of the plugins make sure we don't start downloading
them when running the tests which can lead to timeouts.
Also it makes the build more stable and actually tests the version we
are using when creating new projects.
Created as draft as I'd like Alexey's feedback.
To reproduce:
mvn clean install
mvn clean install -f integration-tests/maven -Dtest=DevMojoIT#testThatSourceChangesAreDetectedOnPomChange
My understanding is that if
compile
is in the command line, we don't execute it ourselves but... when we are restarting dev mode due to a POM change, we should runcompile
again.