Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/20524 )
Change subject: IMPALA-12426: Adds the backend InternalServer class. ...................................................................... Patch Set 20: (5 comments) http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc File be/src/service/internal-server-test.cc: http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@77 PS20, Line 77: ASSERT_FALSE(id_loc == string::npos); > Could this be ASSERT_NE(id_loc, string::npos)? Done http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@81 PS20, Line 81: for(size_t pos = id_loc; pos >= 0; pos--) { > Looks like this will still fail tidy and would never terminate. When pos == Good catch. I didn't realize size_t was unsigned. Fixed by making the condition pos > 0 http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@86 PS20, Line 86: ASSERT_TRUE(contents_str.find("\"state\": \"" + expected_state + "\"", pos) == pos); > ASSERT_EQ? Done http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@114 PS20, Line 114: EXPECT_OK(this->impala_server_->ExecuteIgnoreResults("impala", "create table " + > Use StrCat here. Fixed here and elsewhere throughout the test. http://gerrit.cloudera.org:8080/#/c/20524/20/be/src/service/internal-server-test.cc@456 PS20, Line 456: ASSERT_OK(server->SubmitQuery("select * from " + db_test->GetTableName() + > You might still get a warning from clang-tidy to use StrCat. Switch to using StrCat -- To view, visit http://gerrit.cloudera.org:8080/20524 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27686aa563fac87429657e4980b29b0da91eb9e1 Gerrit-Change-Number: 20524 Gerrit-PatchSet: 20 Gerrit-Owner: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Comment-Date: Tue, 05 Dec 2023 16:50:34 +0000 Gerrit-HasComments: Yes
