----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52718/#review152093 -----------------------------------------------------------
Thanks for introducing these test helpers Gilbert. These would be very useful for everyone. Most of my comments are around whether we can add these as gtest custom actions instead. Though, we can argue for having both, I would prefer having them as actions instead since most use-cases would be around just invoking the default actions (looking at how they are being used today in the present code). src/tests/mesos.hpp (lines 934 - 943) <https://reviews.apache.org/r/52718/#comment220829> I would prefer this to be an action to be consistent with the `LaunchTasks(...)` action we already have for the v0 API. The action should be able to take an `Option<Task>`/`Option<TaskGroupInfo` as arguments. A caller would then invoke it as: ```cpp EXPECT_CALL(*scheduler, offers(_, _)) .WillOnce(LaunchTasks(executorInfo, taskGroup)); ``` src/tests/mesos.hpp (lines 946 - 954) <https://reviews.apache.org/r/52718/#comment220826> I would prefer this to be an action. It would be then consistent with the `executor::SendSubscribe` action we already have. The usage would be like this: ```cpp EXPECT_CALL(*scheduler, connected(_)) .WillOnce(scheduler::SendSubscribe(frameworkInfo); ``` src/tests/mesos.hpp (lines 957 - 971) <https://reviews.apache.org/r/52718/#comment220831> We won't need this if we add an action `LaunchTasks` as per my earlier comment. src/tests/mesos.hpp (lines 974 - 990) <https://reviews.apache.org/r/52718/#comment220828> Most users won't _ever_ bother acknowledging their updates explicitly. In fact, we should think about even getting rid of the one liner they need to write. How about introducing a constructor argument to `TestMesos` to take `implicitAcknowledgements` as an argument? This would then be consistent with the old driver based interface. - Anand Mazumdar On Oct. 11, 2016, 1:15 a.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52718/ > ----------------------------------------------------------- > > (Updated Oct. 11, 2016, 1:15 a.m.) > > > Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Added v1 api helpers to create calls in 'tests/mesos.hpp'. > > > Diffs > ----- > > src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 > > Diff: https://reviews.apache.org/r/52718/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Gilbert Song > >
