----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67291/#review203793 -----------------------------------------------------------
It seems to me that most of the modified tests work just fine with `nobody`. For example, the `FetcherTest` and `CapabilitiesTest` don't need an unpriviledged user that can access test launcher's home. Are we advocating a new pattern here and want to avoid `nobody` for all tests? src/tests/container_logger_tests.cpp Line 747 (original), 751 (patched) <https://reviews.apache.org/r/67291/#comment286100> s/"nobody"/$SUDO_USER/ src/tests/containerizer/docker_containerizer_tests.cpp Lines 4200 (patched) <https://reviews.apache.org/r/67291/#comment286099> Not yours, but how about renaming this test to `ROOT_DOCKER_UNPRIVILEGED_USER_NonRootSandbok`? src/tests/environment.cpp Lines 871 (patched) <https://reviews.apache.org/r/67291/#comment286090> Should we also test that `$SUDO_USER` is not `root`? src/tests/environment.cpp Lines 875 (patched) <https://reviews.apache.org/r/67291/#comment286111> (From Greg) How about `No usable unpriviledged user found from the 'SUDO_USER' environment variable.` so one can run the tests as `root` and populate it with a non-root user manually if they want to? src/tests/environment.cpp Lines 885 (patched) <https://reviews.apache.org/r/67291/#comment286091> Not a real issue here, but I'm thinking if just `UNPRIVILEDGED_` is clear enough. `UNPRIVILEDGED_USER_` is definitely more elaborative, but it's longer and contains a `_` which we also use as keyword deliminators for test filters. Although we already have examples like `NVEDIA_GPU_`, I feel that avoiding `_` in the matching string would also make it easier to see what filters are applyed on a test. src/tests/slave_tests.cpp Line 1121 (original), 1072 (patched) <https://reviews.apache.org/r/67291/#comment286114> ``` ../src/tests/slave_tests.cpp:1072:26: warning: adding 'unsigned int' to a string does not append to the string [-Wstring-plus-int] "test `id -u` == " + uid.get()); ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~ ``` - Chun-Hung Hsiao On May 24, 2018, 5:49 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67291/ > ----------------------------------------------------------- > > (Updated May 24, 2018, 5:49 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer and James Peach. > > > Repository: mesos > > > Description > ------- > > Used `$SUDO_USER` instead because `nobody` sometimes cannot access > direcotries under `$HOME` of the current user running the tests. > > > Diffs > ----- > > src/tests/container_logger_tests.cpp > 7ec6f8701ecbc8f43ef41056fcbc17566c7db416 > src/tests/containerizer/capabilities_tests.cpp > 734aa21c5bda19a1f8d958707249f2dc2b4de6d4 > src/tests/containerizer/cgroups_isolator_tests.cpp > 40c18a1528f2da32c608b6c774500933a08562ea > src/tests/containerizer/docker_containerizer_tests.cpp > e37a9c11fe67846ceeeb6cfbf8a75d1e5753e185 > src/tests/containerizer/mesos_containerizer_tests.cpp > 01f2b38cfa67b144298c361e92170322864ac201 > src/tests/containerizer/provisioner_docker_tests.cpp > c664ff807583d587d94b0ab797330d5d3daf7657 > src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp > 0400052a1c73ccc15c932c573cfb324ca83e2e59 > src/tests/environment.cpp a5a255fc3e92991a201bb6cb4af9a625805395e0 > src/tests/fetcher_tests.cpp 95359ec84ed631bdd7b4fb5192e03f68cc57ed61 > src/tests/slave_tests.cpp 65d860594572b58a50a89358e31e97fd2a10bf08 > > > Diff: https://reviews.apache.org/r/67291/diff/2/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Jie Yu > >
