----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68354/#review208576 -----------------------------------------------------------
Fix it, then Ship it! src/tests/authentication_tests.cpp Lines 422 (patched) <https://reviews.apache.org/r/68354/#comment292636> Nit: I am not a big fan of hard-coded magic numbers, so I'd declare `expected` as: ``` const std::vector<std::pair<Duration, Duration>> expectedLimits = { {Seconds(5), Seconds(7)}, {Seconds(5), Seconds(9)}, {Seconds(5), Seconds(13)}, {Seconds(5), Seconds(21)}, {Seconds(5), Seconds(37)}, {Seconds(5), Minutes(1)}, {Seconds(5), Minutes(1)} }; ``` And then change the loop to: ``` for (auto it = expectedLimits.begin(); it != expectedLimits.end(); ++it) { ``` But I don't expect we to add more items to `expected`, so feel free to drop this issue if you prefer your current version. - Gastón Kleiman On Aug. 16, 2018, 4:54 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68354/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2018, 4:54 p.m.) > > > Review request for mesos, Benjamin Mahler and Gastón Kleiman. > > > Repository: mesos > > > Description > ------- > > This test verifies that the agent backs-off properly when > retrying authentication according to the configured parameters. > > Also mocked `Slave::authenticate()` for this test. > > > Diffs > ----- > > src/slave/slave.hpp 0420109ac93e1249906c52437e5859c5ee033fb6 > src/tests/authentication_tests.cpp f7a2cf17cf6154c9c67e405661bba57cf1254845 > src/tests/mock_slave.hpp 9a74bf35d2cab0a72ba6376392239d8080a49304 > src/tests/mock_slave.cpp 94a5b0d20475f49dde99108a009682b520175aa4 > > > Diff: https://reviews.apache.org/r/68354/diff/5/ > > > Testing > ------- > > make check > > Added test continuously running without failure. > > > Thanks, > > Meng Zhu > >