> On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote: > > src/tests/reservation_tests.cpp, line 213 > > <https://reviews.apache.org/r/40731/diff/2/?file=1150126#file1150126line213> > > > > Is this comment needed ?
I think this a copy-paste issue. This must have come from the previous test case (TEST_F(ReservationTest, ReserveThenUnreserve)) that was copied over. Will remove it. > On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote: > > src/tests/reservation_tests.cpp, line 231 > > <https://reviews.apache.org/r/40731/diff/2/?file=1150126#file1150126line231> > > > > Can we just do: > > > > `EXPECT_CALL(sched, resourceOffers(&driver, _)) > > .WillOnce(FutureArg<1>(&offers));` > > > > This is more in sync with the style followed in the rest of the tests. > > Can we modify the other ocurrences in this test too ? The above exceeds the 80 column limit. I was following the style guide here: http://mesos.apache.org/documentation/latest/c++-style-guide/ The "Function Definition/Invocation" section wants us to follow styles 1,4, 5 and sometimes 3 and 'never' 2 hence the change? > On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote: > > src/tests/reservation_tests.cpp, line 266 > > <https://reviews.apache.org/r/40731/diff/2/?file=1150126#file1150126line266> > > > > We generally avoid redundant/self-explanatory comments. Is this really > > needed at all ? What is it trying to convey ? The problem is that the check failure would lead to a crash in the agent, and wouldn't show up in the mockscheduler. Not sure if we can re-write the test case somehow to reflect it. Hence added an explicit comment to explain the behavior to the reader. > On Dec. 2, 2015, 8:02 p.m., Anand Mazumdar wrote: > > src/tests/reservation_tests.cpp, line 273 > > <https://reviews.apache.org/r/40731/diff/2/?file=1150126#file1150126line273> > > > > Not yours : Can we insert an additional line here. We leave 2 lines > > after each global function/test. ok - Avinash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40731/#review108704 ----------------------------------------------------------- On Dec. 2, 2015, 8:27 a.m., Avinash sridharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40731/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2015, 8:27 a.m.) > > > Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil > Conway. > > > Bugs: MESOS-3552 > https://issues.apache.org/jira/browse/MESOS-3552 > > > Repository: mesos > > > Description > ------- > > Adding the test framework submitted by Mandeep (@mchadha) > https://reviews.apache.org/r/39056/ > > > Diffs > ----- > > src/tests/reservation_tests.cpp e7f14079e766ce0a8bad2da646776347e4a17169 > > Diff: https://reviews.apache.org/r/40731/diff/ > > > Testing > ------- > > Ran make check after adding Mandeep's test case to the GTEST framework. > > > Thanks, > > Avinash sridharan > >
