----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45190/#review128184 -----------------------------------------------------------
Fix it, then Ship it! Thanks for cleaning up the code and fixing the long-pending bug! samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocator.java (line 40) <https://reviews.apache.org/r/45190/#comment191566> Why should config be static? samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java (line 29) <https://reviews.apache.org/r/45190/#comment191573> nit: Unused variable samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java (line 65) <https://reviews.apache.org/r/45190/#comment191558> Looks like container is no longer being used samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java (line 126) <https://reviews.apache.org/r/45190/#comment191561> "satisfied" flag here is simply indicating that we have completed running the postConditionAssertions, right? samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java (line 136) <https://reviews.apache.org/r/45190/#comment191562> method not being used. samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerRequestState.java (line 91) <https://reviews.apache.org/r/45190/#comment191570> unused method - Navina Ramesh On April 8, 2016, 11:46 p.m., Jake Maes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45190/ > ----------------------------------------------------------- > > (Updated April 8, 2016, 11:46 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > SAMZA-910 Fix expired request test in HostAwareContainerAllocator > > Summary of changes: > 1. Remove the last sleep() from HostAwareContainerAllocator > 2. Fix a silent failure in testRerequestOnAnyHostIfContainerStartFails by > setting the neededContainers to 1 before running the test. > 3. Update MockContainerListener so assertion failures in other threads are > thrown in the main thread to fail the test. (no silent failures) This should > help troubleshoot the tests if there are any remaining issues. > 4. Rename obscure hostnames to make it easier to reason about the tests. > > > Diffs > ----- > > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocator.java > b253f98f7258bb611e1ad6672f74b07ab7e20b70 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocatorCommon.java > PRE-CREATION > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestHostAwareContainerAllocator.java > 93e430b6ee986b06ecdac4979552d774724a1fbd > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerListener.java > cb82cccf75b54cfbefd586700e8283cb41173833 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerRequestState.java > 879a7d0d06b087cfe0417f3fa5801b43ac7fc458 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerUtil.java > 2f9669f8b7e77abb65b244ccd067ae7ab1f245c3 > > Diff: https://reviews.apache.org/r/45190/diff/ > > > Testing > ------- > > Ran build and check-all on both of my machines twice. I don't see any > sporadic failures anymore. > > > Thanks, > > Jake Maes > >
