----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10552/#review19273 -----------------------------------------------------------
src/tests/allocator_tests.cpp <https://reviews.apache.org/r/10552/#comment39914> This test is very complex. Would you expand on this comment so we can understand whats happening? src/tests/allocator_tests.cpp <https://reviews.apache.org/r/10552/#comment39915> Any specific reason you followed this pattern here and everywhere else? Would it be sufficient to just EXPECT_CALL(slaveRemoved).Atmost(1) and clusters.shutdown() just curious? src/tests/allocator_tests.cpp <https://reviews.apache.org/r/10552/#comment39917> Can you replace all AWAIT_UNTIL's with AWAIT_READY's? The latter checks that the future is Ready(). - Vinod Kone On April 16, 2013, 6:05 p.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10552/ > ----------------------------------------------------------- > > (Updated April 16, 2013, 6:05 p.m.) > > > Review request for mesos, Benjamin Hindman, Vinod Kone, and Ben Mahler. > > > Description > ------- > > Update allocator tests to use cluster abstraction. Also includes fixes for a > few bugs that were uncovered: > > - We now create separate isolators and executors for each slave in tests with > multiple slaves. > - Tests that launched a task and then immediately launched another framework > without waiting for the task were seg faulting occasionally on Ubuntu when > TestingIsolator::launchExecutor was modifying the environment while > MesosSchedulerDriver::MesosSchedulerDriver was trying to read the > environment. We now wait for the task to finish launching before starting the > second framework in these tests. > - Some tests have been significantly sped up by setting the > allocation_interval flag for the master from the default 1s to 50ms. > - The MockAllocator was reintroduced into the DRFAllocatorTest because it > turns out that its necessary to ensure that the frameworks and slaves are > added to the allocator in the correct order. > - The AllocatorZookeeperTests were occasionally failing on waiting for the > first allocation because it took longer than the 2s that AWAIT_UNTIL waits. I > added another AWAIT_UNTIL on a slightly earlier event to break up this wait > and ensure its always under 2s. > > I also added another Cluster::Masters::start that takes both an > AllocatorProcess and master::Flags. > > > Diffs > ----- > > src/tests/allocator_tests.cpp 85a4981 > src/tests/allocator_zookeeper_tests.cpp 82a498e > src/tests/cluster.hpp 28fc269 > > Diff: https://reviews.apache.org/r/10552/diff/ > > > Testing > ------- > > bin/mesos-tests.sh --gtest_break_on_failure --gtest_repeat=3000 > --gtest_filter=*Allocator* > > > Thanks, > > Thomas Marshall > >
