cleanup(storage): prefer MakeIntegrationTestClient() (#14515)

Use the common function to initialize the `storage::Client` object.

google/cloud/storage/tests/decompressive_transcoding_integration_test.cc

@@ -60,7 +60,9 @@ auto constexpr kFirstLine =
     "000000001: The quick brown fox jumps over the lazy dog";
 
 TEST_F(DecompressiveTranscodingIntegrationTest, WriteAndReadJson) {
-  auto client = Client(
+  if (UsingGrpc()) GTEST_SKIP();
+
+  auto client = MakeIntegrationTestClient(
       Options{}
           .set<TransferStallTimeoutOption>(std::chrono::seconds(3))
           .set<RetryPolicyOption>(LimitedErrorCountRetryPolicy(5).clone()));
@@ -76,9 +78,6 @@ TEST_F(DecompressiveTranscodingIntegrationTest, WriteAndReadJson) {
   EXPECT_EQ(insert->content_encoding(), "gzip");
   EXPECT_EQ(insert->content_type(), "text/plain");
 
-  // TODO(#8829) - decompressive transcoding does not work with gRPC
-  if (UsingGrpc()) return;
-
   auto reader =
       client.ReadObject(bucket_name(), object_name, IfGenerationNotMatch(0));
   ASSERT_STATUS_OK(reader.status());
@@ -92,7 +91,7 @@ TEST_F(DecompressiveTranscodingIntegrationTest, WriteAndReadJson) {
 }
 
 TEST_F(DecompressiveTranscodingIntegrationTest, WriteAndReadCompressedJson) {
-  auto client = Client(
+  auto client = MakeIntegrationTestClient(
       Options{}
           .set<TransferStallTimeoutOption>(std::chrono::seconds(3))
           .set<RetryPolicyOption>(LimitedErrorCountRetryPolicy(5).clone()));
@@ -123,7 +122,7 @@ TEST_F(DecompressiveTranscodingIntegrationTest, ResumeGunzippedDownloadJson) {
   // TODO(#8829) - decompressive transcoding does not work with gRPC
   if (!UsingEmulator() || UsingGrpc()) GTEST_SKIP();
 
-  auto client = Client(
+  auto client = MakeIntegrationTestClient(
       Options{}
           .set<MaximumCurlSocketRecvSizeOption>(16 * 1024)
           .set<DownloadBufferSizeOption>(1024)

google/cloud/storage/tests/key_file_integration_test.cc

@@ -67,12 +67,14 @@ class KeyFileIntegrationTest
 };
 
 TEST_P(KeyFileIntegrationTest, ObjectWriteSignAndReadDefaultAccount) {
+  if (UsingGrpc()) GTEST_SKIP();
+
   auto credentials =
       oauth2::CreateServiceAccountCredentialsFromFilePath(key_filename_);
   ASSERT_STATUS_OK(credentials);
 
-  Client client(Options{}.set<Oauth2CredentialsOption>(*credentials));
-
+  auto client = MakeIntegrationTestClient(
+      Options{}.set<Oauth2CredentialsOption>(*credentials));
   auto object_name = MakeRandomObjectName();
   std::string expected = LoremIpsum();
 
@@ -93,12 +95,14 @@ TEST_P(KeyFileIntegrationTest, ObjectWriteSignAndReadDefaultAccount) {
 }
 
 TEST_P(KeyFileIntegrationTest, ObjectWriteSignAndReadExplicitAccount) {
+  if (UsingGrpc()) GTEST_SKIP();
+
   auto credentials =
       oauth2::CreateServiceAccountCredentialsFromFilePath(key_filename_);
   ASSERT_STATUS_OK(credentials);
 
-  Client client(Options{}.set<Oauth2CredentialsOption>(*credentials));
-
+  auto client = MakeIntegrationTestClient(
+      Options{}.set<Oauth2CredentialsOption>(*credentials));
   auto object_name = MakeRandomObjectName();
   std::string expected = LoremIpsum();
 

google/cloud/storage/tests/object_basic_crud_integration_test.cc

@@ -19,6 +19,7 @@
 #include "google/cloud/status_or.h"
 #include "google/cloud/testing_util/scoped_environment.h"
 #include "google/cloud/testing_util/status_matchers.h"
+#include "absl/strings/match.h"
 #include <gmock/gmock.h>
 #include <algorithm>
 #include <cstring>
@@ -36,8 +37,27 @@ namespace {
 using ::testing::Contains;
 using ::testing::Not;
 using ::testing::UnorderedElementsAreArray;
-using ObjectBasicCRUDIntegrationTest =
-    ::google::cloud::storage::testing::ObjectIntegrationTest;
+
+struct ObjectBasicCRUDIntegrationTest
+    : public ::google::cloud::storage::testing::ObjectIntegrationTest {
+ public:
+  static Client MakeNonDefaultClient() {
+    // Use a different spelling of the default endpoint.
+    auto options = MakeTestOptions();
+    auto endpoint = options.get<RestEndpointOption>();
+    if (absl::StartsWith(endpoint, "https") &&
+        !absl::EndsWith(endpoint, ":443")) {
+      options.set<RestEndpointOption>(endpoint + ":443");
+    }
+    endpoint = options.get<EndpointOption>();
+    if (endpoint.empty()) {
+      options.set<EndpointOption>("storage.googleapis.com:443");
+    } else if (!absl::EndsWith(endpoint, ":443")) {
+      options.set<EndpointOption>(endpoint + ":443");
+    }
+    return MakeIntegrationTestClient(std::move(options));
+  }
+};
 
 /// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations.
 TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) {
@@ -133,28 +153,9 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) {
   EXPECT_THAT(list_object_names(), Not(Contains(object_name)));
 }
 
-Client CreateNonDefaultClient() {
-  auto emulator =
-      google::cloud::internal::GetEnv("CLOUD_STORAGE_EMULATOR_ENDPOINT");
-  google::cloud::testing_util::ScopedEnvironment env(
-      "CLOUD_STORAGE_EMULATOR_ENDPOINT", {});
-  auto options = google::cloud::Options{};
-  if (!emulator) {
-    // Use a different spelling of the default endpoint. This disables the
-    // allegedly "slightly faster" XML endpoints, but should continue to work.
-    options.set<RestEndpointOption>("https://storage.googleapis.com:443");
-    options.set<UnifiedCredentialsOption>(MakeGoogleDefaultCredentials());
-  } else {
-    // Use the emulator endpoint, but not through the environment variable
-    options.set<RestEndpointOption>(*emulator);
-    options.set<UnifiedCredentialsOption>(MakeInsecureCredentials());
-  }
-  return Client(std::move(options));
-}
-
 /// @test Verify that the client works with non-default endpoints.
-TEST_F(ObjectBasicCRUDIntegrationTest, NonDefaultEndpointInsertJSON) {
-  auto client = CreateNonDefaultClient();
+TEST_F(ObjectBasicCRUDIntegrationTest, NonDefaultEndpointInsert) {
+  auto client = MakeNonDefaultClient();
   auto object_name = MakeRandomObjectName();
   auto const expected = LoremIpsum();
   auto insert = client.InsertObject(bucket_name_, object_name, expected);
@@ -168,8 +169,8 @@ TEST_F(ObjectBasicCRUDIntegrationTest, NonDefaultEndpointInsertJSON) {
 }
 
 /// @test Verify that the client works with non-default endpoints.
-TEST_F(ObjectBasicCRUDIntegrationTest, NonDefaultEndpointWriteJSON) {
-  auto client = CreateNonDefaultClient();
+TEST_F(ObjectBasicCRUDIntegrationTest, NonDefaultEndpointWrite) {
+  auto client = MakeNonDefaultClient();
   auto object_name = MakeRandomObjectName();
   auto const expected = LoremIpsum();
   auto writer = client.WriteObject(bucket_name_, object_name);

google/cloud/storage/tests/object_file_integration_test.cc

@@ -49,14 +49,15 @@ class ObjectFileIntegrationTest
     ASSERT_FALSE(bucket_name_.empty());
   }
 
+  // Create a client configured to always use resumable uploads for files.
+  static Client ClientWithSimpleUploadDisabled() {
+    return MakeIntegrationTestClient(
+        Options{}.set<MaximumSimpleUploadSizeOption>(0));
+  }
+
   std::string bucket_name_;
 };
 
-// Create a client configured to always use resumable uploads for files.
-Client ClientWithSimpleUploadDisabled() {
-  return Client(Options{}.set<MaximumSimpleUploadSizeOption>(0));
-}
-
 TEST_F(ObjectFileIntegrationTest, JsonDownloadFile) {
   auto client = MakeIntegrationTestClient();
   auto object_name = MakeRandomObjectName();

google/cloud/storage/tests/object_media_integration_test.cc

@@ -513,7 +513,7 @@ TEST_F(ObjectMediaIntegrationTest, StreamingReadTimeout) {
   auto options = ClientOptions::CreateDefaultClientOptions();
   ASSERT_STATUS_OK(options);
 
-  Client client(
+  auto client = MakeIntegrationTestClient(
       Options{}
           .set<TransferStallTimeoutOption>(std::chrono::seconds(3))
           .set<RetryPolicyOption>(LimitedErrorCountRetryPolicy(3).clone()));
@@ -544,7 +544,7 @@ TEST_F(ObjectMediaIntegrationTest, StreamingReadTimeoutContinues) {
   // The emulator does not support this type of fault injection for gRPC.
   if (!UsingEmulator() || UsingGrpc()) GTEST_SKIP();
 
-  Client client(
+  auto client = MakeIntegrationTestClient(
       Options{}
           .set<TransferStallTimeoutOption>(std::chrono::seconds(3))
           .set<RetryPolicyOption>(LimitedErrorCountRetryPolicy(10).clone()));
@@ -580,7 +580,7 @@ TEST_F(ObjectMediaIntegrationTest, StreamingReadTimeoutContinues) {
 TEST_F(ObjectMediaIntegrationTest, StreamingReadInternalError) {
   if (!UsingEmulator()) GTEST_SKIP();
 
-  Client client(
+  auto client = MakeIntegrationTestClient(
       Options{}
           .set<TransferStallTimeoutOption>(std::chrono::seconds(3))
           .set<RetryPolicyOption>(LimitedErrorCountRetryPolicy(5).clone()));

google/cloud/storage/tests/object_resumable_write_integration_test.cc

@@ -336,7 +336,8 @@ TEST_F(ObjectResumableWriteIntegrationTest, WithXUploadContentLength) {
   auto constexpr kMiB = 1024 * 1024L;
   auto constexpr kChunkSize = 2 * kMiB;
 
-  Client client(Options{}.set<UploadBufferSizeOption>(kChunkSize));
+  auto client = MakeIntegrationTestClient(
+      Options{}.set<UploadBufferSizeOption>(kChunkSize));
 
   auto const chunk = MakeRandomData(kChunkSize);
 
@@ -369,8 +370,8 @@ TEST_F(ObjectResumableWriteIntegrationTest, WithXUploadContentLengthRandom) {
   auto constexpr kQuantum = 256 * 1024L;
   size_t constexpr kChunkSize = 2 * kQuantum;
 
-  Client client(Options{}.set<UploadBufferSizeOption>(kChunkSize));
-
+  auto client = MakeIntegrationTestClient(
+      Options{}.set<UploadBufferSizeOption>(kChunkSize));
   auto const chunk = MakeRandomData(kChunkSize);
 
   std::uniform_int_distribution<std::size_t> size_gen(kQuantum, 5 * kQuantum);

google/cloud/storage/tests/service_account_integration_test.cc

@@ -65,7 +65,9 @@ TEST_F(ServiceAccountIntegrationTest, CreateHmacKeyForProject) {
   // redesigning the tests to use a random service account (or creating one)
   // dynamically.  For now, simply skip these tests.
   if (!UsingEmulator()) GTEST_SKIP();
-  Client client(Options{}.set<ProjectIdOption>(project_id_));
+
+  auto client =
+      MakeIntegrationTestClient(Options{}.set<ProjectIdOption>(project_id_));
 
   StatusOr<std::pair<HmacKeyMetadata, std::string>> key = client.CreateHmacKey(
       service_account_, OverrideDefaultProject(project_id_));
@@ -87,7 +89,8 @@ TEST_F(ServiceAccountIntegrationTest, HmacKeyCRUD) {
   // redesigning the tests to use a random service account (or creating one)
   // dynamically.  For now, simply skip these tests.
   if (!UsingEmulator()) GTEST_SKIP();
-  Client client(Options{}.set<ProjectIdOption>(project_id_));
+  auto client =
+      MakeIntegrationTestClient(Options{}.set<ProjectIdOption>(project_id_));
 
   auto get_current_access_ids = [&client, this]() {
     std::vector<std::string> access_ids;
@@ -132,7 +135,10 @@ TEST_F(ServiceAccountIntegrationTest, HmacKeyCRUD) {
 }
 
 TEST_F(ServiceAccountIntegrationTest, HmacKeyCRUDFailures) {
-  Client client(Options{}.set<ProjectIdOption>(project_id_));
+  if (UsingGrpc()) GTEST_SKIP();
+
+  auto client =
+      MakeIntegrationTestClient(Options{}.set<ProjectIdOption>(project_id_));
 
   // Test failures in the HmacKey operations by using an invalid project id:
   auto create_status = client.CreateHmacKey("invalid-service-account",

google/cloud/storage/tests/signed_url_conformance_test.cc

@@ -83,7 +83,8 @@ TEST_P(V4SignedUrlConformanceTest, V4SignJson) {
   ASSERT_STATUS_OK(creds);
 
   std::string account_email = (*creds)->AccountEmail();
-  Client client(Options{}.set<Oauth2CredentialsOption>(*creds));
+  auto client =
+      MakeIntegrationTestClient(Options{}.set<Oauth2CredentialsOption>(*creds));
   std::string actual_canonical_request;
   std::string actual_string_to_sign;
 
@@ -186,7 +187,8 @@ TEST_P(V4PostPolicyConformanceTest, V4PostPolicy) {
   ASSERT_STATUS_OK(creds);
 
   std::string account_email = (*creds)->AccountEmail();
-  Client client(Options{}.set<Oauth2CredentialsOption>(*creds));
+  auto client =
+      MakeIntegrationTestClient(Options{}.set<Oauth2CredentialsOption>(*creds));
 
   auto const& test_params = (*post_policy_tests)[GetParam()];
   auto const& input = test_params.policyinput();

google/cloud/storage/tests/thread_integration_test.cc

@@ -104,6 +104,7 @@ TEST_F(ThreadIntegrationTest, Unshared) {
       PredefinedAcl("private"), PredefinedDefaultObjectAcl("projectPrivate"),
       Projection("full"));
   ASSERT_STATUS_OK(meta);
+  ScheduleForDelete(*meta);
   EXPECT_EQ(bucket_name, meta->name());
 
   // Clamp the thread count to the [8, 32] range. Sadly, `std::clamp` is a C++17
@@ -159,10 +160,11 @@ class CaptureSendHeaderBackend : public LogBackend {
 };
 
 TEST_F(ThreadIntegrationTest, ReuseConnections) {
-  auto log_backend = std::make_shared<CaptureSendHeaderBackend>();
-
-  Client client(Options{}.set<LoggingComponentsOption>({"raw-client", "http"}));
+  if (UsingGrpc()) GTEST_SKIP();
 
+  auto log_backend = std::make_shared<CaptureSendHeaderBackend>();
+  auto client = MakeIntegrationTestClient(
+      Options{}.set<LoggingComponentsOption>({"raw-client", "http"}));
   std::string bucket_name = MakeRandomBucketName();
 
   auto id = LogSink::Instance().AddBackend(log_backend);

google/cloud/storage/tests/tracing_integration_test.cc

@@ -62,8 +62,10 @@ class TracingIntegrationTest
 };
 
 TEST_F(TracingIntegrationTest, StorageConnection) {
-  auto client =
-      Client(Options{}.set<LoggingComponentsOption>({"raw-client", "http"}));
+  if (UsingGrpc()) GTEST_SKIP();
+
+  auto client = MakeIntegrationTestClient(
+      Options{}.set<LoggingComponentsOption>({"raw-client", "http"}));
 
   ScopedLog log;
   auto const object_name = MakeRandomObjectName();