----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56475/#review165326 -----------------------------------------------------------
3rdparty/libprocess/src/tests/http_tests.cpp (lines 1817 - 1846) <https://reviews.apache.org/r/56475/#comment237169> _Here and below:_ This is a pure personal preference and Mesos doesn't necesarily follow the pattern (TBH, I don't always follow it), but I think blocks of related statements are grouped together, i.e.: ```c++ MockAuthenticator *forbiddenAuthenticator = new MockAuthenticator(); setAuthenticator("realm", Owned<Authenticator>(forbiddenAuthenticator)); AuthenticationResult authenticationForbidden; authenticationForbidden.forbidden = http::Forbidden(); EXPECT_CALL(*forbiddenAuthenticator, authenticate(_)) .WillOnce(Return(authenticationForbidden)); MockAuthenticator *basicAuthenticator = new MockAuthenticator(); setAuthenticator("realm", Owned<Authenticator>(basicAuthenticator)); AuthenticationResult authenticationUnauthorizedBasic; authenticationUnauthorizedBasic.unauthorized = http::Unauthorized({"Basic realm=\"realm\""}); EXPECT_CALL(*basicAuthenticator, authenticate(_)) .WillOnce(Return(authenticationUnauthorizedBasic)); // ... Other similar blocks. Http http; ``` The reason I prefer this is, if you need to remove one block, you need to select one time and you remove all the related code at once. If you need to add a new block, you can copy once and paste once, it also helps with code localization (Check Code Complete, ed 2 chapter 10: General Issues in Using Variables). 3rdparty/libprocess/src/tests/http_tests.cpp (lines 1860 - 1863) <https://reviews.apache.org/r/56475/#comment237170> Probably create the expected value in a variable outside of the `EXPECT_EQ`? - Alexander Rojas On Feb. 9, 2017, 2:03 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56475/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2017, 2:03 a.m.) > > > Review request for mesos, Alexander Rojas, Jan Schlicht, Till Toenshoff, and > Vinod Kone. > > > Bugs: MESOS-7004 > https://issues.apache.org/jira/browse/MESOS-7004 > > > Repository: mesos > > > Description > ------- > > This patch adds the following libprocess tests to ensure > correct behavior when multiple authenticators are loaded > for a single realm: > HttpAuthenticationTest.UnauthorizedMultipleAuthenticators > HttpAuthenticationTest.ForbiddenMultipleAuthenticators > HttpAuthenticationTest.AuthenticatedMultipleAuthenticators > > > Diffs > ----- > > 3rdparty/libprocess/src/tests/http_tests.cpp > fb4da9aecff0370d97a15269c5d8fffb30e0478f > > Diff: https://reviews.apache.org/r/56475/diff/ > > > Testing > ------- > > The following commands were used to test: > `make check` > `3rdparty/libprocess/libprocess-tests > --gtest_filter="*MultipleAuthenticators*" --gtest_repeat=10000` > > > Thanks, > > Greg Mann > >
