> On Oct. 10, 2016, 7:56 p.m., Anand Mazumdar wrote: > > 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).
Thanks for thorough comments, Anand! Personally I dont really like the way to have custom actions for send out `calls`. I would still insist on having both for the following reasons: 1. Explicitly doing `mesos->send()` is more code-readable for other people to understand exactly what is happening in our unit tests, vs, implicitly send out a call under the hood and rely on a mock class to expect a call. 2. Both ways would only contain two lines of codes. We should allow whether to use a gtest custom action or explicitly send a call. 3. Currently the unit tests using v0 api in our code base, use both ways, and we have a bunch of tests doing `driver.launchTasks()` explicitly. > On Oct. 10, 2016, 7:56 p.m., Anand Mazumdar wrote: > > src/tests/mesos.hpp, lines 934-943 > > <https://reviews.apache.org/r/52718/diff/1/?file=1530159#file1530159line934> > > > > 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)); > > ``` Binding `LaunchTasks` with a custom action is fine, but we should also allow doning it explicitly. That is what we are currently doing. We can grab a lot of them by `driver.launchTasks(offer.id(), {task});`. - Gilbert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52718/#review152093 ----------------------------------------------------------- On Oct. 10, 2016, 6:15 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52718/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2016, 6:15 p.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 > >
