----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69064/#review211377 -----------------------------------------------------------
src/tests/master_load_tests.cpp Lines 96-102 (patched) <https://reviews.apache.org/r/69064/#comment296395> Let's be consistent with the underscore convention for data members in this file. This class uses a trailing underscore for data member names, while the blocking authorizer does not. src/tests/master_load_tests.cpp Lines 152 (patched) <https://reviews.apache.org/r/69064/#comment296408> We could do ``` promises.front().associate(futures.front()); ``` src/tests/master_load_tests.cpp Lines 224 (patched) <https://reviews.apache.org/r/69064/#comment296381> If you end up keeping the trailing underscores on the data members of this class, then: s/_authorizer/authorizer/ src/tests/master_load_tests.cpp Lines 293-297 (patched) <https://reviews.apache.org/r/69064/#comment296397> Should we add something like ASSERT_TRUE(Clock::now() - whileLoopStartTime < Seconds(15)); to make sure that we don't end up getting stuck spinning here due to some bug in the future? src/tests/master_load_tests.cpp Lines 310 (patched) <https://reviews.apache.org/r/69064/#comment296398> s/deferals/deferrals/ src/tests/master_load_tests.cpp Lines 357 (patched) <https://reviews.apache.org/r/69064/#comment296399> Why not use `LOG()` here? src/tests/master_load_tests.cpp Lines 400-401 (patched) <https://reviews.apache.org/r/69064/#comment296400> Could you make this patch a dependant of the metric patch and assert that the metric value has been incremented here? Ditto for the other tests in this file. src/tests/master_load_tests.cpp Lines 414 (patched) <https://reviews.apache.org/r/69064/#comment296401> s/roles/frameworks/ ?? Here and below. src/tests/master_load_tests.cpp Lines 461 (patched) <https://reviews.apache.org/r/69064/#comment296402> Remove trailing space after `EXPECT_TRUE`. src/tests/master_load_tests.cpp Lines 480 (patched) <https://reviews.apache.org/r/69064/#comment296403> I think you could remove this comment. src/tests/master_load_tests.cpp Lines 526 (patched) <https://reviews.apache.org/r/69064/#comment296406> Remove space after `EXPECT_TRUE`. src/tests/master_load_tests.cpp Lines 555 (patched) <https://reviews.apache.org/r/69064/#comment296407> Can you use some other header to test this case, so that we don't need to disable the test? You could even set an arbitrary header which is not interpreted by libprocess. - Greg Mann On Dec. 12, 2018, 8:53 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69064/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2018, 8:53 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > This commit adds a set of unit test to verify that > basic Master HTTP endpoints still work correctly > under the presence of request caching. > > > Diffs > ----- > > src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b > src/tests/master_load_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/69064/diff/4/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
