----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32466/#review77692 -----------------------------------------------------------
Ship it! Modulo the questions below, ROOT_CleanUpOrphanTest looks good though. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125874> Why wasn't this needed before? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125878> Why do you need this if you're using WillRepeatedly? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125879> Might want to confirm this compiles on 4.4 if you can. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125880> Why do you need the filters here? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125881> What's this for? Can you add a comment for posterity? (i.e. why is the settle needed before the advance here). src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125885> 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. ``` src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125882> Not yours, but const & ? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125886> What was this trying to test? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125875> Why wasn't this needed before? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125876> Might want to check that this compiles on 4.4. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/32466/#comment125877> You don't need the 'true' here. - Ben Mahler 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 > >
