----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7622/#review12489 -----------------------------------------------------------
That's awesome! src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26582> One thing I am thinking is: in cgroups test, it's highly likely that a second test will fail during setup if the first test fails due to whatever reasons. For example, there are some processes left in a cgroup due to the failure of the first test, causing the second test (and all following tests) to fail during setup. Maybe what we can do is to declare a static flag in class CgroupsTest and set this flag if a test fails so that later tests will fail gracefully (have a print saying that it's due to the failure of the previous test), instead of a random failure. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26592> Compilable? ASSERT_SOME does not return an ostream. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26594> It is possible that here "subsystem" is empty. The createHierarchy will fail when subsystem is empty. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26598> we should add one more check here: ASSERT_EQ(2UL, cgroups.get().size()); src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26597> "mesos_test" should not be returned here. src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26599> Why expect cgroups to be empty here? src/tests/cgroups_tests.cpp <https://reviews.apache.org/r/7622/#comment26602> This should be a CgroupsAnyHierarchyWithCpuMemoryTest - Jie Yu On Oct. 16, 2012, 7:24 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7622/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2012, 7:24 p.m.) > > > Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler. > > > Description > ------- > > * Added new test fixtures: > (1) CgroupsNoHierarchyTest for running tests where we need to create > a hierarchy (and thus, most likely, no existing hierarchy can > exist because it will already have the cpu and or memory > subsystem attached). > (2) CgroupsAnyHierarchyTest and subclasses > CgroupsAnyHierarchyWithCpuMemoryTest and > CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests > with any hierarchy provided it has necessary subsystems > attached. > > * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and > removed nested cgroups by default. The rename was done because we > might run tests inside of an existing hierarchy and we want to avoid > name clashes. The nested cgroups were removed in favor of a test > that explicitly tries to create nested cgroups (since some older > kernels with particular subsystems attached have a hard time with > this, and we'd like to detect that case explicitly). > > * Created an explicit test for nested cgroups (see above). > > * Updated the "write control" test to use a forked process rather than > the test process (to be more conservative in the presence of errors) > > * Updated the "listen event" (i.e., "oom") test to check for the > proper control first (memory.oom_control). > > * Updated the failure mechanism of forked (children) processes to use > 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test > output more readable upon failures. > > * Updated the notify mechanism from forked (children) processes to > parent processes to correctly distinguish a closed pipe from a value > written (to catch more instances of when the test is actually > failing). > > > Diffs > ----- > > src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724 > > Diff: https://reviews.apache.org/r/7622/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
