Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@dbolduc
dbolduc committed Aug 3, 2022
1 parent 9bbccd9 commit 515c2d8
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 28 deletions.
32 changes: 16 additions & 16 deletions google/cloud/bigtable/legacy_table_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,13 @@ TEST_F(ValidContextMdAsyncTest, AsyncReadModifyWriteRow) {
TEST_F(TableTest, ApplyWithOptions) {
Table table(client_, kTableId);
auto status = table.Apply(SingleRowMutation("row"), NonEmptyOptions());
EXPECT_THAT(status, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(status, StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, AsyncApplyWithOptions) {
Table table(client_, kTableId);
auto status = table.AsyncApply(SingleRowMutation("row"), NonEmptyOptions());
EXPECT_THAT(status.get(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(status.get(), StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, BulkApplyWithOptions) {
Expand All @@ -285,9 +285,9 @@ TEST_F(TableTest, BulkApplyWithOptions) {
auto failed = table.BulkApply(bulk, NonEmptyOptions());
ASSERT_EQ(2, failed.size());
EXPECT_EQ(0, failed[0].original_index());
EXPECT_THAT(failed[0].status(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(failed[0].status(), StatusIs(StatusCode::kInvalidArgument));
EXPECT_EQ(1, failed[1].original_index());
EXPECT_THAT(failed[1].status(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(failed[1].status(), StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, AsyncBulkApplyWithOptions) {
Expand All @@ -296,9 +296,9 @@ TEST_F(TableTest, AsyncBulkApplyWithOptions) {
auto failed = table.AsyncBulkApply(bulk, NonEmptyOptions()).get();
ASSERT_EQ(2, failed.size());
EXPECT_EQ(0, failed[0].original_index());
EXPECT_THAT(failed[0].status(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(failed[0].status(), StatusIs(StatusCode::kInvalidArgument));
EXPECT_EQ(1, failed[1].original_index());
EXPECT_THAT(failed[1].status(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(failed[1].status(), StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, ReadRowsWithOptions) {
Expand All @@ -307,7 +307,7 @@ TEST_F(TableTest, ReadRowsWithOptions) {
table.ReadRows(RowSet(), Filter::PassAllFilter(), NonEmptyOptions());
auto it = reader.begin();
EXPECT_NE(it, reader.end());
EXPECT_THAT(*it, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(*it, StatusIs(StatusCode::kInvalidArgument));
EXPECT_EQ(++it, reader.end());
}

Expand All @@ -317,47 +317,47 @@ TEST_F(TableTest, ReadRowsWithLimitWithOptions) {
table.ReadRows(RowSet(), 1, Filter::PassAllFilter(), NonEmptyOptions());
auto it = reader.begin();
EXPECT_NE(it, reader.end());
EXPECT_THAT(*it, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(*it, StatusIs(StatusCode::kInvalidArgument));
EXPECT_EQ(++it, reader.end());
}

TEST_F(TableTest, ReadRowWithOptions) {
Table table(client_, kTableId);
auto row = table.ReadRow("row", Filter::PassAllFilter(), NonEmptyOptions());
EXPECT_THAT(row, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(row, StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, AsyncReadRowWithOptions) {
Table table(client_, kTableId);
auto row =
table.AsyncReadRow("row", Filter::PassAllFilter(), NonEmptyOptions());
EXPECT_THAT(row.get(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(row.get(), StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, CheckAndMutateRowWithOptions) {
Table table(client_, kTableId);
auto branch = table.CheckAndMutateRow("row", Filter::PassAllFilter(), {}, {},
NonEmptyOptions());
EXPECT_THAT(branch, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(branch, StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, AsyncCheckAndMutateRowWithOptions) {
Table table(client_, kTableId);
auto branch = table.AsyncCheckAndMutateRow("row", Filter::PassAllFilter(), {},
{}, NonEmptyOptions());
EXPECT_THAT(branch.get(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(branch.get(), StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, SampleRowsWithOptions) {
Table table(client_, kTableId);
auto rows = table.SampleRows(NonEmptyOptions());
EXPECT_THAT(rows, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(rows, StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, AsyncSampleRowsWithOptions) {
Table table(client_, kTableId);
auto rows = table.AsyncSampleRows(NonEmptyOptions());
EXPECT_THAT(rows.get(), StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(rows.get(), StatusIs(StatusCode::kInvalidArgument));
}

TEST_F(TableTest, AsyncReadRowsWithOptions) {
Expand All @@ -366,7 +366,7 @@ TEST_F(TableTest, AsyncReadRowsWithOptions) {

MockFunction<void(Status)> on_finish;
EXPECT_CALL(on_finish, Call).WillOnce([](Status const& status) {
EXPECT_THAT(status, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(status, StatusIs(StatusCode::kInvalidArgument));
});

Table table(client_, kTableId);
Expand All @@ -380,7 +380,7 @@ TEST_F(TableTest, AsyncReadRowsWithLimitWithOptions) {

MockFunction<void(Status)> on_finish;
EXPECT_CALL(on_finish, Call).WillOnce([](Status const& status) {
EXPECT_THAT(status, StatusIs(StatusCode::kFailedPrecondition));
EXPECT_THAT(status, StatusIs(StatusCode::kInvalidArgument));
});

Table table(client_, kTableId);
Expand Down
22 changes: 11 additions & 11 deletions google/cloud/bigtable/table.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Status Table::Apply(SingleRowMutation mut, Options opts) {
return connection_->Apply(table_name_, std::move(mut));
}
if (!google::cloud::internal::IsEmpty(opts)) {
return Status(StatusCode::kFailedPrecondition,
return Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`.");
}
Expand Down Expand Up @@ -107,7 +107,7 @@ future<Status> Table::AsyncApply(SingleRowMutation mut, Options opts) {
}
if (!google::cloud::internal::IsEmpty(opts)) {
return make_ready_future(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."));
}
Expand Down Expand Up @@ -157,7 +157,7 @@ std::vector<FailedMutation> Table::BulkApply(BulkMutation mut, Options opts) {
}
if (!google::cloud::internal::IsEmpty(opts)) {
return bigtable_internal::MakeFailedMutations(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."),
mut.size());
Expand Down Expand Up @@ -199,7 +199,7 @@ future<std::vector<FailedMutation>> Table::AsyncBulkApply(BulkMutation mut,
}
if (!google::cloud::internal::IsEmpty(opts)) {
return make_ready_future(bigtable_internal::MakeFailedMutations(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."),
mut.size()));
Expand Down Expand Up @@ -228,7 +228,7 @@ RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit,
if (!google::cloud::internal::IsEmpty(opts)) {
return MakeRowReader(
std::make_shared<bigtable_internal::StatusOnlyRowReader>(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`.")));
}
Expand All @@ -249,7 +249,7 @@ StatusOr<std::pair<bool, Row>> Table::ReadRow(std::string row_key,
std::move(filter));
}
if (!google::cloud::internal::IsEmpty(opts)) {
return Status(StatusCode::kFailedPrecondition,
return Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`.");
}
Expand Down Expand Up @@ -285,7 +285,7 @@ StatusOr<MutationBranch> Table::CheckAndMutateRow(
std::move(true_mutations), std::move(false_mutations));
}
if (!google::cloud::internal::IsEmpty(opts)) {
return Status(StatusCode::kFailedPrecondition,
return Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`.");
}
Expand Down Expand Up @@ -328,7 +328,7 @@ future<StatusOr<MutationBranch>> Table::AsyncCheckAndMutateRow(
}
if (!google::cloud::internal::IsEmpty(opts)) {
return make_ready_future<StatusOr<MutationBranch>>(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."));
}
Expand Down Expand Up @@ -385,7 +385,7 @@ StatusOr<std::vector<bigtable::RowKeySample>> Table::SampleRows(Options opts) {
return connection_->SampleRows(table_name_);
}
if (!google::cloud::internal::IsEmpty(opts)) {
return Status(StatusCode::kFailedPrecondition,
return Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`.");
}
Expand Down Expand Up @@ -438,7 +438,7 @@ future<StatusOr<std::vector<bigtable::RowKeySample>>> Table::AsyncSampleRows(
}
if (!google::cloud::internal::IsEmpty(opts)) {
return make_ready_future<StatusOr<std::vector<RowKeySample>>>(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."));
}
Expand Down Expand Up @@ -514,7 +514,7 @@ future<StatusOr<std::pair<bool, Row>>> Table::AsyncReadRow(std::string row_key,
}
if (!google::cloud::internal::IsEmpty(opts)) {
return make_ready_future<StatusOr<std::pair<bool, Row>>>(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."));
}
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/bigtable/table.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ class Table {
}
if (!google::cloud::internal::IsEmpty(opts)) {
on_finish_fn(
Status(StatusCode::kFailedPrecondition,
Status(StatusCode::kInvalidArgument,
"Per-operation options only apply to `Table`s constructed "
"with a `DataConnection`."));
return;
Expand Down

0 comments on commit 515c2d8

Please sign in to comment.