----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50115/#review142608 -----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (lines 3325 - 3326) <https://reviews.apache.org/r/50115/#comment208231> Good fix, thanks! src/tests/hierarchical_allocator_tests.cpp (lines 3354 - 3355) <https://reviews.apache.org/r/50115/#comment208232> Replacing sleep with settle sounds good! Why not keep the 'finished' variable so that we can ensure the settle is doing what we expect? ``` // Wait for all the `addSlave` operations to be processed. Clock::settle(); ASSERT_EQ(slaveCount, offerCallbacks.load()); ``` Note that 'finished' is a bit vague, perhaps offerCallbacks is clearer? src/tests/hierarchical_allocator_tests.cpp (lines 3372 - 3373) <https://reviews.apache.org/r/50115/#comment208233> Ditto here: ``` ``` // Wait for all the `updateSlave` operations to be processed. Clock::settle(); ASSERT_EQ(slaveCount * 2, offerCallbacks.load()); ``` ``` - Benjamin Mahler On July 18, 2016, 3:32 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50115/ > ----------------------------------------------------------- > > (Updated July 18, 2016, 3:32 a.m.) > > > Review request for mesos, Benjamin Mahler, Jie Yu, Klaus Ma, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > Currently, in HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave, > we are using `sleep` to check if the operations are processed, this > is not accurate, we should use `Clock::settle()` instead. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > ce5da6be490b6fce311286eb4018c91eef55067e > > Diff: https://reviews.apache.org/r/50115/diff/ > > > Testing > ------- > > make > make check > > ``` > [==========] Running 1 test from 1 test case. > [----------] Global test environment set-up. > [----------] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/1 > Using 1000 agents and 50 frameworks > Added 50 frameworks in 14624us > Added 1000 agents in 2.253886secs > Updated 1000 agents in 1.940139secs > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/1 > (4493 ms) > [----------] 1 test from > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (4495 ms total) > > [----------] Global test environment tear-down > [==========] 1 test from 1 test case ran. (4520 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Guangya Liu > >
