----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36380/#review91346 -----------------------------------------------------------
src/tests/perf_tests.cpp (line 52) <https://reviews.apache.org/r/36380/#comment144705> split args onto separate lines src/tests/perf_tests.cpp (line 62) <https://reviews.apache.org/r/36380/#comment144710> lines? it's 1 or more, how about just calling input? is debug() a better name for this function? src/tests/perf_tests.cpp (lines 65 - 69) <https://reviews.apache.org/r/36380/#comment144709> use a ternary if here? ```cpp s << (version.isError() ? "Error:" + version.error() : version.get()); ``` src/tests/perf_tests.cpp (line 104) <https://reviews.apache.org/r/36380/#comment144706> I don't think we use this, favoring ```cpp foreach (const tuple<Version, string>&, input) ``` Is Version hashable? If so, iterating over a Version -> input would be much cleaner ```cpp foreachpair(const Version& version, const string& input, inputs) ``` Or, just use the string version and parse the string version inside the loop? ```cpp hashmap<string, string> input { "2.6.39", "123,cycles\n0.123,task-clock" } ``` src/tests/perf_tests.cpp (line 157) <https://reviews.apache.org/r/36380/#comment144714> Do you need to keep all the logging of the context on failed expectations after the test has been correctly written? It clutters the code... src/tests/perf_tests.cpp (line 191) <https://reviews.apache.org/r/36380/#comment144711> ditto src/tests/perf_tests.cpp (line 226) <https://reviews.apache.org/r/36380/#comment144712> ditto - Ian Downes On July 10, 2015, 1:52 p.m., Paul Brett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36380/ > ----------------------------------------------------------- > > (Updated July 10, 2015, 1:52 p.m.) > > > Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: MESOS-2834 > https://issues.apache.org/jira/browse/MESOS-2834 > > > Repository: mesos > > > Description > ------- > > Test cases for performance monitor support of multiple output versions > depending on kernel version. > > > Diffs > ----- > > src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01 > > Diff: https://reviews.apache.org/r/36380/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Paul Brett > >
