Skip to content

Commit 1cb7031

Browse files
coryancoryan
authored
test(GCS+gRPC): prepare integration tests for production (#9807)
A few tests needed some tweaks to work in production (the testbench is more forgiving). I also switched from a generic bug to track the disabled tests to specific bugs that describe (or link) the underlying problem.
1 parent 7c4b8ca commit 1cb7031

12 files changed

+30
-64
lines changed

‎google/cloud/storage/tests/bucket_integration_test.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ TEST_F(BucketIntegrationTest, PatchLifecycleConditions) {
268268
}
269269

270270
TEST_F(BucketIntegrationTest, FullPatch) {
271-
// TODO(#5673) - enable in production.
271+
// TODO(#9801) - enable in production.
272272
if (UsingGrpc() && !UsingEmulator()) GTEST_SKIP();
273273

274274
std::string bucket_name = MakeRandomBucketName();
@@ -478,7 +478,7 @@ TEST_F(BucketIntegrationTest, PublicAccessPreventionPatch) {
478478

479479
/// @test Verify that we can set the RPO in a Bucket.
480480
TEST_F(BucketIntegrationTest, RpoPatch) {
481-
// TODO(#5673) - enable in production
481+
// TODO(#9802) - enable in production.
482482
if (UsingGrpc() && !UsingEmulator()) GTEST_SKIP();
483483

484484
std::string bucket_name = MakeRandomBucketName();
@@ -615,7 +615,7 @@ TEST_F(BucketIntegrationTest, GetMetadataIfMetagenerationNotMatchFailure) {
615615
}
616616

617617
TEST_F(BucketIntegrationTest, AccessControlCRUD) {
618-
// TODO(#5673) - enable in production
618+
// TODO(#9800) - enable in production.
619619
if (UsingGrpc() && !UsingEmulator()) GTEST_SKIP();
620620

621621
std::string bucket_name = MakeRandomBucketName();
@@ -689,7 +689,7 @@ TEST_F(BucketIntegrationTest, AccessControlCRUD) {
689689
}
690690

691691
TEST_F(BucketIntegrationTest, DefaultObjectAccessControlCRUD) {
692-
// TODO(#5673) - enable in production
692+
// TODO(#9800) - enable in production
693693
if (UsingGrpc() && !UsingEmulator()) GTEST_SKIP();
694694

695695
std::string bucket_name = MakeRandomBucketName();
@@ -761,7 +761,7 @@ TEST_F(BucketIntegrationTest, DefaultObjectAccessControlCRUD) {
761761
}
762762

763763
TEST_F(BucketIntegrationTest, NotificationsCRUD) {
764-
// TODO(#5673) - enable when gRPC implements these operations.
764+
// TODO(#9806) - enable when gRPC implements these operations.
765765
if (UsingGrpc() && !UsingEmulator()) GTEST_SKIP();
766766

767767
std::string bucket_name = MakeRandomBucketName();

‎google/cloud/storage/tests/grpc_bucket_acl_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class GrpcBucketAclIntegrationTest
4545
TEST_F(GrpcBucketAclIntegrationTest, AclCRUD) {
4646
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4747
"metadata");
48-
// TODO(#5673) - restore gRPC integration tests against production
48+
// TODO(#9800) - restore gRPC integration tests against production
4949
if (!UsingEmulator()) GTEST_SKIP();
5050

5151
auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");

‎google/cloud/storage/tests/grpc_bucket_metadata_integration_test.cc‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,12 @@ TEST_F(GrpcBucketMetadataIntegrationTest, ObjectMetadataCRUD) {
7979
EXPECT_EQ(get->location_type(), insert->location_type());
8080
EXPECT_EQ(get->storage_class(), insert->storage_class());
8181

82+
// We need to set the retention policy or the request to lock the retention
83+
// policy (see below) will fail.
8284
auto patch = client->PatchBucket(
83-
bucket_name, BucketMetadataPatchBuilder{}.SetLabel("l0", "k0"));
85+
bucket_name, BucketMetadataPatchBuilder{}
86+
.SetLabel("l0", "k0")
87+
.SetRetentionPolicy(std::chrono::seconds(30)));
8488
ASSERT_STATUS_OK(patch);
8589
EXPECT_THAT(patch->labels(), ElementsAre(Pair("l0", "k0")));
8690

@@ -93,8 +97,10 @@ TEST_F(GrpcBucketMetadataIntegrationTest, ObjectMetadataCRUD) {
9397
auto locked =
9498
client->LockBucketRetentionPolicy(bucket_name, updated->metageneration());
9599
ASSERT_STATUS_OK(locked);
96-
EXPECT_FALSE(updated->has_retention_policy());
97-
EXPECT_TRUE(locked->has_retention_policy());
100+
ASSERT_TRUE(updated->has_retention_policy());
101+
ASSERT_TRUE(locked->has_retention_policy());
102+
EXPECT_FALSE(updated->retention_policy().is_locked);
103+
EXPECT_TRUE(locked->retention_policy().is_locked);
98104

99105
// Create a second bucket to make the list more interesting.
100106
auto bucket_name_2 = MakeRandomBucketName();
@@ -118,7 +124,6 @@ TEST_F(GrpcBucketMetadataIntegrationTest, ObjectMetadataCRUD) {
118124
std::back_inserter(roles),
119125
[](NativeIamBinding const& b) { return b.role(); });
120126
EXPECT_THAT(roles, IsSupersetOf({"roles/storage.legacyBucketOwner",
121-
"roles/storage.legacyBucketWriter",
122127
"roles/storage.legacyBucketReader"}));
123128

124129
auto new_policy = *policy;

‎google/cloud/storage/tests/grpc_default_object_acl_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class GrpcDefaultObjectAclIntegrationTest
4545
TEST_F(GrpcDefaultObjectAclIntegrationTest, AclCRUD) {
4646
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4747
"metadata");
48-
// TODO(#5673) - restore gRPC integration tests against production
48+
// TODO(#9800) - restore gRPC integration tests against production
4949
if (!UsingEmulator()) GTEST_SKIP();
5050

5151
auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");

‎google/cloud/storage/tests/grpc_notification_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class GrpcNotificationIntegrationTest
4141
: public google::cloud::storage::testing::StorageIntegrationTest {};
4242

4343
TEST_F(GrpcNotificationIntegrationTest, NotificationCRUD) {
44-
// TODO(#5673) - enable in production
44+
// TODO(#9806) - enable in production
4545
if (!UsingEmulator()) GTEST_SKIP();
4646
auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
4747
ASSERT_THAT(project_id, Not(IsEmpty())) << "GOOGLE_CLOUD_PROJECT is not set";

‎google/cloud/storage/tests/grpc_object_acl_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class GrpcObjectAclIntegrationTest
4545
TEST_F(GrpcObjectAclIntegrationTest, AclCRUD) {
4646
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4747
"metadata");
48-
// TODO(#5673) - restore gRPC integration tests against production
48+
// TODO(#9800) - restore gRPC integration tests against production
4949
if (!UsingEmulator()) GTEST_SKIP();
5050

5151
auto const bucket_name =

‎google/cloud/storage/tests/grpc_object_media_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class GrpcObjectMediaIntegrationTest
4141
TEST_F(GrpcObjectMediaIntegrationTest, CancelResumableUpload) {
4242
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4343
"metadata");
44-
// TODO(#5673) - restore gRPC integration tests against production
44+
// TODO(#9804) - restore gRPC integration tests against production
4545
if (!UsingEmulator()) GTEST_SKIP();
4646

4747
auto const bucket_name =

‎google/cloud/storage/tests/grpc_object_metadata_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class GrpcObjectMetadataIntegrationTest
4343
TEST_F(GrpcObjectMetadataIntegrationTest, ObjectMetadataCRUD) {
4444
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4545
"metadata");
46-
// TODO(#5673) - restore gRPC integration tests against production
46+
// TODO(#9805) - restore gRPC integration tests against production
4747
if (!UsingEmulator()) GTEST_SKIP();
4848

4949
auto const bucket_name =

‎google/cloud/storage/tests/grpc_service_account_integration_test.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ class GrpcServiceAccountIntegrationTest
3838
TEST_F(GrpcServiceAccountIntegrationTest, GetServiceAccount) {
3939
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4040
"metadata");
41-
// TODO(#5673) - restore gRPC integration tests against production
42-
if (!UsingEmulator()) GTEST_SKIP();
43-
4441
auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
4542
ASSERT_THAT(project_id, Not(IsEmpty())) << "GOOGLE_CLOUD_PROJECT is not set";
4643

‎google/cloud/storage/tests/object_basic_crud_integration_test.cc‎

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ using ObjectBasicCRUDIntegrationTest =
4343

4444
/// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations.
4545
TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) {
46+
// TODO(#9805) - restore gRPC integration tests against production
47+
if (!UsingEmulator()) GTEST_SKIP();
48+
4649
StatusOr<Client> client = MakeIntegrationTestClient();
4750
ASSERT_STATUS_OK(client);
4851

@@ -72,43 +75,7 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) {
7275
bucket_name_, object_name, Generation(insert_meta->generation()),
7376
Projection("full"));
7477
ASSERT_STATUS_OK(get_meta);
75-
76-
if (UsingGrpc()) {
77-
// The metadata returned by gRPC (InsertObject) doesn't contain the `acl`,
78-
// `etag`, `media_link`, or `self_link` fields. Just compare field by field:
79-
EXPECT_EQ(get_meta->name(), insert_meta->name());
80-
// EXPECT_EQ(get_meta->acl(), insert_meta->acl());
81-
EXPECT_EQ(get_meta->bucket(), insert_meta->bucket());
82-
EXPECT_EQ(get_meta->cache_control(), insert_meta->cache_control());
83-
EXPECT_EQ(get_meta->component_count(), insert_meta->component_count());
84-
EXPECT_EQ(get_meta->content_disposition(),
85-
insert_meta->content_disposition());
86-
EXPECT_EQ(get_meta->content_encoding(), insert_meta->content_encoding());
87-
EXPECT_EQ(get_meta->content_type(), insert_meta->content_type());
88-
EXPECT_EQ(get_meta->crc32c(), insert_meta->crc32c());
89-
EXPECT_EQ(get_meta->event_based_hold(), insert_meta->event_based_hold());
90-
EXPECT_EQ(get_meta->generation(), insert_meta->generation());
91-
EXPECT_EQ(get_meta->id(), insert_meta->id());
92-
EXPECT_EQ(get_meta->kind(), insert_meta->kind());
93-
EXPECT_EQ(get_meta->kms_key_name(), insert_meta->kms_key_name());
94-
EXPECT_EQ(get_meta->md5_hash(), insert_meta->md5_hash());
95-
EXPECT_EQ(get_meta->media_link(), insert_meta->media_link());
96-
EXPECT_EQ(get_meta->metageneration(), insert_meta->metageneration());
97-
// EXPECT_EQ(get_meta->owner(), insert_meta->owner());
98-
EXPECT_EQ(get_meta->retention_expiration_time(),
99-
insert_meta->retention_expiration_time());
100-
EXPECT_EQ(get_meta->self_link(), insert_meta->self_link());
101-
EXPECT_EQ(get_meta->size(), insert_meta->size());
102-
EXPECT_EQ(get_meta->storage_class(), insert_meta->storage_class());
103-
EXPECT_EQ(get_meta->temporary_hold(), insert_meta->temporary_hold());
104-
EXPECT_EQ(get_meta->time_created(), insert_meta->time_created());
105-
EXPECT_EQ(get_meta->time_deleted(), insert_meta->time_deleted());
106-
EXPECT_EQ(get_meta->time_storage_class_updated(),
107-
insert_meta->time_storage_class_updated());
108-
EXPECT_EQ(get_meta->updated(), insert_meta->updated());
109-
} else {
110-
EXPECT_EQ(*get_meta, *insert_meta);
111-
}
78+
EXPECT_EQ(*get_meta, *insert_meta);
11279

11380
ObjectMetadata update = *get_meta;
11481
update.mutable_acl().emplace_back(
@@ -124,8 +91,8 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) {
12491
bucket_name_, object_name, update, Projection("full"));
12592
ASSERT_STATUS_OK(updated_meta);
12693

127-
// Because some of the ACL values are not predictable we convert the values we
128-
// care about to strings and compare that.
94+
// Because some ACL field values are not predictable, we convert the values we
95+
// care about to strings and compare those.
12996
{
13097
auto acl_to_string_vector =
13198
[](std::vector<ObjectAccessControl> const& acl) {
@@ -154,7 +121,8 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) {
154121

155122
ObjectMetadata desired_patch = *updated_meta;
156123
desired_patch.set_content_language("en");
157-
desired_patch.mutable_metadata().erase("updated");
124+
// TODO(#9803) - enable once gRPC supports partial metadata updates.
125+
if (!UsingGrpc()) desired_patch.mutable_metadata().erase("updated");
158126
desired_patch.mutable_metadata().emplace("patched", "true");
159127
StatusOr<ObjectMetadata> patched_meta =
160128
client->PatchObject(bucket_name_, object_name, *updated_meta,

0 commit comments

Comments
 (0)