-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46229/#review157656
-----------------------------------------------------------




src/tests/containerizer/linux_filesystem_isolator_tests.cpp (line 1531)
<https://reviews.apache.org/r/46229/#comment228301>

    Nit:
    s/use (and assume the presence) of/use (and assume the presence of)/



src/tests/persistent_volume_tests.cpp (lines 554 - 559)
<https://reviews.apache.org/r/46229/#comment228295>

    I think that providing some validation of the nobody user is a good idea, 
but I wonder if this is the best way to accomplish it (and in the test below as 
well). It looks like this block also appears in 
`SlaveTest.DISABLED_ROOT_RunTaskWithCommandInfoWithUser`, but other tests which 
make use of 'nobody' (`SlaveTest.ROOT_LaunchTaskInfoWithContainerInfo`, for 
example) do not have such a check.
    
    We could instead add a new filter to the test filters already present in 
`src/tests/environment.cpp` which would filter out all tests which make use of 
the `nobody` user when it's not present.
    
    I would advocate a more general solution like that, I think. If you opt for 
such an approach, you could do it as part of this work, or just place a TODO 
here which points to a Mesos JIRA for that task.
    
    Thoughts?


- Greg Mann


On Nov. 20, 2016, 4:51 p.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46229/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
>     https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add unit tests for adding a user for persistent volumes.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 24d1a6971d412aaf7bdc20efcc1197b120de51ce 
>   src/tests/hierarchical_allocator_tests.cpp 
> edd5cea8996d7c616cf9428f0f1c05d70c37c307 
>   src/tests/master_validation_tests.cpp 
> f893067859425967654401f3226149268b51cf57 
>   src/tests/mesos.hpp f94882f44e7fd35f6e9aaa381656af5c5a58ff9e 
>   src/tests/persistent_volume_tests.cpp 
> 8651b2c5455089041f16d091c90a7e0d948191b8 
>   src/tests/resources_tests.cpp d6eb7787bac58c1133a4bab0fc17df49117fed87 
>   src/tests/sorter_tests.cpp 9f48abe65c011acfaf79f3d0d79f1d032fd6a4af 
> 
> Diff: https://reviews.apache.org/r/46229/diff/
> 
> 
> Testing
> -------
> 
> All tests passed (including the newly added tests).
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Reply via email to