Skip to content

Commit db1ef0e

Browse files
dbolducdbolduc
authored
refactor(bigtable): consolidate mock policies (#8038)
1 parent 6a6d2ac commit db1ef0e

File tree

8 files changed

+110
-131
lines changed

8 files changed

+110
-131
lines changed

‎google/cloud/bigtable/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,10 @@ if (BUILD_TESTING)
253253
testing/inprocess_data_client.h
254254
testing/mock_admin_client.h
255255
testing/mock_async_failing_rpc_factory.h
256-
testing/mock_backoff_policy.h
257256
testing/mock_data_client.h
258257
testing/mock_instance_admin_client.h
259258
testing/mock_mutate_rows_reader.h
259+
testing/mock_policies.h
260260
testing/mock_read_rows_reader.h
261261
testing/mock_response_reader.h
262262
testing/mock_sample_row_keys_reader.h

‎google/cloud/bigtable/bigtable_client_testing.bzl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ bigtable_client_testing_hdrs = [
2323
"testing/inprocess_data_client.h",
2424
"testing/mock_admin_client.h",
2525
"testing/mock_async_failing_rpc_factory.h",
26-
"testing/mock_backoff_policy.h",
2726
"testing/mock_data_client.h",
2827
"testing/mock_instance_admin_client.h",
2928
"testing/mock_mutate_rows_reader.h",
29+
"testing/mock_policies.h",
3030
"testing/mock_read_rows_reader.h",
3131
"testing/mock_response_reader.h",
3232
"testing/mock_sample_row_keys_reader.h",

‎google/cloud/bigtable/internal/async_bulk_apply_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/bigtable/internal/async_bulk_apply.h"
16-
#include "google/cloud/bigtable/testing/mock_backoff_policy.h"
1716
#include "google/cloud/bigtable/testing/mock_mutate_rows_reader.h"
17+
#include "google/cloud/bigtable/testing/mock_policies.h"
1818
#include "google/cloud/bigtable/testing/table_test_fixture.h"
1919
#include "google/cloud/future.h"
2020
#include "google/cloud/internal/api_client_header.h"

‎google/cloud/bigtable/internal/async_row_sampler_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/bigtable/internal/async_row_sampler.h"
16-
#include "google/cloud/bigtable/testing/mock_backoff_policy.h"
16+
#include "google/cloud/bigtable/testing/mock_policies.h"
1717
#include "google/cloud/bigtable/testing/mock_response_reader.h"
1818
#include "google/cloud/bigtable/testing/table_test_fixture.h"
1919
#include "google/cloud/testing_util/fake_completion_queue_impl.h"

‎google/cloud/bigtable/internal/convert_policies_test.cc‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "google/cloud/bigtable/internal/convert_policies.h"
1616
#include "google/cloud/bigtable/admin/bigtable_instance_admin_options.h"
1717
#include "google/cloud/bigtable/admin/bigtable_table_admin_options.h"
18+
#include "google/cloud/bigtable/testing/mock_policies.h"
1819
#include "google/cloud/grpc_options.h"
1920
#include <gmock/gmock.h>
2021

@@ -24,6 +25,10 @@ namespace bigtable {
2425
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
2526
namespace {
2627

28+
using ::google::cloud::bigtable::testing::MockBackoffPolicy;
29+
using ::google::cloud::bigtable::testing::MockPollingPolicy;
30+
using ::google::cloud::bigtable::testing::MockRetryPolicy;
31+
2732
TEST(ConvertPolicies, InstanceAdmin) {
2833
auto r = DefaultRPCRetryPolicy(internal::kBigtableInstanceAdminLimits);
2934
auto b = DefaultRPCBackoffPolicy(internal::kBigtableInstanceAdminLimits);
@@ -56,36 +61,6 @@ TEST(ConvertPolicies, TableAdmin) {
5661
options.has<bigtable_admin::BigtableTableAdminPollingPolicyOption>());
5762
}
5863

59-
class MockRetryPolicy : public RPCRetryPolicy {
60-
public:
61-
MOCK_METHOD(std::unique_ptr<RPCRetryPolicy>, clone, (), (const, override));
62-
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (const, override));
63-
MOCK_METHOD(bool, OnFailure, (grpc::Status const& status), (override));
64-
MOCK_METHOD(bool, OnFailure, (Status const& status), (override));
65-
};
66-
67-
class MockBackoffPolicy : public RPCBackoffPolicy {
68-
public:
69-
MOCK_METHOD(std::unique_ptr<RPCBackoffPolicy>, clone, (), (const, override));
70-
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (const, override));
71-
MOCK_METHOD(std::chrono::milliseconds, OnCompletion, (Status const&),
72-
(override));
73-
MOCK_METHOD(std::chrono::milliseconds, OnCompletion, (grpc::Status const&),
74-
(override));
75-
};
76-
77-
class MockPollingPolicy : public PollingPolicy {
78-
public:
79-
MOCK_METHOD(std::unique_ptr<PollingPolicy>, clone, (), (const));
80-
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (override));
81-
// MOCK_METHOD(bool, IsPermanentError, (grpc::Status const&), (override));
82-
MOCK_METHOD(bool, IsPermanentError, (Status const&), (override));
83-
// MOCK_METHOD(bool, OnFailure, (grpc::Status const&), (override));
84-
MOCK_METHOD(bool, OnFailure, (Status const&), (override));
85-
MOCK_METHOD(bool, Exhausted, (), (override));
86-
MOCK_METHOD(std::chrono::milliseconds, WaitPeriod, (), (override));
87-
};
88-
8964
/**
9065
* This test converts policies into options, then invokes the `GrpcSetupOption`
9166
* twice. It checks that:

‎google/cloud/bigtable/row_reader_test.cc‎

Lines changed: 35 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "google/cloud/bigtable/row_reader.h"
1616
#include "google/cloud/bigtable/table.h"
17+
#include "google/cloud/bigtable/testing/mock_policies.h"
1718
#include "google/cloud/bigtable/testing/mock_read_rows_reader.h"
1819
#include "google/cloud/bigtable/testing/table_test_fixture.h"
1920
#include "google/cloud/internal/api_client_header.h"
@@ -35,14 +36,18 @@ namespace {
3536
using ::google::bigtable::v2::ReadRowsRequest;
3637
using ::google::bigtable::v2::ReadRowsResponse_CellChunk;
3738
using ::google::cloud::bigtable::Row;
39+
using ::google::cloud::bigtable::testing::MockBackoffPolicy;
3840
using ::google::cloud::bigtable::testing::MockReadRowsReader;
41+
using ::google::cloud::bigtable::testing::MockRetryPolicy;
3942
using ::google::cloud::testing_util::IsContextMDValid;
4043
using ::testing::_;
44+
using ::testing::An;
4145
using ::testing::Contains;
4246
using ::testing::DoAll;
4347
using ::testing::Eq;
4448
using ::testing::HasSubstr;
4549
using ::testing::Matcher;
50+
using ::testing::NiceMock;
4651
using ::testing::Not;
4752
using ::testing::Property;
4853
using ::testing::Return;
@@ -105,36 +110,6 @@ class ReadRowsParserMockFactory
105110
std::deque<ParserPtr> parsers_;
106111
};
107112

108-
class RetryPolicyMock : public bigtable::RPCRetryPolicy {
109-
public:
110-
RetryPolicyMock() = default;
111-
std::unique_ptr<RPCRetryPolicy> clone() const override {
112-
google::cloud::internal::ThrowRuntimeError("Mocks cannot be copied.");
113-
}
114-
115-
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (const, override));
116-
MOCK_METHOD(bool, OnFailure, (grpc::Status const& status), (override));
117-
bool OnFailure(google::cloud::Status const&) override { return true; }
118-
};
119-
120-
class BackoffPolicyMock : public bigtable::RPCBackoffPolicy {
121-
public:
122-
BackoffPolicyMock() = default;
123-
std::unique_ptr<RPCBackoffPolicy> clone() const override {
124-
google::cloud::internal::ThrowRuntimeError("Mocks cannot be copied.");
125-
}
126-
void Setup(grpc::ClientContext&) const override {}
127-
MOCK_METHOD(std::chrono::milliseconds, OnCompletionHook,
128-
(grpc::Status const& s));
129-
std::chrono::milliseconds OnCompletion(grpc::Status const& s) override {
130-
return OnCompletionHook(s);
131-
}
132-
std::chrono::milliseconds OnCompletion(
133-
google::cloud::Status const&) override {
134-
return std::chrono::milliseconds(0);
135-
}
136-
};
137-
138113
// Match the number of expected row keys in a request in EXPECT_CALL
139114
Matcher<ReadRowsRequest const&> RequestWithRowKeysCount(int n) {
140115
return Property(
@@ -151,14 +126,14 @@ class RowReaderTest : public bigtable::testing::TableTestFixture {
151126
public:
152127
RowReaderTest()
153128
: TableTestFixture(CompletionQueue{}),
154-
retry_policy_(new RetryPolicyMock),
155-
backoff_policy_(new BackoffPolicyMock),
129+
retry_policy_(absl::make_unique<NiceMock<MockRetryPolicy>>()),
130+
backoff_policy_(absl::make_unique<NiceMock<MockBackoffPolicy>>()),
156131
metadata_update_policy_(kTableName,
157132
bigtable::MetadataParamTypes::TABLE_NAME),
158133
parser_factory_(new ReadRowsParserMockFactory) {}
159134

160-
std::unique_ptr<RetryPolicyMock> retry_policy_;
161-
std::unique_ptr<BackoffPolicyMock> backoff_policy_;
135+
std::unique_ptr<MockRetryPolicy> retry_policy_;
136+
std::unique_ptr<MockBackoffPolicy> backoff_policy_;
162137
bigtable::MetadataUpdatePolicy metadata_update_policy_;
163138
std::unique_ptr<ReadRowsParserMockFactory> parser_factory_;
164139
};
@@ -316,8 +291,9 @@ TEST_F(RowReaderTest, FailedStreamIsRetried) {
316291
EXPECT_CALL(*stream, Finish())
317292
.WillOnce(Return(grpc::Status(grpc::StatusCode::INTERNAL, "retry")));
318293

319-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(true));
320-
EXPECT_CALL(*backoff_policy_, OnCompletionHook)
294+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
295+
.WillOnce(Return(true));
296+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
321297
.WillOnce(Return(std::chrono::milliseconds(0)));
322298

323299
// the stub will free it
@@ -354,8 +330,10 @@ TEST_F(RowReaderTest, FailedStreamWithNoRetryThrowsNoExcept) {
354330
EXPECT_CALL(*stream, Finish())
355331
.WillOnce(Return(grpc::Status(grpc::StatusCode::INTERNAL, "retry")));
356332

357-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(false));
358-
EXPECT_CALL(*backoff_policy_, OnCompletionHook).Times(0);
333+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
334+
.WillOnce(Return(false));
335+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
336+
.Times(0);
359337
}
360338

361339
parser_factory_->AddParser(std::move(parser));
@@ -386,8 +364,9 @@ TEST_F(RowReaderTest, FailedStreamRetriesSkipAlreadyReadRows) {
386364
EXPECT_CALL(*stream, Finish())
387365
.WillOnce(Return(grpc::Status(grpc::StatusCode::INTERNAL, "retry")));
388366

389-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(true));
390-
EXPECT_CALL(*backoff_policy_, OnCompletionHook)
367+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
368+
.WillOnce(Return(true));
369+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
391370
.WillOnce(Return(std::chrono::milliseconds(0)));
392371

393372
// the stub will free it
@@ -429,8 +408,9 @@ TEST_F(RowReaderTest, FailedParseIsRetried) {
429408
.WillOnce(SetArgReferee<1>(
430409
grpc::Status(grpc::StatusCode::INTERNAL, "parser exception")));
431410

432-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(true));
433-
EXPECT_CALL(*backoff_policy_, OnCompletionHook)
411+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
412+
.WillOnce(Return(true));
413+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
434414
.WillOnce(Return(std::chrono::milliseconds(0)));
435415

436416
// the stub will free it
@@ -475,8 +455,9 @@ TEST_F(RowReaderTest, FailedParseRetriesSkipAlreadyReadRows) {
475455
.WillOnce(SetArgReferee<0>(
476456
grpc::Status(grpc::StatusCode::INTERNAL, "InternalError")));
477457

478-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(true));
479-
EXPECT_CALL(*backoff_policy_, OnCompletionHook)
458+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
459+
.WillOnce(Return(true));
460+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
480461
.WillOnce(Return(std::chrono::milliseconds(0)));
481462

482463
// the stub will free it
@@ -517,8 +498,10 @@ TEST_F(RowReaderTest, FailedParseWithNoRetryThrowsNoExcept) {
517498
EXPECT_CALL(*parser, HandleEndOfStreamHook)
518499
.WillOnce(SetArgReferee<0>(
519500
grpc::Status(grpc::StatusCode::INTERNAL, "InternalError")));
520-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(false));
521-
EXPECT_CALL(*backoff_policy_, OnCompletionHook).Times(0);
501+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
502+
.WillOnce(Return(false));
503+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
504+
.Times(0);
522505
}
523506

524507
parser_factory_->AddParser(std::move(parser));
@@ -600,8 +583,9 @@ TEST_F(RowReaderTest, RowLimitIsDecreasedOnRetry) {
600583
EXPECT_CALL(*stream, Finish())
601584
.WillOnce(Return(grpc::Status(grpc::StatusCode::INTERNAL, "retry")));
602585

603-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(true));
604-
EXPECT_CALL(*backoff_policy_, OnCompletionHook)
586+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
587+
.WillOnce(Return(true));
588+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
605589
.WillOnce(Return(std::chrono::milliseconds(0)));
606590

607591
// the stub will free it
@@ -757,8 +741,9 @@ TEST_F(RowReaderTest, FailedStreamRetryNewContext) {
757741
EXPECT_CALL(*stream, Finish())
758742
.WillOnce(Return(grpc::Status(grpc::StatusCode::INTERNAL, "retry")));
759743

760-
EXPECT_CALL(*retry_policy_, OnFailure).WillOnce(Return(true));
761-
EXPECT_CALL(*backoff_policy_, OnCompletionHook)
744+
EXPECT_CALL(*retry_policy_, OnFailure(An<grpc::Status const&>()))
745+
.WillOnce(Return(true));
746+
EXPECT_CALL(*backoff_policy_, OnCompletion(An<grpc::Status const&>()))
762747
.WillOnce(Return(std::chrono::milliseconds(0)));
763748

764749
// the stub will free it

‎google/cloud/bigtable/testing/mock_backoff_policy.h‎

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎google/cloud/bigtable/testing/mock_policies.h‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_TESTING_MOCK_POLICIES_H
16+
#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_TESTING_MOCK_POLICIES_H
17+
18+
#include "google/cloud/bigtable/polling_policy.h"
19+
#include "google/cloud/bigtable/rpc_backoff_policy.h"
20+
#include "google/cloud/bigtable/rpc_retry_policy.h"
21+
#include "google/cloud/status.h"
22+
#include <gmock/gmock.h>
23+
#include <grpcpp/client_context.h>
24+
#include <grpcpp/impl/codegen/status.h>
25+
#include <chrono>
26+
#include <memory>
27+
28+
namespace google {
29+
namespace cloud {
30+
namespace bigtable {
31+
namespace testing {
32+
33+
class MockRetryPolicy : public RPCRetryPolicy {
34+
public:
35+
MOCK_METHOD(std::unique_ptr<RPCRetryPolicy>, clone, (), (const, override));
36+
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (const, override));
37+
MOCK_METHOD(bool, OnFailure, (grpc::Status const& status), (override));
38+
MOCK_METHOD(bool, OnFailure, (Status const& status), (override));
39+
};
40+
41+
class MockBackoffPolicy : public RPCBackoffPolicy {
42+
public:
43+
MOCK_METHOD(std::unique_ptr<RPCBackoffPolicy>, clone, (), (const, override));
44+
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (const, override));
45+
MOCK_METHOD(std::chrono::milliseconds, OnCompletion, (Status const&),
46+
(override));
47+
MOCK_METHOD(std::chrono::milliseconds, OnCompletion, (grpc::Status const&),
48+
(override));
49+
};
50+
51+
class MockPollingPolicy : public PollingPolicy {
52+
public:
53+
MOCK_METHOD(std::unique_ptr<PollingPolicy>, clone, (), (const));
54+
MOCK_METHOD(void, Setup, (grpc::ClientContext&), (override));
55+
MOCK_METHOD(bool, IsPermanentError, (Status const&), (override));
56+
MOCK_METHOD(bool, OnFailure, (Status const&), (override));
57+
MOCK_METHOD(bool, Exhausted, (), (override));
58+
MOCK_METHOD(std::chrono::milliseconds, WaitPeriod, (), (override));
59+
};
60+
61+
} // namespace testing
62+
} // namespace bigtable
63+
} // namespace cloud
64+
} // namespace google
65+
66+
#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_TESTING_MOCK_POLICIES_H

0 commit comments

Comments
 (0)