----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63056/#review190471 -----------------------------------------------------------
src/tests/agent_container_api_tests.cpp Lines 88 (patched) <https://reviews.apache.org/r/63056/#comment267851> Can you comment on what each field means? src/tests/agent_container_api_tests.cpp Lines 164 (patched) <https://reviews.apache.org/r/63056/#comment267853> hum, i guess you cannot use `AWAIT_READY` because it's not in the test body. In general, I'd suggest folks to limit the test fixture to have just helper methods, and not having EXPECT/ASSERT in the helper. I would actually suggest you just inline this method in the following test body. By doing that way, you probably don't need `NormalParentContainerDependencies` as well. src/tests/agent_container_api_tests.cpp Lines 251-291 (patched) <https://reviews.apache.org/r/63056/#comment267855> Do you still need this? Can you just use LAUNCH_CONTAINER for both cases? Ditto below. Or you just want to exercise that path? src/tests/agent_container_api_tests.cpp Lines 420 (patched) <https://reviews.apache.org/r/63056/#comment267852> move `{` to the next line. src/tests/agent_container_api_tests.cpp Lines 472 (patched) <https://reviews.apache.org/r/63056/#comment267856> can you avoid `std::` prefix and use a `using` clause above? - Jie Yu On Nov. 2, 2017, 4 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63056/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 4 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-7305 > https://issues.apache.org/jira/browse/MESOS-7305 > > > Repository: mesos > > > Description > ------- > > This introduces a new test class that is parameterized based > on the type of API used to launch (1) the parent container, > (2) the nested container, and (3) the content type (like all > other API tests). The test is also parameterized based > on the enabled set of isolators. > > Because this change overlaps with existing nested container > API tests, those test(s) will be removed. > > > Diffs > ----- > > src/Makefile.am 1c97b1fd8151f87c4e9e6d62884b0ef7d582c312 > src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76 > src/tests/agent_container_api_tests.cpp PRE-CREATION > src/tests/api_tests.cpp b3efac942f538bcf2594a397d54042028a7aa7a5 > > > Diff: https://reviews.apache.org/r/63056/diff/2/ > > > Testing > ------- > > make check > > On... > OSX and Ubuntu 16 > > > Thanks, > > Joseph Wu > >
