----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22471/#review46659 -----------------------------------------------------------
src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82189> @Jie: Why? If those includes are only used by network isolator code I prefer them to be guarded by #ifdef. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82187> should this be ifdefed too? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82190> reorder alphabetically. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82191> s/external// src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82208> Is resolv.conf guaranteed to have atleast one external name server? If not, looks like the tests that use this will fail. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82210> 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. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82192> s/assit/assist/ src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82194> s/Port/port/ src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82195> s/partialPorts1/container1Ports/ ? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82196> ditto. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82207> These need not be member variables. Just define them in the tests. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82197> s/connects to/attempts to connect to/ src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82211> Is 'nc' guaranteed to be present on every linux box by default? If not maybe the tests should be disabled? Same with arping? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82200> what about public port like you did in the previous test? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82202> s/erroPort/errorPort/ src/tests/isolator_tests.cpp <https://reviews.apache.org/r/22471/#comment82203> s/UDP/TCP/ - Vinod Kone On June 25, 2014, 2:10 a.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22471/ > ----------------------------------------------------------- > > (Updated June 25, 2014, 2:10 a.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 0bbec09 > > Diff: https://reviews.apache.org/r/22471/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Chi Zhang > >
