----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71022/#review216412 -----------------------------------------------------------
Fix it, then Ship it! src/tests/master_quota_tests.cpp Lines 153-187 (patched) <https://reviews.apache.org/r/71022/#comment303638> Can you make these free standing rather than part of the fixture? IMHO this fixture already makes the tests less readable (e.g. need to know about `defaultAgentResources`, need to know whether these other functions do anything special since they're non static members, etc). So it would be nice to prune it down to just a named fixture eventually. src/tests/master_quota_tests.cpp Lines 975 (patched) <https://reviews.apache.org/r/71022/#comment303639> Hm.. was this needed? It looks like the additional EXPECT below will work without it? Ditto for the others below src/tests/master_quota_tests.cpp Lines 1307-1308 (patched) <https://reviews.apache.org/r/71022/#comment303641> On all of these, it looks more natural to have the headers declared after the EXPECT, since the headers are related to the http::post - Benjamin Mahler On July 7, 2019, 10:12 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71022/ > ----------------------------------------------------------- > > (Updated July 7, 2019, 10:12 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-8968 > https://issues.apache.org/jira/browse/MESOS-8968 > > > Repository: mesos > > > Description > ------- > > These tests reuse the existing tests for `SET_QUOTA` and > `REMOVE_QUOTA` calls. In general, `UPDATE_QUOTA` request > should fail where `SET_QUOTA` fails. When the existing > test expects `SET_QUOTA` call succeeds, we test the > `UPDATE_QUOTA` call by first remove the set quota and then > send the `UPDATE_QUOTA` request. > > > Diffs > ----- > > src/tests/master_quota_tests.cpp 34a652029e31827f8d94bb66d408fbe3d121fd8f > > > Diff: https://reviews.apache.org/r/71022/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
