----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37466/#review97551 -----------------------------------------------------------
Thinking about this further, the test parameteratization here seems to be a bit of a mess, could we instead just create lists of valid / invalid inputs within a test and loop over them? I don't think the test parameterization here is worth the complexity. src/tests/containerizer/perf_tests.cpp (line 51) <https://reviews.apache.org/r/37466/#comment153537> Why not just Events? This is testing both valid and invalid events. Also, can you pull this out into a separate patch? src/tests/containerizer/perf_tests.cpp (line 67) <https://reviews.apache.org/r/37466/#comment153545> Why is this called ParseTypes and the one below called ParseCgroups? They both seem to parse cgroup-based perf output, so I'm a bit confused. src/tests/containerizer/perf_tests.cpp (lines 85 - 100) <https://reviews.apache.org/r/37466/#comment153541> Please instantiate right below the class as it makes it clearer when there are many tests under the test fixture (it's also what we do in all the other tests). Ditto for the rest of these. src/tests/containerizer/perf_tests.cpp (lines 223 - 233) <https://reviews.apache.org/r/37466/#comment153544> What's with the inconsistent newlines here? src/tests/containerizer/perf_tests.cpp (line 253) <https://reviews.apache.org/r/37466/#comment153543> I think you meant to split the string literal on the newline like you did in most cases, can you do a sweep? - Ben Mahler On Sept. 2, 2015, 9:13 p.m., Paul Brett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37466/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2015, 9:13 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-2834 > https://issues.apache.org/jira/browse/MESOS-2834 > > > Repository: mesos > > > Description > ------- > > Update perf tests to including testing the supported perf output formats. > > > Diffs > ----- > > src/tests/containerizer/perf_tests.cpp > 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 > > Diff: https://reviews.apache.org/r/37466/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Paul Brett > >
