Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-30 Thread Paul Brett
On March 17, 2015, 5:51 p.m., Ian Downes wrote: src/tests/port_mapping_tests.cpp, line 196 https://reviews.apache.org/r/32133/diff/1/?file=896458#file896458line196 Why an empty JSON object rather than a TryJSON::Object? Current implementation of the mesos-network-helper has the

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-30 Thread Paul Brett
On March 18, 2015, 8:58 p.m., Jie Yu wrote: Looking at all the port mapping isolator tests here, they are pretty much duplicating logics in MesosContainerier (e.g., launchHelper, statisticsHelper). It's not good for readability. It would be cool if we can simply write integration

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-18 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76933 --- Thanks for improving this part! While you are at it, could you

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-18 Thread Paul Brett
On March 17, 2015, 5:51 p.m., Ian Downes wrote: src/tests/port_mapping_tests.cpp, lines 266-286 https://reviews.apache.org/r/32133/diff/1/?file=896458#file896458line266 I don't see the value in pulling these out as separate functions compared to the original code? _Perhaps_ a

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76943 --- Looking at all the port mapping isolator tests here, they are

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-17 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76744 --- src/tests/port_mapping_tests.cpp

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-17 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76785 --- src/tests/port_mapping_tests.cpp

Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-16 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/ --- Review request for mesos and Ian Downes. Bugs: mesos-2332

Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics

2015-03-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76681 --- Patch looks great! Reviews applied: [32133] All tests passed. -