-
Notifications
You must be signed in to change notification settings - Fork 26
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
Search for Liberty modules to use in Run/Debug config #1216
Conversation
Signed-off-by: Paul Gooderham <[email protected]>
… to avoid NPE Signed-off-by: Paul Gooderham <[email protected]>
Signed-off-by: Paul Gooderham <[email protected]>
… for simplicity Signed-off-by: Paul Gooderham <[email protected]>
Signed-off-by: Paul Gooderham <[email protected]>
… return null Signed-off-by: Paul Gooderham <[email protected]>
Signed-off-by: Paul Gooderham <[email protected]>
I tried to simplify the nested lambdas in |
Signed-off-by: Paul Gooderham <[email protected]>
Investigating test failures. |
bcd5910
to
51fc882
Compare
Signed-off-by: Paul Gooderham <[email protected]>
Signed-off-by: Paul Gooderham <[email protected]>
Latest #5 Mac: MyList MavenSingleModMPProjectTest > testMultipleConfigEditHistory() FAILED at io.openliberty.tools.intellij.it.fixtures.ProjectFrameFixture.getMyList(ProjectFrameFixture.java:338) Windows: messages.log GradleSingleModMPProjectTest > testCustomStartParametersClearedOnConfigRemoval() FAILED at io.openliberty.tools.intellij.it.TestUtils.validateLibertyServerStopped(TestUtils.java:106) |
Signed-off-by: Paul Gooderham <[email protected]>
Since commit 35c3458 was added Linux passed and Windows failed with the same error as in the comment above ("Timed out waiting for message CWWKE0036I"). Mac: "Timed out waiting for message CWWKE0036I" and testMultipleConfigEditHistory(). |
Signed-off-by: Paul Gooderham <[email protected]>
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
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.
Thanks for the demo. Looks good. I had a few minor comments, but shouldn't stop you from merging now.
The fix for the bug is in
LibertyRunConfiguration
but to get that new method the logic needed to be extracted from the classLibertyExplorer
. See the comment in the issue.In the process of refactoring that method (
buildTree()
) I noticed that two loops did almost the same things for Gradle and Maven files so I combined them into one loop. A lot of code was removed. Also the module code that moved fromLibertyExplorer
toLibertyModules
was two loops and they were combined into one. I added data validation to the classBuildFile
mainly to document the acceptable values but also to ensure correctness.I noticed that some error statements in the test log did not indicate that the test was going to retry and this confused me so I took the opportunity to change the output string by adding the word "Retrying..." like the other tests.
Fixes #1202