Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13078 )
Change subject: IMPALA-8446: Create a unit test for Admission Controller. ...................................................................... Patch Set 1: (5 comments) Looks good overall. I think Bikram should take a look too since he's spent more time hands on with this code. http://gerrit.cloudera.org:8080/#/c/13078/1/be/src/scheduling/admission-controller-test.cc File be/src/scheduling/admission-controller-test.cc: http://gerrit.cloudera.org:8080/#/c/13078/1/be/src/scheduling/admission-controller-test.cc@49 PS1, Line 49: string QUEUE_A = "root.queueA"; nit: static const or static constexpr http://gerrit.cloudera.org:8080/#/c/13078/1/be/src/scheduling/admission-controller-test.cc@86 PS1, Line 86: , bool is_coordinator = false, : bool is_executor = false unused args? http://gerrit.cloudera.org:8080/#/c/13078/1/be/src/scheduling/admission-controller-test.cc@109 PS1, Line 109: vales values http://gerrit.cloudera.org:8080/#/c/13078/1/be/src/scheduling/admission-controller-test.cc@115 PS1, Line 115: EXPECT_OK Maybe ASSERT_OK since the rest of assertions don't make sense if this fails. http://gerrit.cloudera.org:8080/#/c/13078/1/be/src/scheduling/admission-controller-test.cc@126 PS1, Line 126: /// Simple AdmissionController test. Maybe elaborate slightly on what it's testing? -- To view, visit http://gerrit.cloudera.org:8080/13078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a840590b868f2df1a06f3f397b7b0fc2b29462c Gerrit-Change-Number: 13078 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 22 Apr 2019 22:21:48 +0000 Gerrit-HasComments: Yes
