-
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
Pass quarkus args to dev mode gradle task #31318
base: main
Are you sure you want to change the base?
Conversation
/cc @snazy |
This comment has been minimized.
This comment has been minimized.
@glefloch any chance you could have a look at that one soon? |
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.
Sure @gsmet.
Thanks @jacobdotcosta for this work. I just left a small comment.
commandOutputLogFile.close(); | ||
return false; | ||
} catch (Exception e) { | ||
System.out.println(String.format("e: <message, %s>, <cause, %s>", e.getMessage(), e.getCause())); |
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.
Could this be replace by proper logging?
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.
@jacobdotcosta any chance you could take care of this request? Thanks!
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.
Hi! Sure thing. Didn't realize you already requested it a couple of weeks ago.
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.
I've replaced the System out with logging but...
@gsmet checking the other tests they have no logs. Should I simply remove the log message? I thought it would be interesting to have the error cause but I have no problem with eliminating it so this test is similar to others!
5870205
to
1d0cee3
Compare
The errors seem to be :
I'm not sure the latter are related with the OOM. |
1d0cee3
to
a8d0c25
Compare
a8d0c25
to
7c48db2
Compare
This comment has been minimized.
This comment has been minimized.
I rebased to see if things would be better with all the latest changes. Our dev mode tests are leaking class loaders so that might just be the addition of a test that causes the issue, let's see how it goes. |
This comment has been minimized.
This comment has been minimized.
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✖ | Gradle Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
✔️ | Gradle Tests - JDK 17 Windows | Logs | Raw logs | 🔍 |
Full information is available in the Build summary check run.
You can consult the Develocity build scans.
Failures
⚙️ Gradle Tests - JDK 17 #
- Failing: integration-tests/gradle
📦 integration-tests/gradle
✖ io.quarkus.gradle.devmode.BasicJavaMainApplicationModuleDevModeTest.main
- History - More details - Source on GitHub
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.gradle.devmode.BasicJavaMainApplicationModuleDevModeTest was not fulfilled within 30 seconds.
at app//org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
at app//org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
at app//org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
at app//org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
at app//org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
at app//io.quarkus.gradle.devmode.BasicJavaMainApplicationModuleDevModeTest.testDevMode(BasicJavaMainApplicationModuleDevModeTest.java:36)
at app//io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.main(QuarkusDevGradleTestBase.java:60)
Tested the fix using the jacobdotcosta/quarkus-issue-28443 project.
Recreates PR #29250
Re-closes #28443