> On June 19, 2014, 6:41 p.m., Vinod Kone wrote: > > Only partial review. But after looking at some tests, this parameterizing > > test behavior inside the tests seems more complicated than its worth. I > > think its better to just convert all these tests to rate limiting tests. We > > have lot of other tests in our code base that test the code without rate > > limiting. You could still ensure that the counters are correct when you are > > using rate limiting. The correctness of the counters code doesn't depend on > > whether you are rate limiting or not, so it should be fine. Thoughts?
Other tests, without modifications, do not test the counters and explicit unlimited rates. In my tests, with throttling == false, counters directly go to the "processed" state whereas with throttling they go through the intermediate "received but not processed" stage and transition only happens with Clock advances. That's how I am using the the template parameter. But I agree it's simpler not to have the template and just test rate limiting for most tests. I'll make the change. I'll turn RateLimitingTest.FrameworkMessageCounterMetrics into RateLimitingTest.NoRateLimiting so we have the test for the "explicit unlimited rates" case too. > On June 19, 2014, 6:41 p.m., Vinod Kone wrote: > > src/tests/rate_limiting_tests.cpp, lines 113-119 > > <https://reviews.apache.org/r/22740/diff/2/?file=613101#file613101line113> > > > > Can you do it after you start the master? Yeah I realized this after I saw a few flaky Jenkins tests but the issue here I believe is that I need to do a Clock::settle after the AWAIT_READY. StartMaster() already waits for Master's recovery to finish but it's "not quite" finished when FUTUER_DISPATCH(_recover) is ready because _recover has yet to finish execution. Maybe we should add that settle into StartMaster(). > On June 19, 2014, 6:41 p.m., Vinod Kone wrote: > > src/tests/rate_limiting_tests.cpp, line 194 > > <https://reviews.apache.org/r/22740/diff/2/?file=613101#file613101line194> > > > > you can kill this. this is done by the test teardown by default. Done. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22740/#review46243 ----------------------------------------------------------- On June 19, 2014, 12:03 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22740/ > ----------------------------------------------------------- > > (Updated June 19, 2014, 12:03 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Bugs: MESOS-1445 > https://issues.apache.org/jira/browse/MESOS-1445 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/tests/rate_limiting_tests.cpp 9a544618e70e7ce3091fa303597eb1bb3d161bf9 > > Diff: https://reviews.apache.org/r/22740/diff/ > > > Testing > ------- > > make check all test and *RateLimitingTest* for high iterations. > > > Thanks, > > Jiang Yan Xu > >
