> On July 2, 2012, 4:54 a.m., Benjamin Hindman wrote: > > src/tests/allocator_tests.cpp, line 685 > > <https://reviews.apache.org/r/5532/diff/4/?file=117150#file117150line685> > > > > Sometimes you do .WillRepeatedly(Return()) and sometimes you do > > .Times(AnyNumber()). Are there semantic differences that keep you from > > being consistent and just picking one?
The difference is that WillRepeatedly(Return()) won't do the default whereas Times(AnyNumber()) will. I'll change all of the Times(AnyNumber()) to WillRepeatedly(DoDefault()) to make this clearer. > On July 2, 2012, 4:54 a.m., Benjamin Hindman wrote: > > src/tests/allocator_tests.cpp, line 608 > > <https://reviews.apache.org/r/5532/diff/4/?file=117150#file117150line608> > > > > So, I think the right thing to do here is make this protected not > > public. In addition, I think you want to add a static void SetUpTestCase() > > here that just calls BaseZooKeeperTest::SetUpTestCase(), and make > > BaseZooKeeperTest::SetUpTestCase be protected again. Apparently SetUpTestCase must be public for TYPED_TESTs (like not putting DoDefaults in DoAlls, it's for "technical reasons"), but at the very least I can make BaseZooKeeperTest::SetUpTestCase protected by declaring a public MasterFailoverAllocatorTest::SetUpTestCase which calls it. > On July 2, 2012, 4:54 a.m., Benjamin Hindman wrote: > > src/tests/allocator_tests.cpp, line 393 > > <https://reviews.apache.org/r/5532/diff/4/?file=117150#file117150line393> > > > > Is this Return() necessary? As far as I can tell, the compiler doesn't recognize DoAll unless it contains at least one predefined gmock action, as opposed to custom actions we defined. I can't find any documentation on this, but I've asked the Google Mock Group about it, so I'll leave it in for now and fix it later if they get back to me with a solution. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5532/#review8779 ----------------------------------------------------------- On June 28, 2012, 10:19 p.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5532/ > ----------------------------------------------------------- > > (Updated June 28, 2012, 10:19 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Description > ------- > > Added tests for allocators. > > This patch depends on two other currently pending review requests: > https://reviews.apache.org/r/5448/ > https://reviews.apache.org/r/5451/ > > > This addresses bug MESOS-224. > https://issues.apache.org/jira/browse/MESOS-224 > > > Diffs > ----- > > src/Makefile.am 8760f59 > src/common/type_utils.hpp acf3e65 > src/tests/allocator_tests.cpp PRE-CREATION > src/tests/base_zookeeper_test.hpp 4613376 > src/tests/resource_offers_tests.cpp c1f1760 > src/tests/utils.hpp e81ec82 > src/tests/utils.cpp e7cda40 > > Diff: https://reviews.apache.org/r/5532/diff/ > > > Testing > ------- > > make check on Lion > > > Thanks, > > Thomas Marshall > >
