Khazar Mammadli has posted comments on this change. ( http://gerrit.cloudera.org:8080/17577 )
Change subject: [rest] add integration test ...................................................................... Patch Set 43: (7 comments) http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc File src/kudu/integration-tests/rest_server-itest.cc: http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@50 PS43, Line 50: CHECK_OK > Why not to use ASSERT_OK() here? The whole pre set up part I've taken from the c++ example on kudu usage, but have changed these Checks with Asserts as it fits the testing logic much better and going to upload soon http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@66 PS43, Line 66: KuduTableCreator* table_creator > In Kudu we tend not using raw pointers in the code: consider wrapping this Done http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@71 PS43, Line 71: KUDU_CHECK_OK > Here and above: would ASSERT_OK() work here? What's the reason using KUDU_ The whole pre set up part I've taken from the c++ example on kudu usage, but have changed these Checks with Asserts as it fits the testing logic much better and going to upload soon http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@115 PS43, Line 115: ASSERT_OK(curl.FetchURL(url, &response)); > So, how do we know that the web server returned proper results? I guess th I've expanded it further to double check the contents of the returned table. http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@124 PS43, Line 124: ASSERT_OK(curl.FetchURL(url, &response)); > Ah, that's interesting: why creation of the table done via PostToURL, but d As Creation of the table takes a JSON object as data, but in the case of the deletion the parameters are passed thru the parameter list(URL) and I couldn't find a way to make a post request with empty data through EasyCurl http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@125 PS43, Line 125: } > I think this test scenario should verify that after the server reported suc Done http://gerrit.cloudera.org:8080/#/c/17577/43/src/kudu/integration-tests/rest_server-itest.cc@134 PS43, Line 134: } > I guess after this the scenario should verify that GetKuduTableSchema REST Done -- To view, visit http://gerrit.cloudera.org:8080/17577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1b2900c67d601fa5a3a747d2475e1e86a6ad2e3 Gerrit-Change-Number: 17577 Gerrit-PatchSet: 43 Gerrit-Owner: Khazar Mammadli <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Khazar Mammadli <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 27 Apr 2022 14:40:25 +0000 Gerrit-HasComments: Yes
