----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63862/#review192436 -----------------------------------------------------------
src/tests/containerizer/docker_tests.cpp Lines 62 (patched) <https://reviews.apache.org/r/63862/#comment270582> We should probably comment as to why this doesn't use PowerShell (nanoserver image doesn't have it). Especially considering that `SLEEP_COMMAND` exists elsewhere, someone could easily assume they should change it. src/tests/containerizer/docker_tests.cpp Lines 63 (patched) <https://reviews.apache.org/r/63862/#comment270583> Is this the canonical path it should use on Windows? Seems like we could choose an arbitrary path. src/tests/containerizer/docker_tests.cpp Lines 107-111 (patched) <https://reviews.apache.org/r/63862/#comment270584> Since this could end up being really slow and make the tests appear hung, I think we should `LOG(WARNING) << "Downloading Docker image...";` or something to that effect. (So much better than Hours(1) though.) src/tests/containerizer/docker_tests.cpp Lines 121-128 (patched) <https://reviews.apache.org/r/63862/#comment270593> I see where this is quite useful, but perhaps the name could be a bit mores descriptive? I was thinking `from_root_dir` maybe? src/tests/containerizer/docker_tests.cpp Line 170 (original), 232-237 (patched) <https://reviews.apache.org/r/63862/#comment270585> Saw your reply explaining this on the review. I think we need to add a comment with the same explanation. src/tests/containerizer/docker_tests.cpp Line 318 (original), 391-428 (patched) <https://reviews.apache.org/r/63862/#comment270591> LGTM though another maintainer may want to take a look. src/tests/containerizer/docker_tests.cpp Lines 624-630 (patched) <https://reviews.apache.org/r/63862/#comment270592> Ah the woes of new technology. - Andrew Schwartzmeyer On Nov. 27, 2017, 9:45 a.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63862/ > ----------------------------------------------------------- > > (Updated Nov. 27, 2017, 9:45 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer and John Kordich. > > > Bugs: MESOS-7342 > https://issues.apache.org/jira/browse/MESOS-7342 > > > Repository: mesos > > > Description > ------- > > Ported the disabled tests to run on Windows. The following tests > could not be ported due to Windows platform limitations and remain > diabled: > - ROOT_DOCKER_MountRelativeContainerPath (can't mount volumes inside > sandbox). > - ROOT_DOCKER_NVIDIA_GPU_DeviceAllow (no GPU container support). > - ROOT_DOCKER_NVIDIA_GPU_InspectDevices (no GPU container support). > > > Diffs > ----- > > src/tests/containerizer/docker_tests.cpp > 5cabf5a0b0164f9923008677c58806c8931cbc8d > > > Diff: https://reviews.apache.org/r/63862/diff/2/ > > > Testing > ------- > > Windows mesos-test: > [==========] 754 tests from 77 test cases ran. (270586 ms total) > [ PASSED ] 754 tests. > > > Windows DockerTest only: > [==========] 11 tests from 1 test case ran. (85617 ms total) > [ PASSED ] 11 tests. > > Linux DockerTest (only 12 tests instead of 14, because I don't have Nvidia > GPU): > [==========] 12 tests from 1 test case ran. (12413 ms total) > [ PASSED ] 12 tests. > > > Thanks, > > Akash Gupta > >
