> On Aug. 11, 2014, 10:56 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, line 1254 > > <https://reviews.apache.org/r/22313/diff/24/?file=653349#file653349line1254> > > > > Why are you failing the promise here? Don't you want to set it to > > Nothing and then make sure that task doesn't launch? Otherwise, what you > > are testing is "containerizer update failure" path and not > > "executor/framework removed" path. > > Yifan Gu wrote: > Umm, I can return Nothing here, but I think since the "executor/framework > removed" path is visited first, so either will test this path.
that is true, but what if someone in the future comes along and changes the order of the if statements in __runTask()? > On Aug. 11, 2014, 10:56 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, line 1314 > > <https://reviews.apache.org/r/22313/diff/24/?file=653349#file653349line1314> > > > > Kill this? Why would there be subsequent updates here? > > Yifan Gu wrote: > I remember that last time I ignored the subsequent updates, it will cause > the tests to fail. > https://issues.apache.org/jira/browse/MESOS-1460 > > But I have run these tests for more than 3000 times without > .WillRepeatedly(Return()); They haven't failed yet... maybe it failed earlier because you didn't have the .WillRepeatedly() expectation below where you set the expectation for status2. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22313/#review50239 ----------------------------------------------------------- On Aug. 12, 2014, 11:07 p.m., Yifan Gu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22313/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2014, 11:07 p.m.) > > > Review request for mesos, Ian Downes and Vinod Kone. > > > Bugs: MESOS-886 > https://issues.apache.org/jira/browse/MESOS-886 > > > Repository: mesos-git > > > Description > ------- > > Added __runTask() to wait for the completion of containerizer->update() and > check the result before sending RunTaskMessage. > > > Diffs > ----- > > src/slave/slave.hpp 9d4607e > src/slave/slave.cpp 787bd05 > src/tests/slave_tests.cpp 69be28f > > Diff: https://reviews.apache.org/r/22313/diff/ > > > Testing > ------- > > SlaveTest.WillNotLaunchTaskBeforeContainerizerUpdate > SlaveTest.WillNotLaunchTaskIfFrameworkIsRemoved > SlaveTest.LaunchTaskAfterContainerizerUpdate > > ./bin/mesos-tests.sh --gtest_repeat=-1 --gtest_break_on_failure > --gtest_filter=*LaunchTask* > > successful times > 2000 > > make check > > > File Attachments > ---------------- > > framework will exit > > https://reviews.apache.org/media/uploaded/files/2014/06/18/fbe73273-7aa9-4faa-b1c5-003ab03042a9__issue-886.diff > log > > https://reviews.apache.org/media/uploaded/files/2014/06/18/84d801a0-5c2a-4bb9-901b-e1962031461c__log > > > Thanks, > > Yifan Gu > >