diff --git a/velox/connectors/hive/storage_adapters/gcs/tests/GcsEmulator.h b/velox/connectors/hive/storage_adapters/gcs/tests/GcsEmulator.h index 1a33326850d28..12db0a12808f8 100644 --- a/velox/connectors/hive/storage_adapters/gcs/tests/GcsEmulator.h +++ b/velox/connectors/hive/storage_adapters/gcs/tests/GcsEmulator.h @@ -50,14 +50,14 @@ class GcsEmulator : public testing::Environment { if (const auto* env = std::getenv("PYTHON")) { names = {env}; } - auto error = std::string( - "Coud not start GCS emulator." - " Used the following list of python interpreter names:"); + std::stringstream error; + error << R"""({>>"Coud not start GCS emulator." + " The following list of python interpreter names were used:"})"""; for (const auto& interpreter : names) { auto exe_path = bp::search_path(interpreter); - error += " " + interpreter; + error << " " << interpreter; if (exe_path.empty()) { - error += " (exe not found)"; + error << " (exe not found)"; continue; } @@ -69,19 +69,18 @@ class GcsEmulator : public testing::Environment { "--port", port, group_); - if (serverProcess_.valid()) - break; - error += " (failed to start)"; + if (serverProcess_.valid()) { + return; + } + error << " (failed to start)"; serverProcess_.terminate(); serverProcess_.wait(); } - if (serverProcess_.valid() && serverProcess_.valid()) - return; - error_ = std::move(error); + VELOX_FAIL(error.str()); } ~GcsEmulator() override { - // Brutal shutdown, kill the full process group because the GCS testbench + // Brutal shutdown, kill the full process group because the GCS emulator // may launch additional children. group_.terminate(); if (serverProcess_.valid()) { @@ -113,7 +112,6 @@ class GcsEmulator : public testing::Environment { void bootstrap() { ASSERT_THAT(this, ::testing::NotNull()); - ASSERT_THAT(this->error_, ::testing::IsEmpty()); // Create a bucket and a small file in the testbench. This makes it easier // to bootstrap GcsFileSystem and its tests. @@ -122,16 +120,12 @@ class GcsEmulator : public testing::Environment { .set(this->endpoint_) .set(gc::MakeInsecureCredentials())); - bucketName_ = "test1-gcs"; - google::cloud::StatusOr bucket = - client.CreateBucketForProject( - bucketName_, "ignored-by-testbench", gcs::BucketMetadata{}); + auto bucket = client.CreateBucketForProject( + bucketName_, "ignored-by-testbench", gcs::BucketMetadata{}); ASSERT_TRUE(bucket.ok()) << "Failed to create bucket <" << bucketName_ << ">, status=" << bucket.status(); - objectName_ = "test-object-name"; - google::cloud::StatusOr object = - client.InsertObject(bucketName_, objectName_, kLoremIpsum); + auto object = client.InsertObject(bucketName_, objectName_, kLoremIpsum); ASSERT_TRUE(object.ok()) << "Failed to create object <" << objectName_ << ">, status=" << object.status(); } @@ -140,9 +134,11 @@ class GcsEmulator : public testing::Environment { std::string endpoint_; bp::child serverProcess_; bp::group group_; - std::string error_; - std::string bucketName_; - std::string objectName_; + static std::string bucketName_; + static std::string objectName_; }; +std::string GcsEmulator::bucketName_{"test1-gcs"}; +std::string GcsEmulator::objectName_{"test-object-name"}; + } // namespace facebook::velox::filesystems diff --git a/velox/connectors/hive/storage_adapters/gcs/tests/GcsFileSystemTest.cpp b/velox/connectors/hive/storage_adapters/gcs/tests/GcsFileSystemTest.cpp index 69202142340a7..87d12fa15c8a6 100644 --- a/velox/connectors/hive/storage_adapters/gcs/tests/GcsFileSystemTest.cpp +++ b/velox/connectors/hive/storage_adapters/gcs/tests/GcsFileSystemTest.cpp @@ -80,27 +80,27 @@ TEST_F(GcsFileSystemTest, writeAndReadFile) { filesystems::GcsFileSystem gcfs(testbench_->hiveConfig()); gcfs.initializeClient(); auto writeFile = gcfs.openFileForWrite(gcsFile); - std::string dataContent = + std::string_view kDataContent = "Dance me to your beauty with a burning violin" "Dance me through the panic till I'm gathered safely in" "Lift me like an olive branch and be my homeward dove" "Dance me to the end of love"; EXPECT_EQ(writeFile->size(), 0); - std::int64_t contentSize = dataContent.length(); - writeFile->append(dataContent.substr(0, 10)); + std::int64_t contentSize = kDataContent.length(); + writeFile->append(kDataContent.substr(0, 10)); EXPECT_EQ(writeFile->size(), 10); - writeFile->append(dataContent.substr(10, contentSize - 10)); + writeFile->append(kDataContent.substr(10, contentSize - 10)); EXPECT_EQ(writeFile->size(), contentSize); writeFile->flush(); writeFile->close(); VELOX_ASSERT_THROW( - writeFile->append(dataContent.substr(0, 10)), "File is not open"); + writeFile->append(kDataContent.substr(0, 10)), "File is not open"); auto readFile = gcfs.openFileForRead(gcsFile); std::int64_t size = readFile->size(); EXPECT_EQ(readFile->size(), contentSize); - EXPECT_EQ(readFile->pread(0, size), dataContent); + EXPECT_EQ(readFile->pread(0, size), kDataContent); // Opening an existing file for write must be an error. filesystems::GcsFileSystem newGcfs(testbench_->hiveConfig());