----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31012/#review79387 -----------------------------------------------------------
src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/31012/#comment128677> Where does the number 15 come from? src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/31012/#comment128684> s/statistics/statistic It just returns a single statistic, right? src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/31012/#comment128680> Why is this a member variable: it's not used internally by the class. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/31012/#comment128678> Can you document in the comment what this test does (and for subsequent tests), e.g., something like: // Create a cgroup with a memory limit of 64 MB, fork a helper process in the cgroup then instruct it to page in an additonal 15 MB of resident memory. We expect the cgroup to have between 15 and 16 MB of memory where the ~ 1 MB accounts for the process's other pages. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/31012/#comment128682> Does the test fixture ensure this process is killed? src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/31012/#comment128681> No need for a named variable: ```cpp EXPECT_LT(statistics("rss") - defaultAllocation.bytes(), Megabytes(1)); ``` - Ian Downes On April 1, 2015, 4:13 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31012/ > ----------------------------------------------------------- > > (Updated April 1, 2015, 4:13 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: mesos-2572 > https://issues.apache.org/jira/browse/mesos-2572 > > > Repository: mesos > > > Description > ------- > > Added memory statisics test fixure and a test for RSS. > > > Diffs > ----- > > src/tests/cgroups_tests.cpp e18aed1feca182da89a117f81bed0897a00fb0ef > > Diff: https://reviews.apache.org/r/31012/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
