> On Dec. 1, 2017, 2:33 a.m., Andrew Schwartzmeyer wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 62 (patched) > > <https://reviews.apache.org/r/63862/diff/2/?file=1901750#file1901750line62> > > > > 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. > > Akash Gupta wrote: > There's a nanoserver image at `microsoft/powershell`. I can use that.
Added comment on why `SLEEP_COMMAND` can't be used. > On Dec. 1, 2017, 2:33 a.m., Andrew Schwartzmeyer wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 107-111 (patched) > > <https://reviews.apache.org/r/63862/diff/2/?file=1901750#file1901750line107> > > > > 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.) Added. > On Dec. 1, 2017, 2:33 a.m., Andrew Schwartzmeyer wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 121-128 (patched) > > <https://reviews.apache.org/r/63862/diff/2/?file=1901750#file1901750line121> > > > > I see where this is quite useful, but perhaps the name could be a bit > > mores descriptive? I was thinking `from_root_dir` maybe? Changed. > On Dec. 1, 2017, 2:33 a.m., Andrew Schwartzmeyer wrote: > > src/tests/containerizer/docker_tests.cpp > > Line 170 (original), 232-237 (patched) > > <https://reviews.apache.org/r/63862/diff/2/?file=1901750#file1901750line232> > > > > Saw your reply explaining this on the review. I think we need to add a > > comment with the same explanation. Yeah. Added an explanation - Akash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63862/#review192436 ----------------------------------------------------------- On Dec. 7, 2017, 12:05 p.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63862/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2017, 12:05 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Jie Yu, John Kordich, and > Joseph Wu. > > > 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/3/ > > > 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 > >
