> On June 25, 2014, 7:15 p.m., Vinod Kone wrote:
> > src/tests/isolator_tests.cpp, lines 55-66
> > <https://reviews.apache.org/r/22471/diff/5/?file=616523#file616523line55>
> >
> >     @Jie: Why? If those includes are only used by network isolator code I 
> > prefer them to be guarded by #ifdef.

properly #ifdef'ed and in alphabetical order, both for includes and 'using' 
statements. #ifdef's are split up when necessary.


> On June 25, 2014, 7:15 p.m., Vinod Kone wrote:
> > src/tests/isolator_tests.cpp, line 769
> > <https://reviews.apache.org/r/22471/diff/5/?file=616523#file616523line769>
> >
> >     s/external//

trying to make a distinction to the name server on localhost?  


> On June 25, 2014, 7:15 p.m., Vinod Kone wrote:
> > src/tests/isolator_tests.cpp, lines 786-800
> > <https://reviews.apache.org/r/22471/diff/5/?file=616523#file616523line786>
> >
> >     Initialization of these is very straightforward. I wonder if it's 
> > better to just define them in each test for better readability. I'll leave 
> > it up to you.

Tt was like that originally. I felt this could be better because it's a lot of 
duplicate code and different cases actually use different subsets of them. 


> On June 25, 2014, 7:15 p.m., Vinod Kone wrote:
> > src/tests/isolator_tests.cpp, line 927
> > <https://reviews.apache.org/r/22471/diff/5/?file=616523#file616523line927>
> >
> >     Is 'nc' guaranteed to be present on every linux box by default? If not 
> > maybe the tests should be disabled? Same with arping?

checked in SetupTestCase.


> On June 25, 2014, 7:15 p.m., Vinod Kone wrote:
> > src/tests/isolator_tests.cpp, line 780
> > <https://reviews.apache.org/r/22471/diff/5/?file=616523#file616523line780>
> >
> >     Is resolv.conf guaranteed to have atleast one external name server? If 
> > not, looks like the tests that use this will fail.

will abort for now. added a TODO.


- Chi


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22471/#review46659
-----------------------------------------------------------


On June 26, 2014, 6:03 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22471/
> -----------------------------------------------------------
> 
> (Updated June 26, 2014, 6:03 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Vinod Kone, and Cong Wang.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> These tests test different different protocols (TCP, UDP, ICMP, ARP, DNS) in 
> different connection scenarios (C2C, H2C). 
> 
> 
> Diffs
> -----
> 
>   src/tests/isolator_tests.cpp 6fef4e0 
> 
> Diff: https://reviews.apache.org/r/22471/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Reply via email to