[kudu-CR] [rest] add integration test

["Alexey Serbin (Code Review)" <ge...@cloudera.org>]

Alexey Serbin 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?


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 into std::unique_ptr()


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_CHECK_OK() instead of ASSERT_OK()?


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 that should be the actual work done in this test scenario after verifying the server returned code 200, no?


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 deletion -- via FetchURL?


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 success on DELETE operation for the table, the table is indeed deleted/dropped.


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 method returns the proper results if queried about the renamed table, right?



-- 
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 <ma...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Apr 2022 22:25:30 +0000
Gerrit-HasComments: Yes