> 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. > > Thomas Marshall wrote: > 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.
Our assumption is that all cgroups tests have "ROOT_" and "CGROUPS_ " in their test name (though our logic here accepts them being in test case name too). So, I'm ok with what you did here. @BenM any more objections? else, i can commit this. - Vinod ----------------------------------------------------------- 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 > >
