> On July 3, 2014, 12:18 a.m., Vinod Kone wrote: > > src/tests/port_mapping_tests.cpp, line 1233 > > <https://reviews.apache.org/r/23214/diff/2/?file=623164#file623164line1233> > > > > can you use ASSERT_NE? > > > > Also, how is this possible? Aren't we controlling whether this > > isolation is set or not? > > Chi Zhang wrote: > flags.isolation is set in > ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags. > network/portmapping could be dropped if certain checks don't meet.
That seems dangerous. Should these tests be even running if those checks (non-linux, non-root, non-network-isolator-configuration) are not met? You might want to update environment.cpp and this file to make these tests are not run in the first place. > On July 3, 2014, 12:18 a.m., Vinod Kone wrote: > > src/tests/port_mapping_tests.cpp, line 1246 > > <https://reviews.apache.org/r/23214/diff/2/?file=623164#file623164line1246> > > > > This is only used in one test. Not sure it belongs in the fixture. > > Chi Zhang wrote: > had this thought too. Then I thought this 'flags' could be kept in > general for future since we might want to test more scenarios that involve > supporting mixed containers. we can pull it out into the fixture in the future when we need it. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23214/#review47258 ----------------------------------------------------------- On July 3, 2014, 7:40 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23214/ > ----------------------------------------------------------- > > (Updated July 3, 2014, 7:40 p.m.) > > > Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone. > > > Bugs: https://issues.apache.org/jira/browse/MESOS-1557 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1557 > > > Repository: mesos-git > > > Description > ------- > > This eases deployment where a slave could be upgraded to use Network Isolator > without removing all the existing tasks. > > - Added a new test. > - Moved all portmapping tests to a new file. > > > Diffs > ----- > > src/Makefile.am 6438939 > src/slave/containerizer/isolators/network/port_mapping.hpp ac3bee3 > src/slave/containerizer/isolators/network/port_mapping.cpp d16547a > src/tests/isolator_tests.cpp 4650f97 > src/tests/mesos.cpp 0d7f335 > src/tests/port_mapping_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/23214/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
