From b32d571cf61eaa38b00895a0b52e8d18e606fe4c Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Thu, 5 Apr 2018 21:57:42 +0100 Subject: [PATCH 1/7] ReadRows integration test first draft --- .../client/testing/table_integration_test.cc | 10 ++ .../client/testing/table_integration_test.h | 2 + bigtable/tests/data_integration_test.cc | 114 ++++++++++++++++++ 3 files changed, 126 insertions(+) diff --git a/bigtable/client/testing/table_integration_test.cc b/bigtable/client/testing/table_integration_test.cc index 5f9168cd7d2f1..baf0f92cc3155 100644 --- a/bigtable/client/testing/table_integration_test.cc +++ b/bigtable/client/testing/table_integration_test.cc @@ -71,6 +71,16 @@ std::vector TableIntegrationTest::ReadRows( return result; } +std::vector TableIntegrationTest::MoveCellsFromReader( + bigtable::RowReader& reader) { + std::vector result; + for (auto const& row : reader) { + std::copy(row.cells().begin(), row.cells().end(), + std::back_inserter(result)); + } + return result; +} + /// A helper function to create a list of cells. void TableIntegrationTest::CreateCells( bigtable::Table& table, std::vector const& cells) { diff --git a/bigtable/client/testing/table_integration_test.h b/bigtable/client/testing/table_integration_test.h index 5be7432754f25..94d16a4f73964 100644 --- a/bigtable/client/testing/table_integration_test.h +++ b/bigtable/client/testing/table_integration_test.h @@ -65,6 +65,8 @@ class TableIntegrationTest : public ::testing::Test { std::int64_t rows_limit, bigtable::Filter filter); + std::vector MoveCellsFromReader(bigtable::RowReader& reader); + /// Return all the cells in @p table that pass @p filter. void CreateCells(bigtable::Table& table, std::vector const& cells); diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index 419629e782574..8b4f33976c696 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -211,6 +211,120 @@ TEST_F(DataIntegrationTest, TableReadRowNotExistTest) { EXPECT_FALSE(row_cell.first); } +TEST_F(DataIntegrationTest, TableReadRowsAllRows) { + std::string const table_name = "table-read-rows-all-rows"; + auto table = CreateTable(table_name, table_config); + std::string const row_key1 = "row-key-1"; + std::string const row_key2 = "row-key-2"; + std::string const row_key3(1024, '3'); // a long key + std::string const long_value(1024, 'v'); // a long value + + std::vector created{ + {row_key1, family, "c1", 1000, "data1", {}}, + {row_key1, family, "c2", 1000, "data2", {}}, + {row_key2, family, "c1", 1000, "", {}}, + {row_key3, family, "c1", 1000, long_value, {}}}; + + CreateCells(*table, created); + + // Some equivalent ways to read the three rows + auto read1 = + table->ReadRows(bigtable::RowSet(bigtable::RowRange::InfiniteRange()), + bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(created, MoveCellsFromReader(read1)); + + auto read2 = + table->ReadRows(bigtable::RowSet(bigtable::RowRange::InfiniteRange()), 3, + bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(created, MoveCellsFromReader(read2)); + + auto read3 = table->ReadRows( + bigtable::RowSet(bigtable::RowRange::InfiniteRange()), + bigtable::RowReader::NO_ROWS_LIMIT, bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(created, MoveCellsFromReader(read3)); + + DeleteTable(table_name); +} + +TEST_F(DataIntegrationTest, TableReadRowsPartialRows) { + std::string const table_name = "table-read-rows-partial-rows"; + auto table = CreateTable(table_name, table_config); + std::string const row_key1 = "row-key-1"; + std::string const row_key2 = "row-key-2"; + std::string const row_key3 = "row-key-3"; + + std::vector created{ + {row_key1, family, "c1", 1000, "data1", {}}, + {row_key1, family, "c2", 1000, "data2", {}}, + {row_key2, family, "c1", 1000, "data3", {}}, + {row_key3, family, "c1", 1000, "data4", {}}}; + + CreateCells(*table, created); + + std::vector expected{ + {row_key1, family, "c1", 1000, "data1", {}}, + {row_key1, family, "c2", 1000, "data2", {}}, + {row_key2, family, "c1", 1000, "data3", {}}}; + + // Some equivalent ways of reading just the first two rows + auto read1 = + table->ReadRows(bigtable::RowSet(bigtable::RowRange::InfiniteRange()), 2, + bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(expected, MoveCellsFromReader(read1)); + + bigtable::RowSet rs2; + rs2.Append(row_key1); + rs2.Append(row_key2); + auto read2 = + table->ReadRows(std::move(rs2), bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(expected, MoveCellsFromReader(read2)); + + bigtable::RowSet rs3(bigtable::RowRange::Closed(row_key1, row_key2)); + auto read3 = + table->ReadRows(std::move(rs3), bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(expected, MoveCellsFromReader(read3)); + + DeleteTable(table_name); +} + +TEST_F(DataIntegrationTest, TableReadRowsNoRows) { + std::string const table_name = "table-read-rows-no-rows"; + auto table = CreateTable(table_name, table_config); + std::string const row_key1 = "row-key-1"; + std::string const row_key2 = "row-key-2"; + std::string const row_key3 = "row-key-3"; + + std::vector created{ + {row_key1, family, "c1", 1000, "data1", {}}, + {row_key3, family, "c1", 1000, "data2", {}}}; + + CreateCells(*table, created); + + std::vector expected; // empty + + // read nonexistent rows + auto read1 = table->ReadRows(bigtable::RowSet(row_key2), + bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(expected, MoveCellsFromReader(read1)); + + auto read2 = + table->ReadRows(bigtable::RowSet(bigtable::RowRange::Prefix(row_key2)), + bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(expected, MoveCellsFromReader(read2)); + + DeleteTable(table_name); +} + +TEST_F(DataIntegrationTest, TableReadRowsWrongTable) { + std::string const table_name = "table-read-rows-wrong-table"; + bigtable::Table table(data_client_, table_name); + + auto read1 = + table.ReadRows(bigtable::RowSet(bigtable::RowRange::InfiniteRange()), + bigtable::Filter::PassAllFilter()); + EXPECT_THROW(read1.begin(), std::exception); +} + TEST_F(DataIntegrationTest, TableCheckAndMutateRowPass) { std::string const table_name = "table-check-and-mutate-row-pass"; auto table = CreateTable(table_name, table_config); From b96888167f524f6f32effd16823f8fccaa82eaf3 Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Fri, 6 Apr 2018 11:09:01 +0100 Subject: [PATCH 2/7] Review iteration. --- .../client/testing/table_integration_test.cc | 3 ++- bigtable/tests/data_integration_test.cc | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/bigtable/client/testing/table_integration_test.cc b/bigtable/client/testing/table_integration_test.cc index baf0f92cc3155..ea229f604a308 100644 --- a/bigtable/client/testing/table_integration_test.cc +++ b/bigtable/client/testing/table_integration_test.cc @@ -15,6 +15,7 @@ #include "bigtable/client/testing/table_integration_test.h" #include "bigtable/client/internal/make_unique.h" #include +#include #include namespace bigtable { @@ -75,7 +76,7 @@ std::vector TableIntegrationTest::MoveCellsFromReader( bigtable::RowReader& reader) { std::vector result; for (auto const& row : reader) { - std::copy(row.cells().begin(), row.cells().end(), + std::move(row.cells().begin(), row.cells().end(), std::back_inserter(result)); } return result; diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index 8b4f33976c696..6d6f210be2b9d 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -40,6 +40,10 @@ class DataIntegrationTest : public bigtable::testing::TableIntegrationTest { {}); }; +bool UsingCloudBigtableEmulator() { + return std::getenv("BIGTABLE_EMULATOR_HOST") != nullptr; +} + } // anonymous namespace int main(int argc, char* argv[]) { @@ -243,6 +247,12 @@ TEST_F(DataIntegrationTest, TableReadRowsAllRows) { bigtable::RowReader::NO_ROWS_LIMIT, bigtable::Filter::PassAllFilter()); CheckEqualUnordered(created, MoveCellsFromReader(read3)); + if (!UsingCloudBigtableEmulator()) { + // TODO(#151) - remove workarounds for emulator bug(s). + auto read4 = + table->ReadRows(bigtable::RowSet(), bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(created, MoveCellsFromReader(read4)); + } DeleteTable(table_name); } @@ -312,6 +322,8 @@ TEST_F(DataIntegrationTest, TableReadRowsNoRows) { bigtable::Filter::PassAllFilter()); CheckEqualUnordered(expected, MoveCellsFromReader(read2)); + // TODO(#404): also call with bigtable::RowSet(bigtable::RowRange::Empty()) + DeleteTable(table_name); } @@ -322,7 +334,11 @@ TEST_F(DataIntegrationTest, TableReadRowsWrongTable) { auto read1 = table.ReadRows(bigtable::RowSet(bigtable::RowRange::InfiniteRange()), bigtable::Filter::PassAllFilter()); - EXPECT_THROW(read1.begin(), std::exception); +#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS + EXPECT_THROW(read1.begin(), std::runtime_error); +#else + EXPECT_DEATH_IF_SUPPORTED(read1.begin(), "exceptions are disabled"); +#endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS } TEST_F(DataIntegrationTest, TableCheckAndMutateRowPass) { From f1f0b53b7e025d0df497c3a51b51c03fecd07cd6 Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Wed, 18 Apr 2018 11:16:29 +0100 Subject: [PATCH 3/7] Remove the death test. --- bigtable/tests/data_integration_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index 6d6f210be2b9d..1e6d8586248e0 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -336,8 +336,6 @@ TEST_F(DataIntegrationTest, TableReadRowsWrongTable) { bigtable::Filter::PassAllFilter()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS EXPECT_THROW(read1.begin(), std::runtime_error); -#else - EXPECT_DEATH_IF_SUPPORTED(read1.begin(), "exceptions are disabled"); #endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS } From d290fc7dec36b99e6a2e359810e584246483b56f Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Fri, 20 Apr 2018 15:15:46 +0100 Subject: [PATCH 4/7] Add a call with a row set initialized from the empty range. --- bigtable/tests/data_integration_test.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index 9a9fdaa33147f..a365beb931282 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -323,7 +323,10 @@ TEST_F(DataIntegrationTest, TableReadRowsNoRows) { bigtable::Filter::PassAllFilter()); CheckEqualUnordered(expected, MoveCellsFromReader(read2)); - // TODO(#404): also call with bigtable::RowSet(bigtable::RowRange::Empty()) + auto read3 = + table->ReadRows(bigtable::RowSet(bigtable::RowRange::Empty()), + bigtable::Filter::PassAllFilter()); + CheckEqualUnordered(expected, MoveCellsFromReader(read3)); DeleteTable(table_name); } From 794621cf188423c0a4aa906553e5fb8b48988b9b Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Fri, 20 Apr 2018 15:40:39 +0100 Subject: [PATCH 5/7] Set raise_on_error to false in noex::Table --- bigtable/client/internal/table.cc | 4 ++-- bigtable/tests/data_integration_test.cc | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bigtable/client/internal/table.cc b/bigtable/client/internal/table.cc index 03aa391db3df4..22921806f3dfc 100644 --- a/bigtable/client/internal/table.cc +++ b/bigtable/client/internal/table.cc @@ -124,7 +124,7 @@ RowReader Table::ReadRows(RowSet row_set, Filter filter, bool raise_on_error) { metadata_update_policy_, bigtable::internal::make_unique< bigtable::internal::ReadRowsParserFactory>(), - raise_on_error); + false); } RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit, @@ -134,7 +134,7 @@ RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit, rpc_backoff_policy_->clone(), metadata_update_policy_, bigtable::internal::make_unique< bigtable::internal::ReadRowsParserFactory>(), - raise_on_error); + false); } std::pair Table::ReadRow(std::string row_key, Filter filter, diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index a365beb931282..4a8648e1cb2ff 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -340,6 +340,10 @@ TEST_F(DataIntegrationTest, TableReadRowsWrongTable) { bigtable::Filter::PassAllFilter()); #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS EXPECT_THROW(read1.begin(), std::runtime_error); +#else + EXPECT_EQ(read1.begin(), read1.end()); + grpc::Status status1 = read1.Finish(); + EXPECT_FALSE(status1.ok()); #endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS } From e5ede9ae9975fa6cb37364680a0803924aae2eec Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Fri, 20 Apr 2018 15:44:00 +0100 Subject: [PATCH 6/7] (formatting) --- bigtable/tests/data_integration_test.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index 4a8648e1cb2ff..f531bb1463cd3 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -323,9 +323,8 @@ TEST_F(DataIntegrationTest, TableReadRowsNoRows) { bigtable::Filter::PassAllFilter()); CheckEqualUnordered(expected, MoveCellsFromReader(read2)); - auto read3 = - table->ReadRows(bigtable::RowSet(bigtable::RowRange::Empty()), - bigtable::Filter::PassAllFilter()); + auto read3 = table->ReadRows(bigtable::RowSet(bigtable::RowRange::Empty()), + bigtable::Filter::PassAllFilter()); CheckEqualUnordered(expected, MoveCellsFromReader(read3)); DeleteTable(table_name); From f9a2fc30c17ace63ee98fb823e94f33f6147f34e Mon Sep 17 00:00:00 2001 From: Daniel Mahu Date: Fri, 20 Apr 2018 21:10:17 +0100 Subject: [PATCH 7/7] Revert changes to raise_on_error and use noex::Table instead --- bigtable/client/internal/table.cc | 4 ++-- bigtable/client/testing/inprocess_admin_client.h | 2 +- bigtable/client/testing/inprocess_data_client.h | 2 +- bigtable/tests/data_integration_test.cc | 6 ++++++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bigtable/client/internal/table.cc b/bigtable/client/internal/table.cc index 22921806f3dfc..03aa391db3df4 100644 --- a/bigtable/client/internal/table.cc +++ b/bigtable/client/internal/table.cc @@ -124,7 +124,7 @@ RowReader Table::ReadRows(RowSet row_set, Filter filter, bool raise_on_error) { metadata_update_policy_, bigtable::internal::make_unique< bigtable::internal::ReadRowsParserFactory>(), - false); + raise_on_error); } RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit, @@ -134,7 +134,7 @@ RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit, rpc_backoff_policy_->clone(), metadata_update_policy_, bigtable::internal::make_unique< bigtable::internal::ReadRowsParserFactory>(), - false); + raise_on_error); } std::pair Table::ReadRow(std::string row_key, Filter filter, diff --git a/bigtable/client/testing/inprocess_admin_client.h b/bigtable/client/testing/inprocess_admin_client.h index e8ccf450128f8..6a75c595a0bd9 100644 --- a/bigtable/client/testing/inprocess_admin_client.h +++ b/bigtable/client/testing/inprocess_admin_client.h @@ -52,7 +52,7 @@ class InProcessAdminClient : public bigtable::AdminClient { std::shared_ptr channel_; }; -} // namespace BIGTABLE_CLIENT_NS +} // namespace testing } // namespace bigtable #endif // GOOGLE_CLOUD_CPP_BIGTABLE_CLIENT_TESTING_INPROCESS_ADMIN_CLIENT_H_ diff --git a/bigtable/client/testing/inprocess_data_client.h b/bigtable/client/testing/inprocess_data_client.h index 3e73cede7c905..26cf4aaf1a52f 100644 --- a/bigtable/client/testing/inprocess_data_client.h +++ b/bigtable/client/testing/inprocess_data_client.h @@ -57,7 +57,7 @@ class InProcessDataClient : public bigtable::DataClient { std::shared_ptr channel_; }; -} // namespace BIGTABLE_CLIENT_NS +} // namespace testing } // namespace bigtable #endif // GOOGLE_CLOUD_CPP_BIGTABLE_CLIENT_TESTING_INPROCESS_DATA_CLIENT_H_ diff --git a/bigtable/tests/data_integration_test.cc b/bigtable/tests/data_integration_test.cc index f531bb1463cd3..a8431404c2816 100644 --- a/bigtable/tests/data_integration_test.cc +++ b/bigtable/tests/data_integration_test.cc @@ -332,11 +332,17 @@ TEST_F(DataIntegrationTest, TableReadRowsNoRows) { TEST_F(DataIntegrationTest, TableReadRowsWrongTable) { std::string const table_name = "table-read-rows-wrong-table"; + +#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS bigtable::Table table(data_client_, table_name); +#else + bigtable::noex::Table table(data_client_, table_name); +#endif auto read1 = table.ReadRows(bigtable::RowSet(bigtable::RowRange::InfiniteRange()), bigtable::Filter::PassAllFilter()); + #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS EXPECT_THROW(read1.begin(), std::runtime_error); #else