> On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, line 1810 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1810> > > > > Might want to confirm this compiles on 4.4 if you can.
I always use 4.4. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, line 1753 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1753> > > > > Why wasn't this needed before? OK, found the issue MESOS-2554. Added a NOTE. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, lines 1779-1780 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1779> > > > > Why do you need this if you're using WillRepeatedly? Added a NOTE. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, lines 1927-1933 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1927> > > > > What was this trying to test? I don't know, thus I removed it. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, lines 1854-1855 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1854> > > > > What's this for? Can you add a comment for posterity? (i.e. why is the > > settle needed before the advance here). Added a comment. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, lines 1916-1918 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1916> > > > > Can we keep the comment you removed above but have it down here? > > > > e.g. > > > > ``` > > // Ensure that both containers (with and without network isolation) > > were recovered. > > ``` Done. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, line 1920 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1920> > > > > Not yours, but const & ? Done. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, line 1953 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1953> > > > > Why wasn't this needed before? Fixed. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, line 1993 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line1993> > > > > Might want to check that this compiles on 4.4. I alwasy use 4.4. > On March 25, 2015, 12:57 a.m., Ben Mahler wrote: > > src/tests/port_mapping_tests.cpp, line 2003 > > <https://reviews.apache.org/r/32466/diff/1/?file=904882#file904882line2003> > > > > You don't need the 'true' here. Killed. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32466/#review77692 ----------------------------------------------------------- On March 25, 2015, 12:04 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32466/ > ----------------------------------------------------------- > > (Updated March 25, 2015, 12:04 a.m.) > > > Review request for mesos, Ben Mahler, Chi Zhang, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Cleaned up and simplified PortMappingMesosTest. > > > Diffs > ----- > > src/tests/port_mapping_tests.cpp 623840e71938791a562a32989775818275e6d37e > > Diff: https://reviews.apache.org/r/32466/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Jie Yu > >
