Alexey Serbin has posted comments on this change. Change subject: Implement an upgrade test ......................................................................
Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc File src/kudu/integration-tests/upgrade-test.cc: PS2, Line 18: #include <gflags/gflags.h> : #include <gtest/gtest.h> nit: consider listing these after STL includes, as recommended by the style guide . Line 22: #include <unordered_map> nit: is this really needed for the code below? Line 41: using std::unique_ptr; consider adding using std::vector and using std::string as well and then dropping the std:: prefix in all places using those classes. PS2, Line 53: if (FLAGS_test_version_a_bin.empty() || FLAGS_test_version_b_bin.empty()) { : LOG(FATAL) << "Must specify --test_version_a_bin and --test_version_b_bin flags"; : } nit: does it make sense to check for this first before running KuduTest::SetUp()? PS2, Line 58: std::vector<std::string>() nit: you could use c++11 features to shorten the signature, i.e. use '{}' instead of 'std::vector<std::string>()' for the default parameters. Line 104: Do you think it's worth verifying that the server with the same version is able to read what's has been written, prior to an attempt to read the data with the upgraded version? -- To view, visit http://gerrit.cloudera.org:8080/4424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
