> On Sept. 11, 2016, 2:22 p.m., Qian Zhang wrote: > > src/tests/containerizer/cgroups_isolator_tests.cpp, line 91 > > <https://reviews.apache.org/r/51783/diff/2/?file=1495554#file1495554line91> > > > > The test name `ROOT_CGROUPS_PERF_NET_CLS_UserCgroup` seems a bit weird > > to me, if we follow this way, we may need to name it as > > `ROOT_CGROUPS_CPU_DEVICES_MEM_PERF_NET_CLS_UserCgroup` since `cgroups/cpu`, > > `cgroups/devices` and `cgroups/mem` are also enabled in this case, that is > > obviouly a too long test name :-) > > > > So I am thinking maybe we just name it as `ROOT_CGROUPS_UserCgroup`, > > and in https://reviews.apache.org/r/51781, we can still do the filter based > > on test fixture name rather than test name, like this: > > ``` > > if (matches(test, "NetClsIsolatorTest") || > > matches(test, "UserCgroupsIsolatorTest") { > > return netClsError; > > } > > ```
I think `NET_CLS_` looks more clear since we don't need update `environment.cpp` every time when we add a test cases depend on `net_cls` subsystem. Another benefit is we could find the test case depends on `net_cls` by its name. If we don't use `NET_CLS_`, we are not aware of `UserCgroupsIsolatorTest` depends on `net_cls` cgroup subsystem. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51783/#review148420 ----------------------------------------------------------- On Sept. 11, 2016, 4:01 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51783/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2016, 4:01 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang. > > > Repository: mesos > > > Description > ------- > > Refactored `UserCgroupsIsolatorTest`. > > > Diffs > ----- > > src/tests/containerizer/cgroups_isolator_tests.cpp > c4e467c8227f9e4129b05d173812592f39a04e06 > > Diff: https://reviews.apache.org/r/51783/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
