> On June 18, 2016, 2:43 p.m., haosdent huang wrote: > > src/tests/api_tests.cpp, line 456 > > <https://reviews.apache.org/r/48094/diff/5/?file=1423341#file1423341line456> > > > > I think we could not gruantee the `v1Response->get_roles().roles(2)` > > equal to `role2`. How about `foreach` here and find the role that name is > > `role2`? > > Abhishek Dasgupta wrote: > roles are getting inserted as per the roleList which is set<string> . So, > role2 comes in v1Response->get_roles().roles(2) .. Am I missing something > here?? > > haosdent huang wrote: > I see, but if we don't document this implmentation details in our > document/proto, we could not guarantee the order of roles here. Suppose we > change the roles order alphabetcially in the implementation, this line would > be break. Let's avoid to write fragile codes.
Haosdent, we are not advertising the implementation details to the user. The idea is to check the functional correctness of our implementation without being too verbose. Since, we know that the `roleList` here is a `set`, we can _assume_ the order in tests. We already follow this pattern for some of the existing tests, e.g., https://github.com/apache/mesos/blob/master/src/tests/role_tests.cpp#L434 - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48094/#review138405 ----------------------------------------------------------- On June 18, 2016, 4:21 p.m., Abhishek Dasgupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48094/ > ----------------------------------------------------------- > > (Updated June 18, 2016, 4:21 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-5494 > https://issues.apache.org/jira/browse/MESOS-5494 > > > Repository: mesos > > > Description > ------- > > Implemented GET_ROLES Call in v1 master API. > > > Diffs > ----- > > include/mesos/master/master.proto fa92240 > include/mesos/mesos.proto e4c5bd3 > include/mesos/v1/master/master.proto 59e978f > include/mesos/v1/mesos.proto 9be22f0 > src/master/http.cpp a6beb17 > src/master/master.hpp 618d928 > src/tests/api_tests.cpp afa5ffa > > Diff: https://reviews.apache.org/r/48094/diff/ > > > Testing > ------- > > On Ubuntu 16.04: > > sudo GTEST_FILTER="*MasterAPITest.*" make -j2 check > > > Thanks, > > Abhishek Dasgupta > >
