> On June 4, 2014, 7:20 p.m., Adam B wrote: > > src/tests/slave_tests.cpp, line 719 > > <https://reviews.apache.org/r/21968/diff/15/?file=602051#file602051line719> > > > > "this->" is unnecessary here and elsewhere in this function.
Yes, I agree. But in slave_recovery_tests.cpp and master_tests.cpp, "this->" is used there, but in gc_tests.cpp it is not used again, this gets me a little bit confused. Could you please tell me what is the baseline about whether or not to use "this->" in the tests? Or maybe just according to the "style" of the particular test file? Thanks in advance! > On June 4, 2014, 7:20 p.m., Adam B wrote: > > src/tests/slave_tests.cpp, line 767 > > <https://reviews.apache.org/r/21968/diff/15/?file=602051#file602051line767> > > > > Re: "when the master comes back" - The detector.appoint call doesn't > > actually failover the master, it just notifies the slave that there is a > > "new" master that it should reregister with. You can probably just cut that > > section out so the comment reads "Pause the clock here so the slave will > > not send multiple reregister messages before we change its state to > > TERMINATING." I see. So how about this: // Pause the clock here so after detecting a new master, // the slave will not send multiple reregister messages // before we change its state to TERMINATING - Yifan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21968/#review44741 ----------------------------------------------------------- On June 2, 2014, 11:02 p.m., Yifan Gu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21968/ > ----------------------------------------------------------- > > (Updated June 2, 2014, 11:02 p.m.) > > > Review request for mesos, Adam B, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-878 > https://issues.apache.org/jira/browse/MESOS-878 > > > Repository: mesos-git > > > Description > ------- > > Added a sentence to check the slave's state in doReliableRegistration() to > make sure that the function returns when the slave's state is TERMINATING. > Also, to write the test, I need to make sure there's no additional > doReliableRegistration() calls, so I added NO_FUTURE_PROTOBUFS() and > NO_FUTURE_MESSAGES() to make sure there's no more SlaveReregisteredMessages. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/gmock.hpp daba7e3 > src/slave/slave.cpp 3b10a4c > src/tests/mesos.hpp 9ece77a > src/tests/slave_tests.cpp 80fe3cf > > Diff: https://reviews.apache.org/r/21968/diff/ > > > Testing > ------- > > Slave_tests.cpp:TerminatingSlaveDoesNotReregister. > > > File Attachments > ---------------- > > Using DROP_PROTOBUF() > > https://reviews.apache.org/media/uploaded/files/2014/05/30/7c0e1b69-19c1-4868-865e-d47fbb739585__mesos.diff > > > Thanks, > > Yifan Gu > >
