----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14097/#review26291 -----------------------------------------------------------
Looks good, I just had a comment below to try to clean up some of the boilerplate code needed in the tests to set up the executors / testing isolator. src/tests/allocator_tests.cpp <https://reviews.apache.org/r/14097/#comment51381> Seems like a lot of boilerplate to set up the TestingIsolator! Why not use CREATE_EXECUTOR_INFO up here and have it take a string for the executorId? This saves the need to explicitly construct and set the executor id. e.g. ExecutorInfo executor1 = CREATE_EXECUTOR_INFO("e1", "exit 1"); Ditto elsewhere. src/tests/isolator.hpp <https://reviews.apache.org/r/14097/#comment51377> Consider logging this explicitly when the framework id is different and the executor id is the same (to avoid potential confusion later). src/tests/master_tests.cpp <https://reviews.apache.org/r/14097/#comment51378> Why the s/executorInfo.executor_id()/DEFAULT_EXECUTOR_ID/ change here? - Ben Mahler On Sept. 12, 2013, 1:52 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14097/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2013, 1:52 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-467 and MESOS-686 > https://issues.apache.org/jira/browse/MESOS-467 > https://issues.apache.org/jira/browse/MESOS-686 > > > Repository: mesos-git > > > Description > ------- > > TestingIsolator now rejects launching of multiple executors with same > ExecutorID. This means 2 different frameworks cannot used the same executor > id. While this is a restriction it is better than changing all tests to grab > the framework id before creating testing isolator. > > Also fixed tests that were incorrectly launching multiple executors. > > > Diffs > ----- > > src/launcher/launcher.cpp 6f2677a5e04871fa09ca3d0a926e1474eff609c7 > src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 > src/tests/allocator_zookeeper_tests.cpp > 6e3214c15c14dc8ba82082738c172c8833cd0887 > src/tests/fault_tolerance_tests.cpp > 10e52c401476eb8416361de49b8e4061bb7ac4f3 > src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f > src/tests/isolator.hpp fe6b38d4f31f7a8cb901c37c0d153f4ddc08c923 > src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 > src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e > > Diff: https://reviews.apache.org/r/14097/diff/ > > > Testing > ------- > > make check > > sudo ./bin/mesos-tests.sh --gtest_filter="*FrameworkExited*" > --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle > > sudo ./bin/mesos-tests.sh --gtest_filter="*GarbageCollectorIntegrationTest*" > --gtest_repeat=1000 --gtest_break_on_failure --gtest_shuffle > > > Thanks, > > Vinod Kone > >
