----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34816/#review85795 -----------------------------------------------------------
src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137579> Why are we specializing it for the NoopResourceEstimator here? src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137581> Couldn't we drop 'Option<std::string>()' or replace with None()? src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137583> '{' on newline src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137582> kill newline src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137585> newline src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137577> Why unconst ref and rename? We tend to name, as Jie did :) src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137586> Do you still need this, if we can mock the oversubscribable call? src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137589> You should be able to get rid of this, if we rely on mocking the oversubscribable call src/tests/mesos.hpp <https://reviews.apache.org/r/34816/#comment137588> Why the 'RE' prefix? Was there a conflict? src/tests/oversubscription_tests.cpp <https://reviews.apache.org/r/34816/#comment137592> My thought on mocking, was that we could override the oversubscribable() call and have the test own the queue. @Vinod - what do you think? - Niklas Nielsen On May 29, 2015, 11:18 a.m., Bartek Plotka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34816/ > ----------------------------------------------------------- > > (Updated May 29, 2015, 11:18 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 b8f7a2f9236166e42421d926718af8d45e857eba > src/tests/oversubscription_tests.cpp > 75c25b04c1e6a8e0e7e8fd55440743fe1699af88 > > Diff: https://reviews.apache.org/r/34816/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Bartek Plotka > >
