-
Notifications
You must be signed in to change notification settings - Fork 81
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
[SHRINKRES-351] enh: using maven 4 project-local dependency resolution, if available #341
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,7 @@ void testDaemonShouldWaitForBuildSuccess() throws TimeoutException { | |
.withWaitUntilOutputLineMatches(".*BUILD SUCCESS.*") | ||
.build(); | ||
|
||
Awaitility.await("Wait till thread is not be alive").atMost(20, TimeUnit.SECONDS) | ||
Awaitility.await("Wait till thread is not be alive").atMost(45, TimeUnit.SECONDS) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why increasing the timeout? Did the changes in the PR resulted in longer run? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, I noticed flaky test in CI. Has nothing to do with the changes in this PR, just trying to fix flakiness as I see it :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me. Can I ask for a separate commit here as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I will do that. thanks |
||
.until(() -> !daemonBuild.isAlive()); | ||
|
||
Assertions.assertNotNull(daemonBuild.getBuiltProject()); | ||
|
@@ -60,7 +60,7 @@ void testDaemonWithoutWaitShouldNotReachTheEndOfTheBuild() { | |
Assertions.assertTrue(daemonBuild.isAlive()); | ||
Assertions.assertNull(daemonBuild.getBuiltProject()); | ||
|
||
Awaitility.await("Wait till thread is not be alive").atMost(20, TimeUnit.SECONDS) | ||
Awaitility.await("Wait till thread is not be alive").atMost(45, TimeUnit.SECONDS) | ||
.until(() -> !daemonBuild.isAlive()); | ||
|
||
Assertions.assertFalse(daemonBuild.isAlive()); | ||
|
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.
Even though this is just a simple print, I wouldn't make printing the line dependent on deleting the file. Especially if the file is not related to the actual extraction
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 is because I was trying to fix flaky test that relies on this message.
Failing to delete marker file indicates a failure in the code, at least that's how I am reading this,
so actual functionality should be correct.
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.
Got it, found the test. Must admit, not my favorite way of checking whether the file was extracted, but I can understand this change.
Can I ask you to put this in a separate commit? As this isn't related to the project-local dependencies I'd prefer git blame to show a related commit when it comes to checking history of this change
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.
yes