> On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote: > > src/tests/allocator_tests.cpp, line 715 > > <https://reviews.apache.org/r/5532/diff/1/?file=115993#file115993line715> > > > > DoDefault?
Unfortunately, DoDefault can't be used in a DoAll, but I added a comment to make this clearer > On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote: > > src/tests/base_zookeeper_test.hpp, line 89 > > <https://reviews.apache.org/r/5532/diff/1/?file=115994#file115994line89> > > > > Was SetUpTestCase not protected? This was necessary to be able to extend BaseZooKeeperTest for MasterFailoverAllocatorTest. I agree that taking something that was protected and making it public isn't ideal, but I don't know how else to solve this problem in C++. > On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote: > > src/tests/allocator_tests.cpp, line 160 > > <https://reviews.apache.org/r/5532/diff/1/?file=115993#file115993line160> > > > > If the 'this->' isn't actually necessary we should remove it (here and > > everywhere else). I agree that it doesn't seem like this should be necessary, but it doesn't compile without it (I guess because of the way the TYPED_TEST macro expands). > On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote: > > src/tests/allocator_tests.cpp, line 288 > > <https://reviews.apache.org/r/5532/diff/1/?file=115993#file115993line288> > > > > Why do you need to invoke this directly? A comment here (and anywhere > > else you're doing something like this) would be very useful. Also, this > > isn't "thread" safe, you'll need to dispatch here if you actually need to > > do this. I was invoking them directly in order to ensure that they're deterministically called "out of order", i.e. a framework is removed and then some of its resources are recovered, but I can do the same thing with triggers and dispatches. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5532/#review8549 ----------------------------------------------------------- On June 26, 2012, 7:01 p.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5532/ > ----------------------------------------------------------- > > (Updated June 26, 2012, 7:01 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/ > > > Diffs > ----- > > src/Makefile.am 8760f59 > 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 > >
