----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57474/#review171690 -----------------------------------------------------------
Test logic looks good, although I question "archbishop" as a principal. And it doesn't look like you thoroughly reviewed the text in your comments. src/tests/authorization_tests.cpp Lines 888 (patched) <https://reviews.apache.org/r/57474/#comment244705> s/Cheks/Checks/ src/tests/authorization_tests.cpp Lines 912 (patched) <https://reviews.apache.org/r/57474/#comment244706> When has an archbishop ever also been a king? I was expecting another person's name. As a title, "archbishop" makes more sense as a Mesos role than a principal. Archbishop William King was technically both. https://en.wikipedia.org/wiki/William_King_(bishop) src/tests/authorization_tests.cpp Lines 1048 (patched) <https://reviews.apache.org/r/57474/#comment244708> s/Not not/Not/ s/irself/itself/ src/tests/authorization_tests.cpp Lines 1184 (patched) <https://reviews.apache.org/r/57474/#comment244709> s/register frameworks/reserve resources/ src/tests/authorization_tests.cpp Lines 1478 (patched) <https://reviews.apache.org/r/57474/#comment244710> "Not not" and "irself" again. Please fix throughout. src/tests/authorization_tests.cpp Lines 1597 (patched) <https://reviews.apache.org/r/57474/#comment244711> s/register frameworks/create volumes/ src/tests/authorization_tests.cpp Lines 1866 (patched) <https://reviews.apache.org/r/57474/#comment244712> These could uses comments like the others src/tests/authorization_tests.cpp Line 1578 (original), 1943 (patched) <https://reviews.apache.org/r/57474/#comment244714> "register frameworks" again. Please fix throughout. src/tests/authorization_tests.cpp Lines 4522-4524 (patched) <https://reviews.apache.org/r/57474/#comment244715> This comment belongs above the previous block of archbishop->king src/tests/authorization_tests.cpp Lines 4588 (patched) <https://reviews.apache.org/r/57474/#comment244719> s/view any role/update the weight of any role/ src/tests/authorization_tests.cpp Lines 4737 (patched) <https://reviews.apache.org/r/57474/#comment244716> s/get weights/get quotas/g and s/update weights/get quotas/g throughout this test. src/tests/authorization_tests.cpp Lines 4753 (patched) <https://reviews.apache.org/r/57474/#comment244718> s/update the weight/view the quota/ src/tests/authorization_tests.cpp Lines 4760 (patched) <https://reviews.apache.org/r/57474/#comment244717> s/view any role/view quota for any role/ - Adam B On April 11, 2017, 1:47 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57474/ > ----------------------------------------------------------- > > (Updated April 11, 2017, 1:47 a.m.) > > > Review request for mesos, Adam B and Benjamin Bannier. > > > Bugs: MESOS-7026 > https://issues.apache.org/jira/browse/MESOS-7026 > > > Repository: mesos > > > Description > ------- > > Adds tests for each of the actions which support hierarchical roles. > > > Diffs > ----- > > src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d > > > Diff: https://reviews.apache.org/r/57474/diff/4/ > > > Testing > ------- > > `make check` > > > Thanks, > > Alexander Rojas > >
