> On June 1, 2015, 7: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?
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. - Bartek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34816/#review86051 ----------------------------------------------------------- On June 1, 2015, 5:37 a.m., Bartek Plotka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34816/ > ----------------------------------------------------------- > > (Updated June 1, 2015, 5:37 a.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 > >
