Adar Dembo has posted comments on this change. Change subject: KUDU-1970: node density integration test ......................................................................
Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/6662/9/src/kudu/integration-tests/dense_node-itest.cc File src/kudu/integration-tests/dense_node-itest.cc: PS9, Line 71: > right, most are benchmarks among other tests or actual tests that test for I don't think a new convention is necessary, and FWIW I see enough overlap with existing tests called from benchmarks.sh. tablet_server-stress-test and full_stack-insert-scan-test, to name two. http://gerrit.cloudera.org:8080/#/c/6662/10/src/kudu/integration-tests/test_workload.cc File src/kudu/integration-tests/test_workload.cc: PS10, Line 233: // Do some sanity checks on the schema. They reflect how the rest of : // TestWorkload is going to use the schema. : CHECK_GT(schema_.num_columns(), 0) << "Schema should have at least one column"; : vector<int> key_indexes; : schema_.GetPrimaryKeyColumnIndexes(&key_indexes); : CHECK_EQ(1, key_indexes.size()) << "Schema should have just one key column"; : CHECK_EQ(0, key_indexes[0]) << "Schema's key column should be index 0"; : KuduColumnSchema key = schema_.Column(0); : CHECK_EQ("key", key.name()) << "Schema column should be named 'key'"; : CHECK_EQ(KuduColumnSchema::INT32, key.type()) << "Schema key column should be of type INT32"; > why not do this when the schema is set? we know that the simple schema abid Done -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
