----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46175/#review128783 -----------------------------------------------------------
src/examples/test_hook_module.cpp (line 197) <https://reviews.apache.org/r/46175/#comment192239> Mind committing this as a separate patch? src/tests/hook_tests.cpp (lines 751 - 753) <https://reviews.apache.org/r/46175/#comment192249> Why did you need the mock here? src/tests/hook_tests.cpp (line 761) <https://reviews.apache.org/r/46175/#comment192246> How about no newline here to make the code block more clear and to be consistent with StartMaster above? src/tests/hook_tests.cpp (lines 764 - 768) <https://reviews.apache.org/r/46175/#comment192250> Ditto here, do you need the mock? src/tests/hook_tests.cpp (line 776) <https://reviews.apache.org/r/46175/#comment192247> How about no newline here to make the code block more clear and to be consistent with StartMaster above? src/tests/hook_tests.cpp (line 797) <https://reviews.apache.org/r/46175/#comment192244> `offers->size` src/tests/hook_tests.cpp (line 806) <https://reviews.apache.org/r/46175/#comment192241> How about the following to make the sandbox from TemporaryDirectoryTest::sandbox usage more explicit? ``` const string file = path::join(sandbox.get(), "post_fetch_hook"); ``` src/tests/hook_tests.cpp (line 823) <https://reviews.apache.org/r/46175/#comment192251> Seems like you could remove the repeatedly case, we don't expect more updates, right? src/tests/hook_tests.cpp (line 837) <https://reviews.apache.org/r/46175/#comment192243> Why `docker.get()->ps` instead of `docker->ps`? src/tests/hook_tests.cpp (line 843) <https://reviews.apache.org/r/46175/#comment192242> Why `docker.get()->rm` instead of `docker->rm`? - Ben Mahler On April 13, 2016, 10:33 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46175/ > ----------------------------------------------------------- > > (Updated April 13, 2016, 10:33 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-5209 > https://issues.apache.org/jira/browse/MESOS-5209 > > > Repository: mesos > > > Description > ------- > > Added a test for slavePostFetchHook. > > > Diffs > ----- > > src/examples/test_hook_module.cpp 3ff9fd71c275fd1a705ab3aca7a8041f29289bb0 > src/tests/hook_tests.cpp 97ff55ac7ec875b5ba5d4f17d80646c2d1dd4142 > > Diff: https://reviews.apache.org/r/46175/diff/ > > > Testing > ------- > > `sudo bin/mesos-tests.sh > --gtest_filter=*ROOT_DOCKER_VerifySlavePostFetchHook* ` > > > Thanks, > > Jie Yu > >
