> On Jan. 10, 2018, 1:27 p.m., Alexander Rukletsov wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 130-137 (patched) > > <https://reviews.apache.org/r/63862/diff/5/?file=1932789#file1932789line130> > > > > Looks like instead of this adhoc routine we should rather introduce a > > root prefix in stout. > > Andrew Schwartzmeyer wrote: > I don't think we should introduce this abstraction into stout until it > becomes the case that other code needs it too. Currently, this abstraction is > needed solely to make the Docker tests compatible on both platforms. Let's > avoid unnecessary stout bloat.
Yeah, the function is just a helper function for some of the Docker tests. I don't see how this would be used anywhere else. > On Jan. 10, 2018, 1:27 p.m., Alexander Rukletsov wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 139-153 (patched) > > <https://reviews.apache.org/r/63862/diff/5/?file=1932789#file1932789line139> > > > > Again, this is something we should likely do in a broader scope. > > Chances are, someone else in a different place will try to solve the same > > problem. > > Andrew Schwartzmeyer wrote: > Same as above. At the point of someone else solving the same problem, > then it would make sense to move the code into a more general purpose > location and reuse it. But we shouldn't do it until then, because otherwise > we're just guessing at future technical needs. This is specific to Docker and only for test code that checks the container exit value, which is onlt used here. So, I don't think other places will ever use this. - Akash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63862/#review195133 ----------------------------------------------------------- On Jan. 5, 2018, 6:33 p.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63862/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2018, 6:33 p.m.) > > > Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston > Kleiman, Jie Yu, John Kordich, Joseph Wu, and Michael Park. > > > 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 > 0ac4a79e03d5e11ead5a57a9749e26c20a1ddd57 > > > Diff: https://reviews.apache.org/r/63862/diff/5/ > > > 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 > >
