> On April 15, 2016, 11:39 a.m., Alexander Rojas wrote: > > src/tests/metrics_tests.cpp, line 288 > > <https://reviews.apache.org/r/46261/diff/2/?file=1346287#file1346287line288> > > > > To be consistent with your other patches, could you add tests for wrong > > credentials. > > > > Also, would be nice to have a test where you actually can access the > > metrics endpoint.
I think I would actually prefer to eliminate the tests from other patches which include requests containing invalid credentials. Since the libprocess test `HttpAuthenticationTest.Basic` already tests the basic HTTP authenticator with both missing credentials and invalid credentials, I think it's sufficient in these tests to only test one of those cases. What do you think? Regarding a test which successfully accesses the metrics endpoint, see my reply to your comment on the tests for the MetricsTests in libprocess. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46261/#review129100 ----------------------------------------------------------- On April 15, 2016, 7:01 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46261/ > ----------------------------------------------------------- > > (Updated April 15, 2016, 7:01 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-4902 > https://issues.apache.org/jira/browse/MESOS-4902 > > > Repository: mesos > > > Description > ------- > > The tests `MetricsTest.AgentAuthenticationEnabled` and > `MetricsTest.MasterAuthenticationEnabled` are added in > this patch. > > > Diffs > ----- > > src/tests/metrics_tests.cpp eacff678d06da7ba8afee6ab68261968561dffc3 > > Diff: https://reviews.apache.org/r/46261/diff/ > > > Testing > ------- > > `sudo make check` on OSX. > > > Thanks, > > Greg Mann > >
