> On June 1, 2015, 12:34 p.m., Jie Yu wrote: > > src/tests/oversubscription_tests.cpp, line 96 > > <https://reviews.apache.org/r/34816/diff/4/?file=975492#file975492line96> > > > > This looks weired to me since we are actually creating a > > TestResourceEstimator, but TypeParam == NoopResourceEstimator. > > > > IMO, all the tests so far in this file does not work for an arbitrary > > resource estimator (i.e., the expectations are tied with the estimator). As > > a result, using a typed test doesn't seem to make sense to me. > > > > Thoughts? > > Bartek Plotka wrote: > That is a good point! > > I agree that we do not test NoopResourceEstimator but the plumbing right > now. However, is there anything bad in testing both module RE and normal RE > in such typed test? > In fact these typed tests are going to be necessary when other > ResourceEstimators will be implemented.
One thought; We need two different things: 1.a) Test the default behavior of a non-oversubscribe scenario i.e. NoopEstimator 1.b) Test modules work (same as 1.a) but using Module typed test 2.) Using the mock tests to control the estimates in a fine grained fashion (MockEstimator). In other words, can we separate the two? - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34816/#review86051 ----------------------------------------------------------- On May 31, 2015, 10:37 p.m., Bartek Plotka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34816/ > ----------------------------------------------------------- > > (Updated May 31, 2015, 10:37 p.m.) > > > Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod > Kone. > > > Repository: mesos > > > Description > ------- > > This patch prepares Resource Estimator tests for modularized > ResourceEstimator. > > It includes: > 1. Made oversubscription_test to be typed_test. > 2. TestResourceEstimator changed to be a templated MockResourceEstimator with > mocked methods - to inject resources and test plumbing between RE and Slave. > > > Diffs > ----- > > src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b > src/tests/oversubscription_tests.cpp > ea5857cb579aa904fd05530684bdde51a0b3f27f > > Diff: https://reviews.apache.org/r/34816/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Bartek Plotka > >
