----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50127/#review144762 -----------------------------------------------------------
src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 17) <https://reviews.apache.org/r/50127/#comment210862> You are losing the following incldue: ``` #include <list> ``` src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 61 - 62) <https://reviews.apache.org/r/50127/#comment210863> move this between Line 50 and Line 51 src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 65) <https://reviews.apache.org/r/50127/#comment210861> It is not suggested to use `namespace` directly, so I think that you need keep the following: ``` using process::Future; using process::Owned; ``` And add the one as you code. src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 81 - 141) <https://reviews.apache.org/r/50127/#comment210866> Seems those are from docker_containerizer_test.cpp, I think that we do not need to copy the code here but add the test case to docker_containerizer_test.cpp as you want to test docker containerizer. src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 745) <https://reviews.apache.org/r/50127/#comment210877> This will cause test failed if the system do not have nvml. Can you please add a filter to filter out the test if the nvml is not avaiable? You can refer to https://github.com/apache/mesos/blob/master/src/tests/environment.cpp#L279-L313 for how to add a filter. src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 758) <https://reviews.apache.org/r/50127/#comment210869> Do you need this with docker containerizer? src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 770) <https://reviews.apache.org/r/50127/#comment210876> s/.get()/-> src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 815) <https://reviews.apache.org/r/50127/#comment210875> s/offers.get().size()/offers->size() src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 851) <https://reviews.apache.org/r/50127/#comment210873> ``` driver.launchTasks(offers1->at(0).id(), {task}); ``` src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 898 - 899) <https://reviews.apache.org/r/50127/#comment210871> ``` EXPECT_EQ(1, statusRunning.get().container_status().network_infos(0).ip_addresses().size()); // NOLINT(whitespace/line_length) ``` src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 920) <https://reviews.apache.org/r/50127/#comment210872> s/unsigned int/size_t - Guangya Liu On 八月 2, 2016, 2:23 p.m., Yubo Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50127/ > ----------------------------------------------------------- > > (Updated 八月 2, 2016, 2:23 p.m.) > > > Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull. > > > Bugs: MESOS-5795 > https://issues.apache.org/jira/browse/MESOS-5795 > > > Repository: mesos > > > Description > ------- > > This added a testing case for end-to-end GPU support for docker > containerizer. > > > Diffs > ----- > > src/tests/containerizer/nvidia_gpu_isolator_tests.cpp > fea1f9f0a03373692ef2a6dd2bc5722dc6f46d5b > > Diff: https://reviews.apache.org/r/50127/diff/ > > > Testing > ------- > > GTEST_FILTER="NvidiaGpuDockerContainerizerTest.ROOT_DOCKER_LaunchWithGpu" > make -j check > > > Thanks, > > Yubo Li > >
