cleanup(GCS+gRPC): simplify integration tests (#14514)

googleapis · Jul 20, 2024 · ff999ec · ff999ec
1 parent 40b2ac6
commit ff999ec
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 91 deletions.
diff --git a/google/cloud/storage/testing/storage_integration_test.cc b/google/cloud/storage/testing/storage_integration_test.cc
@@ -20,6 +20,7 @@
 #include "google/cloud/storage/testing/random_names.h"
 #include "google/cloud/storage/testing/remove_stale_buckets.h"
 #include "google/cloud/internal/getenv.h"
+#include "google/cloud/testing_util/scoped_environment.h"
 #include "absl/strings/match.h"
 #include <nlohmann/json.hpp>
 
@@ -78,28 +79,16 @@ Options StorageIntegrationTest::MakeTestOptions(Options opts) {
   return google::cloud::internal::MergeOptions(std::move(opts), fallback);
 }
 
-google::cloud::storage::Client
-StorageIntegrationTest::MakeIntegrationTestClient(Options opts) {
-  opts = MakeTestOptions(std::move(opts));
-#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
-  if (UseGrpcForMedia() || UseGrpcForMetadata()) {
-    return storage_experimental::DefaultGrpcClient(std::move(opts));
-  }
-#endif  // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
-  return Client(std::move(opts));
-}
-
-google::cloud::storage::Client
-StorageIntegrationTest::MakeBucketIntegrationTestClient() {
-  if (UsingEmulator()) return MakeIntegrationTestClient();
+Options StorageIntegrationTest::MakeBucketTestOptions() {
+  if (UsingEmulator()) return MakeTestOptions();
 
   auto constexpr kInitialDelay = std::chrono::seconds(5);
   auto constexpr kMaximumBackoffDelay = std::chrono::minutes(5);
   auto constexpr kBackoffScalingFactor = 2.0;
   // This is comparable to the timeout for each integration test, it makes
   // little sense to wait any longer.
   auto constexpr kMaximumRetryTime = std::chrono::minutes(10);
-  return MakeIntegrationTestClient(
+  return MakeTestOptions(
       Options{}
           .set<RetryPolicyOption>(
               LimitedTimeRetryPolicy(kMaximumRetryTime).clone())
@@ -109,6 +98,32 @@ StorageIntegrationTest::MakeBucketIntegrationTestClient() {
                   .clone()));
 }
 
+google::cloud::storage::Client
+StorageIntegrationTest::MakeIntegrationTestClient(bool use_grpc, Options opts) {
+  opts = MakeTestOptions(std::move(opts));
+#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
+  if (use_grpc) {
+    testing_util::ScopedEnvironment env("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
+                                        "metadata");
+    return storage_experimental::DefaultGrpcClient(std::move(opts));
+  }
+#else
+  (void)use_grpc;
+#endif  // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
+  return Client(std::move(opts));
+}
+
+google::cloud::storage::Client
+StorageIntegrationTest::MakeIntegrationTestClient(Options opts) {
+  auto const use_grpc = UseGrpcForMedia() || UseGrpcForMetadata();
+  return MakeIntegrationTestClient(use_grpc, std::move(opts));
+}
+
+google::cloud::storage::Client
+StorageIntegrationTest::MakeBucketIntegrationTestClient() {
+  return MakeIntegrationTestClient(MakeBucketTestOptions());
+}
+
 std::unique_ptr<BackoffPolicy> StorageIntegrationTest::TestBackoffPolicy() {
   std::chrono::milliseconds initial_delay(std::chrono::seconds(1));
   auto constexpr kShortDelayForEmulator = std::chrono::milliseconds(10);

diff --git a/google/cloud/storage/testing/storage_integration_test.h b/google/cloud/storage/testing/storage_integration_test.h
@@ -46,6 +46,13 @@ class StorageIntegrationTest
    */
   static Options MakeTestOptions(Options opts = {});
 
+  /**
+   * Create options suitable for Bucket integration tests.
+   *
+   * Buckets need longer initial backoffs.
+   */
+  static Options MakeBucketTestOptions();
+
   /**
    * Return a client suitable for most integration tests.
    *
@@ -56,6 +63,15 @@ class StorageIntegrationTest
   static google::cloud::storage::Client MakeIntegrationTestClient(
       Options opts = {});
 
+  /**
+   * Create a gRPC or JSON client.
+   *
+   * If @p use_grpc is `true` and gRPC is not compiled-in, it creates a JSON
+   * client.
+   */
+  static google::cloud::storage::Client MakeIntegrationTestClient(
+      bool use_grpc, Options opts = {});
+
   /**
    * Return a client with retry policies suitable for CreateBucket() class.
    *

diff --git a/google/cloud/storage/tests/grpc_bucket_acl_integration_test.cc b/google/cloud/storage/tests/grpc_bucket_acl_integration_test.cc
@@ -14,7 +14,6 @@
 
 #include "google/cloud/storage/testing/storage_integration_test.h"
 #include "google/cloud/internal/getenv.h"
-#include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <iterator>
@@ -28,7 +27,6 @@ namespace {
 
 using ::google::cloud::internal::GetEnv;
 using ::google::cloud::storage::testing::AclEntityNames;
-using ::google::cloud::testing_util::ScopedEnvironment;
 using ::google::cloud::testing_util::StatusIs;
 using ::testing::Contains;
 using ::testing::IsEmpty;
@@ -38,13 +36,12 @@ class GrpcBucketAclIntegrationTest
     : public google::cloud::storage::testing::StorageIntegrationTest {};
 
 TEST_F(GrpcBucketAclIntegrationTest, AclCRUD) {
-  ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                "metadata");
   auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
   ASSERT_THAT(project_id, Not(IsEmpty())) << "GOOGLE_CLOUD_PROJECT is not set";
 
   std::string bucket_name = MakeRandomBucketName();
-  auto client = MakeBucketIntegrationTestClient();
+  auto client =
+      MakeIntegrationTestClient(/*use_grpc=*/true, MakeBucketTestOptions());
 
   // Create a new bucket to run the test, with the "private" PredefinedAcl so
   // we know what the contents of the ACL will be.

diff --git a/google/cloud/storage/tests/grpc_bucket_metadata_integration_test.cc b/google/cloud/storage/tests/grpc_bucket_metadata_integration_test.cc
@@ -14,7 +14,6 @@
 
 #include "google/cloud/storage/testing/storage_integration_test.h"
 #include "google/cloud/internal/getenv.h"
-#include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <algorithm>
@@ -28,7 +27,6 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace {
 
 using ::google::cloud::internal::GetEnv;
-using ::google::cloud::testing_util::ScopedEnvironment;
 using ::google::cloud::testing_util::StatusIs;
 using ::testing::_;
 using ::testing::AllOf;
@@ -44,15 +42,12 @@ class GrpcBucketMetadataIntegrationTest
     : public google::cloud::storage::testing::StorageIntegrationTest {};
 
 TEST_F(GrpcBucketMetadataIntegrationTest, BucketMetadataCRUD) {
-  ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                "metadata");
-
   auto const project_name = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
   ASSERT_THAT(project_name, Not(IsEmpty()))
       << "GOOGLE_CLOUD_PROJECT is not set";
 
-  auto client = MakeIntegrationTestClient();
-
+  auto client =
+      MakeIntegrationTestClient(/*use_grpc=*/true, MakeBucketTestOptions());
   auto bucket_name = MakeRandomBucketName();
   auto insert = client.CreateBucketForProject(bucket_name, project_name,
                                               BucketMetadata());
@@ -138,14 +133,11 @@ TEST_F(GrpcBucketMetadataIntegrationTest, BucketMetadataCRUD) {
 }
 
 TEST_F(GrpcBucketMetadataIntegrationTest, PatchLabels) {
-  ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                "metadata");
-
   auto const project_name = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
   ASSERT_THAT(project_name, Not(IsEmpty()))
       << "GOOGLE_CLOUD_PROJECT is not set";
 
-  auto client = MakeIntegrationTestClient();
+  auto client = MakeIntegrationTestClient(/*use_grpc=true*/);
   auto bucket_name = MakeRandomBucketName();
 
   auto insert = client.CreateBucketForProject(bucket_name, project_name,

diff --git a/google/cloud/storage/tests/grpc_default_object_acl_integration_test.cc b/google/cloud/storage/tests/grpc_default_object_acl_integration_test.cc
@@ -14,7 +14,6 @@
 
 #include "google/cloud/storage/testing/storage_integration_test.h"
 #include "google/cloud/internal/getenv.h"
-#include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <iterator>
@@ -28,7 +27,6 @@ namespace {
 
 using ::google::cloud::internal::GetEnv;
 using ::google::cloud::storage::testing::AclEntityNames;
-using ::google::cloud::testing_util::ScopedEnvironment;
 using ::google::cloud::testing_util::StatusIs;
 using ::testing::Contains;
 using ::testing::IsEmpty;
@@ -38,13 +36,12 @@ class GrpcDefaultObjectAclIntegrationTest
     : public google::cloud::storage::testing::StorageIntegrationTest {};
 
 TEST_F(GrpcDefaultObjectAclIntegrationTest, AclCRUD) {
-  ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                "metadata");
   auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
   ASSERT_THAT(project_id, Not(IsEmpty())) << "GOOGLE_CLOUD_PROJECT is not set";
 
   std::string bucket_name = MakeRandomBucketName();
-  auto client = MakeBucketIntegrationTestClient();
+  auto client =
+      MakeIntegrationTestClient(/*use_grpc=*/true, MakeBucketTestOptions());
 
   // Create a new bucket to run the test, with the "private"
   // PredefinedDefaultObjectAcl, so we know what the contents of the ACL will

diff --git a/google/cloud/storage/tests/grpc_hmac_key_integration_test.cc b/google/cloud/storage/tests/grpc_hmac_key_integration_test.cc
@@ -14,7 +14,6 @@
 
 #include "google/cloud/storage/testing/storage_integration_test.h"
 #include "google/cloud/internal/getenv.h"
-#include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 
@@ -25,7 +24,6 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace {
 
 using ::google::cloud::internal::GetEnv;
-using ::google::cloud::testing_util::ScopedEnvironment;
 using ::testing::Contains;
 using ::testing::IsEmpty;
 using ::testing::Not;
@@ -49,9 +47,7 @@ TEST_F(GrpcHmacKeyMetadataIntegrationTest, HmacKeyCRUD) {
           .value_or("");
   ASSERT_THAT(service_account, Not(IsEmpty()));
 
-  ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                "metadata");
-  auto client = MakeIntegrationTestClient();
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/true);
 
   auto get_ids = [&] {
     std::vector<std::string> ids;

diff --git a/google/cloud/storage/tests/grpc_integration_test.cc b/google/cloud/storage/tests/grpc_integration_test.cc
@@ -14,7 +14,6 @@
 
 #include "google/cloud/storage/testing/storage_integration_test.h"
 #include "google/cloud/internal/getenv.h"
-#include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/setenv.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
@@ -35,13 +34,9 @@ class GrpcIntegrationTest
     : public google::cloud::storage::testing::StorageIntegrationTest,
       public ::testing::WithParamInterface<std::string> {
  protected:
-  GrpcIntegrationTest()
-      : grpc_config_("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", {}) {}
+  GrpcIntegrationTest() = default;
 
   void SetUp() override {
-    std::string const grpc_config_value = GetParam();
-    google::cloud::testing_util::SetEnv("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                        grpc_config_value);
     project_id_ = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
     ASSERT_THAT(project_id_, Not(IsEmpty()))
         << "GOOGLE_CLOUD_PROJECT is not set";
@@ -52,18 +47,20 @@ class GrpcIntegrationTest
         << "GOOGLE_CLOUD_CPP_STORAGE_TEST_BUCKET_NAME is not set";
   }
 
+  static bool ParamIsGrpc() { return GetParam() == "grpc"; }
+
   std::string project_id() const { return project_id_; }
   std::string bucket_name() const { return bucket_name_; }
 
  private:
   std::string project_id_;
   std::string bucket_name_;
-  testing_util::ScopedEnvironment grpc_config_;
 };
 
 TEST_P(GrpcIntegrationTest, ObjectCRUD) {
-  auto bucket_client = MakeBucketIntegrationTestClient();
-  auto client = MakeIntegrationTestClient();
+  auto bucket_client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc(),
+                                                 MakeBucketTestOptions());
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc());
   auto bucket_name = MakeRandomBucketName();
   auto object_name = MakeRandomObjectName();
 
@@ -93,8 +90,9 @@ TEST_P(GrpcIntegrationTest, ObjectCRUD) {
 }
 
 TEST_P(GrpcIntegrationTest, WriteResume) {
-  auto bucket_client = MakeBucketIntegrationTestClient();
-  auto client = MakeIntegrationTestClient();
+  auto bucket_client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc(),
+                                                 MakeBucketTestOptions());
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc());
   auto bucket_name = MakeRandomBucketName();
   auto object_name = MakeRandomObjectName();
 
@@ -141,8 +139,9 @@ TEST_P(GrpcIntegrationTest, WriteResume) {
 }
 
 TEST_P(GrpcIntegrationTest, InsertLarge) {
-  auto bucket_client = MakeBucketIntegrationTestClient();
-  auto client = MakeIntegrationTestClient();
+  auto bucket_client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc(),
+                                                 MakeBucketTestOptions());
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc());
   auto bucket_name = MakeRandomBucketName();
   auto object_name = MakeRandomObjectName();
 
@@ -164,8 +163,9 @@ TEST_P(GrpcIntegrationTest, InsertLarge) {
 }
 
 TEST_P(GrpcIntegrationTest, StreamLargeChunks) {
-  auto bucket_client = MakeBucketIntegrationTestClient();
-  auto client = MakeIntegrationTestClient();
+  auto bucket_client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc(),
+                                                 MakeBucketTestOptions());
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc());
   auto bucket_name = MakeRandomBucketName();
   auto object_name = MakeRandomObjectName();
 
@@ -192,7 +192,7 @@ TEST_P(GrpcIntegrationTest, StreamLargeChunks) {
 }
 
 TEST_P(GrpcIntegrationTest, QuotaUser) {
-  auto client = MakeIntegrationTestClient();
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc());
   auto object_name = MakeRandomObjectName();
 
   auto metadata =
@@ -204,10 +204,10 @@ TEST_P(GrpcIntegrationTest, QuotaUser) {
 
 TEST_P(GrpcIntegrationTest, FieldFilter) {
   if (UsingEmulator()) GTEST_SKIP();
-  auto const* fields = UsingGrpc() ? "resource.bucket,resource.name,resource."
-                                     "generation,resource.content_type"
-                                   : "bucket,name,generation,contentType";
-  auto client = MakeIntegrationTestClient();
+  auto const* fields = ParamIsGrpc() ? "resource.bucket,resource.name,resource."
+                                       "generation,resource.content_type"
+                                     : "bucket,name,generation,contentType";
+  auto client = MakeIntegrationTestClient(/*use_grpc=*/ParamIsGrpc());
   auto object_name = MakeRandomObjectName();
 
   auto metadata = client.InsertObject(
@@ -226,7 +226,7 @@ TEST_P(GrpcIntegrationTest, FieldFilter) {
 
 #if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
 INSTANTIATE_TEST_SUITE_P(GrpcIntegrationMediaTest, GrpcIntegrationTest,
-                         ::testing::Values("media"));
+                         ::testing::Values("grpc"));
 #else
 INSTANTIATE_TEST_SUITE_P(GrpcIntegrationMediaTest, GrpcIntegrationTest,
                          ::testing::Values("none"));

diff --git a/google/cloud/storage/tests/grpc_notification_integration_test.cc b/google/cloud/storage/tests/grpc_notification_integration_test.cc
@@ -14,7 +14,6 @@
 
 #include "google/cloud/storage/testing/storage_integration_test.h"
 #include "google/cloud/internal/getenv.h"
-#include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 
@@ -25,7 +24,6 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace {
 
 using ::google::cloud::internal::GetEnv;
-using ::google::cloud::testing_util::ScopedEnvironment;
 using ::google::cloud::testing_util::StatusIs;
 using ::testing::Contains;
 using ::testing::ElementsAre;
@@ -42,9 +40,6 @@ TEST_F(GrpcNotificationIntegrationTest, NotificationCRUD) {
   // TODO(#14396) - figure out what to do with the Notifications and gRPC
   if (!UsingEmulator()) GTEST_SKIP();
 
-  ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
-                                "metadata");
-
   auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");
   ASSERT_THAT(project_id, Not(IsEmpty())) << "GOOGLE_CLOUD_PROJECT is not set";
   auto const topic_name = google::cloud::internal::GetEnv(
@@ -53,7 +48,8 @@ TEST_F(GrpcNotificationIntegrationTest, NotificationCRUD) {
   ASSERT_THAT(topic_name, Not(IsEmpty()));
 
   std::string bucket_name = MakeRandomBucketName();
-  auto client = MakeBucketIntegrationTestClient();
+  auto client =
+      MakeIntegrationTestClient(/*use_grpc=*/true, MakeBucketTestOptions());
 
   auto metadata =
       client.CreateBucketForProject(bucket_name, project_id, BucketMetadata());