----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52639/#review158935 -----------------------------------------------------------
Did an initial pass on the test. Refer to my comments around splitting this test into two. Would take a detailed look again post the split. src/tests/api_tests.cpp (line 1465) <https://reviews.apache.org/r/52639/#comment229774> hmm, let's make this test limited to testing if the `recovered` field is correctly propagated in the `GetAgents` response - When the master/agent are started up. - Terminate the agent and restart master (1 recovered agent, 0 active agent) - Restart the agent now (0 recovered agent, 1 active agent) This is exactly the same split as you already. Let's create another test in `src/tests/master_tests.cpp` that does the same for `/slaves` and the `/state` endpoint. src/tests/api_tests.cpp (line 1467) <https://reviews.apache.org/r/52639/#comment229775> Can you remove the `Step xxx` prefix? None of the other tests in this file have it. src/tests/api_tests.cpp (line 1472) <https://reviews.apache.org/r/52639/#comment229776> Kill this comment and the one earlier around starting a master. They are self-explanatory. src/tests/api_tests.cpp (line 1493) <https://reviews.apache.org/r/52639/#comment229780> Can you do this outside of this scope as it's common to all subsequent blocks? It would then be consistent to all the other tests in this file. - Anand Mazumdar On Nov. 1, 2016, 5:30 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52639/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2016, 5:30 p.m.) > > > Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar. > > > Bugs: MESOS-6177 > https://issues.apache.org/jira/browse/MESOS-6177 > > > Repository: mesos > > > Description > ------- > > Added test for `recovered` AgentID and `AGENT_ADDED` after reregister. > > > Diffs > ----- > > src/tests/api_tests.cpp f87130d7554f98265ff6da39ea1d366839193e85 > > Diff: https://reviews.apache.org/r/52639/diff/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
