----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51031/#review154359 -----------------------------------------------------------
src/tests/containerizer/cgroups_tests.cpp (line 184) <https://reviews.apache.org/r/51031/#comment223881> This would require all testing cgroups to be rooted under TEST_CGROUPS_ROOT (which would be clean) but some current tests just create cgroups with "mesos_test" as a prefix, like "mesos_test1". Your previous revision of this review with `startsWith` would work with such cases but this way wouldn't. Maybe we can fix the tests to not use patterns like "mesos_test1"? Of course any of these cleanups doesn't have to be in this review. src/tests/containerizer/cgroups_tests.cpp (lines 395 - 398) <https://reviews.apache.org/r/51031/#comment223880> Nit: So `cgroups1` and `cgroups2` are captured in variables but `path::join(TEST_CGROUPS_ROOT, "3"))` is not. It looks better if we call `string cgroup3 = path::join(TEST_CGROUPS_ROOT, "3");` and `string cgroup4 = path::join(cgroup3, "4");` (so the variable name matches the string literal. Then we can use variables consistently. Not having any such variable is fine too (so it's consistent too) because your are only repeating once and they are simple. - Jiang Yan Xu On Oct. 27, 2016, 10:13 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51031/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2016, 10:13 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and > Jiang Yan Xu. > > > Bugs: MESOS-6035 > https://issues.apache.org/jira/browse/MESOS-6035 > > > Repository: mesos > > > Description > ------- > > In some cases, we just want to get the children cgroups instead of > retrieve descendant cgroups recursively. We added an argument to > `cgroups::get` to indicate whether to retrieve cgroups recursively but > made recursive retrieve the default behaviour. This patch fixed some > incorrect `TEST_CGROUPS_ROOT` checks as well. > > > Diffs > ----- > > src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d > src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 > src/tests/containerizer/cgroups_tests.cpp > 0afaec6ae948cabf1472bf01103210d8f9809cb1 > > Diff: https://reviews.apache.org/r/51031/diff/ > > > Testing > ------- > > ``` > sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*" --verbose > ``` > > > Thanks, > > haosdent huang > >
