> On April 16, 2013, 1:11 a.m., Ben Mahler wrote: > > src/tests/environment.cpp, line 71 > > <https://reviews.apache.org/r/10531/diff/1/?file=281408#file281408line71> > > > > Shouldn't this be comparing against "CgroupsNoHierarchyTest"? It should > > probably also ensure "ROOT_" and "CGROUPS_" are present, no? > > > > That would obviate the need for you to verify the root user.
Leaving off "Test" makes the logic here more flexible, since you could define something like "CgroupsNoHierarchyXXXXTest" and still get the filtering, but I think that a cleaner way to deal with this would be to add a "CGROUPSNOHIERARCHY_" flag, since this is more consistent. Checking for "ROOT_" and "CGROUPS_" gets messy since its possible that the flags won't appear together (i.e. some in the test name, some in the test case name). So, if "ROOT_" appears in the name and "NOHIERARCHY" in the case name, "NOHIERARCHY" will be checked first and getting the current hierarchies will fail if the user isn't root instead of having the check for "ROOT_" return false. I think that the best solution to this is to make "NOHIERARCHY" also check for root and cgroups support, since there's no situation in which "NOHIERARCHY" would be present that root and cgroups support wouldn't also be necessary. This means that the "ROOT_" and "CGROUPS_" flags aren't necessary if "NOHIERARCHY" is present, though of course there's no problem with including them. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10531/#review19237 ----------------------------------------------------------- On April 16, 2013, 12:32 a.m., Thomas Marshall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10531/ > ----------------------------------------------------------- > > (Updated April 16, 2013, 12:32 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > I know that I'm not assigned to this issue, but its causing about 75% of the > test failures that we're seeing in the AMP lab's Jenkins now, so I figured I > would tackle it: > > > This addresses bug MESOS-414. > https://issues.apache.org/jira/browse/MESOS-414 > > > Diffs > ----- > > src/tests/environment.cpp 3096e5a > > Diff: https://reviews.apache.org/r/10531/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Thomas Marshall > >
