From f358a29b35a9059c42aa8e6bcdd7a17136f14647 Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Thu, 24 Sep 2020 14:25:31 +0200 Subject: [PATCH] ARROW-10076: [C++] Use temporary directory facility in all unit tests Closes #8252 from emkornfield/temp_dir Lead-authored-by: Micah Kornfield Co-authored-by: Antoine Pitrou Signed-off-by: Antoine Pitrou --- cpp/src/arrow/io/buffered_test.cc | 10 ++++- cpp/src/arrow/io/file_test.cc | 64 ++++++++++++++++++---------- cpp/src/arrow/ipc/read_write_test.cc | 41 ++++++++++++------ cpp/src/arrow/util/io_util_test.cc | 11 ++++- 4 files changed, 88 insertions(+), 38 deletions(-) diff --git a/cpp/src/arrow/io/buffered_test.cc b/cpp/src/arrow/io/buffered_test.cc index 38b9f51be4e9e..1fefc261b1d33 100644 --- a/cpp/src/arrow/io/buffered_test.cc +++ b/cpp/src/arrow/io/buffered_test.cc @@ -42,11 +42,14 @@ #include "arrow/memory_pool.h" #include "arrow/status.h" #include "arrow/testing/gtest_util.h" +#include "arrow/util/io_util.h" #include "arrow/util/string_view.h" namespace arrow { namespace io { +using ::arrow::internal::TemporaryDir; + static std::string GenerateRandomData(size_t nbytes) { // MSVC doesn't accept uint8_t for std::independent_bits_engine<> typedef unsigned long UInt; // NOLINT @@ -61,7 +64,11 @@ template class FileTestFixture : public ::testing::Test { public: void SetUp() { - path_ = "arrow-test-io-buffered-stream.txt"; + ASSERT_OK_AND_ASSIGN(temp_dir_, TemporaryDir::Make("buffered-test-")); + path_ = temp_dir_->path() + .Join("arrow-test-io-buffered-stream.txt") + .ValueOrDie() + .ToString(); EnsureFileDeleted(); } @@ -79,6 +86,7 @@ class FileTestFixture : public ::testing::Test { int fd_; std::shared_ptr buffered_; std::string path_; + std::unique_ptr temp_dir_; }; // ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/io/file_test.cc b/cpp/src/arrow/io/file_test.cc index 7fca2de3c185e..7d3d1c621cec1 100644 --- a/cpp/src/arrow/io/file_test.cc +++ b/cpp/src/arrow/io/file_test.cc @@ -54,16 +54,22 @@ using internal::FileOpenWritable; using internal::FileRead; using internal::FileSeek; using internal::PlatformFilename; +using internal::TemporaryDir; namespace io { class FileTestFixture : public ::testing::Test { public: void SetUp() { - path_ = "arrow-test-io-file.txt"; + ASSERT_OK_AND_ASSIGN(temp_dir_, TemporaryDir::Make("file-test-")); + path_ = TempFile("arrow-test-io-file.txt"); EnsureFileDeleted(); } + std::string TempFile(arrow::util::string_view path) { + return temp_dir_->path().Join(std::string(path)).ValueOrDie().ToString(); + } + void TearDown() { EnsureFileDeleted(); } void EnsureFileDeleted() { @@ -73,6 +79,7 @@ class FileTestFixture : public ::testing::Test { } protected: + std::unique_ptr temp_dir_; std::string path_; }; @@ -556,13 +563,24 @@ TEST_F(TestPipeIO, ReadableFileFails) { class TestMemoryMappedFile : public ::testing::Test, public MemoryMapFixture { public: - void TearDown() { MemoryMapFixture::TearDown(); } + void SetUp() override { + ASSERT_OK_AND_ASSIGN(temp_dir_, TemporaryDir::Make("memory-map-test-")); + } + + void TearDown() override { MemoryMapFixture::TearDown(); } + + std::string TempFile(arrow::util::string_view path) { + return temp_dir_->path().Join(std::string(path)).ValueOrDie().ToString(); + } + + protected: + std::unique_ptr temp_dir_; }; TEST_F(TestMemoryMappedFile, InvalidUsages) {} TEST_F(TestMemoryMappedFile, ZeroSizeFile) { - std::string path = "io-memory-map-zero-size"; + std::string path = TempFile("io-memory-map-zero-size"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(0, path)); ASSERT_OK_AND_EQ(0, result->Tell()); @@ -578,7 +596,7 @@ TEST_F(TestMemoryMappedFile, MapPartFile) { const int reps = 128; - std::string path = "io-memory-map-offset"; + std::string path = TempFile("io-memory-map-offset"); // file size = 128k CreateFile(path, reps * buffer_size); @@ -622,7 +640,7 @@ TEST_F(TestMemoryMappedFile, WriteRead) { const int reps = 5; - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(reps * buffer_size, path)); int64_t position = 0; @@ -641,7 +659,7 @@ TEST_F(TestMemoryMappedFile, ReadAsync) { std::vector buffer(buffer_size); random_bytes(1024, 0, buffer.data()); - std::string path = "io-memory-map-read-async-test"; + std::string path = TempFile("io-memory-map-read-async-test"); ASSERT_OK_AND_ASSIGN(auto mmap, InitMemoryMap(buffer_size, path)); ASSERT_OK(mmap->Write(buffer.data(), buffer_size)); @@ -661,7 +679,7 @@ TEST_F(TestMemoryMappedFile, WillNeed) { std::vector buffer(buffer_size); random_bytes(1024, 0, buffer.data()); - std::string path = "io-memory-map-will-need-test"; + std::string path = TempFile("io-memory-map-will-need-test"); ASSERT_OK_AND_ASSIGN(auto mmap, InitMemoryMap(buffer_size, path)); ASSERT_OK(mmap->Write(buffer.data(), buffer_size)); @@ -672,7 +690,7 @@ TEST_F(TestMemoryMappedFile, WillNeed) { } TEST_F(TestMemoryMappedFile, InvalidReads) { - std::string path = "io-memory-map-invalid-reads-test"; + std::string path = TempFile("io-memory-map-invalid-reads-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(4096, path)); uint8_t buffer[10]; @@ -692,7 +710,7 @@ TEST_F(TestMemoryMappedFile, WriteResizeRead) { random_bytes(buffer_size, 0, b.data()); } - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); int64_t position = 0; @@ -716,7 +734,7 @@ TEST_F(TestMemoryMappedFile, ResizeRaisesOnExported) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); ASSERT_OK(result->Write(buffer.data(), buffer_size)); @@ -745,7 +763,7 @@ TEST_F(TestMemoryMappedFile, WriteReadZeroInitSize) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(0, path)); ASSERT_OK(result->Resize(buffer_size)); @@ -761,7 +779,7 @@ TEST_F(TestMemoryMappedFile, WriteThenShrink) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size * 2, path)); ASSERT_OK(result->Resize(buffer_size)); @@ -780,7 +798,7 @@ TEST_F(TestMemoryMappedFile, WriteThenShrinkToHalfThenWrite) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); ASSERT_OK(result->Write(buffer.data(), buffer_size)); @@ -808,7 +826,7 @@ TEST_F(TestMemoryMappedFile, ResizeToZeroThanWrite) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); // just a sanity check that writing works ook @@ -841,7 +859,7 @@ TEST_F(TestMemoryMappedFile, WriteAt) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); ASSERT_OK(result->WriteAt(0, buffer.data(), buffer_size / 2)); @@ -859,7 +877,7 @@ TEST_F(TestMemoryMappedFile, WriteBeyondEnd) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); ASSERT_OK(result->Seek(1)); @@ -875,7 +893,7 @@ TEST_F(TestMemoryMappedFile, WriteAtBeyondEnd) { std::vector buffer(buffer_size); random_bytes(buffer_size, 0, buffer.data()); - std::string path = "io-memory-map-write-read-test"; + std::string path = TempFile("io-memory-map-write-read-test"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(buffer_size, path)); // Attempt to write beyond end of memory map @@ -886,7 +904,7 @@ TEST_F(TestMemoryMappedFile, WriteAtBeyondEnd) { } TEST_F(TestMemoryMappedFile, GetSize) { - std::string path = "io-memory-map-get-size"; + std::string path = TempFile("io-memory-map-get-size"); ASSERT_OK_AND_ASSIGN(auto result, InitMemoryMap(16384, path)); ASSERT_OK_AND_EQ(16384, result->GetSize()); @@ -902,7 +920,7 @@ TEST_F(TestMemoryMappedFile, ReadOnly) { const int reps = 5; - std::string path = "ipc-read-only-test"; + std::string path = TempFile("ipc-read-only-test"); ASSERT_OK_AND_ASSIGN(auto rwmmap, InitMemoryMap(reps * buffer_size, path)); int64_t position = 0; @@ -933,7 +951,7 @@ TEST_F(TestMemoryMappedFile, LARGE_MEMORY_TEST(ReadWriteOver4GbFile)) { const int64_t reps = 5000; - std::string path = "ipc-read-over-4gb-file-test"; + std::string path = TempFile("ipc-read-over-4gb-file-test"); ASSERT_OK_AND_ASSIGN(auto rwmmap, InitMemoryMap(reps * buffer_size, path)); AppendFile(path); @@ -964,7 +982,7 @@ TEST_F(TestMemoryMappedFile, RetainMemoryMapReference) { random_bytes(1024, 0, buffer.data()); - std::string path = "ipc-read-only-test"; + std::string path = TempFile("ipc-read-only-test"); CreateFile(path, buffer_size); { @@ -995,7 +1013,7 @@ TEST_F(TestMemoryMappedFile, InvalidMode) { random_bytes(1024, 0, buffer.data()); - std::string path = "ipc-invalid-mode-test"; + std::string path = TempFile("ipc-invalid-mode-test"); CreateFile(path, buffer_size); ASSERT_OK_AND_ASSIGN(auto rommap, MemoryMappedFile::Open(path, FileMode::READ)); @@ -1015,7 +1033,7 @@ TEST_F(TestMemoryMappedFile, CastableToFileInterface) { TEST_F(TestMemoryMappedFile, ThreadSafety) { std::string data = "foobar"; - std::string path = "ipc-multithreading-test"; + std::string path = TempFile("ipc-multithreading-test"); CreateFile(path, static_cast(data.size())); ASSERT_OK_AND_ASSIGN(auto file, MemoryMappedFile::Open(path, FileMode::READWRITE)); diff --git a/cpp/src/arrow/ipc/read_write_test.cc b/cpp/src/arrow/ipc/read_write_test.cc index d622a2bd6b064..ceb3156177258 100644 --- a/cpp/src/arrow/ipc/read_write_test.cc +++ b/cpp/src/arrow/ipc/read_write_test.cc @@ -49,6 +49,7 @@ #include "arrow/type.h" #include "arrow/util/bit_util.h" #include "arrow/util/checked_cast.h" +#include "arrow/util/io_util.h" #include "arrow/util/key_value_metadata.h" #include "generated/Message_generated.h" // IWYU pragma: keep @@ -57,6 +58,7 @@ namespace arrow { using internal::checked_cast; using internal::GetByteWidth; +using internal::TemporaryDir; namespace ipc { @@ -375,7 +377,14 @@ class ExtensionTypesMixin { class IpcTestFixture : public io::MemoryMapFixture, public ExtensionTypesMixin { public: - void SetUp() { options_ = IpcWriteOptions::Defaults(); } + void SetUp() { + options_ = IpcWriteOptions::Defaults(); + ASSERT_OK_AND_ASSIGN(temp_dir_, TemporaryDir::Make("ipc-test-")); + } + + std::string TempFile(util::string_view file) { + return temp_dir_->path().Join(std::string(file)).ValueOrDie().ToString(); + } void DoSchemaRoundTrip(const Schema& schema, std::shared_ptr* result) { ASSERT_OK_AND_ASSIGN(std::shared_ptr serialized_schema, @@ -437,8 +446,8 @@ class IpcTestFixture : public io::MemoryMapFixture, public ExtensionTypesMixin { int64_t buffer_size = 1 << 20) { std::stringstream ss; ss << "test-write-row-batch-" << g_file_number++; - ASSERT_OK_AND_ASSIGN(mmap_, - io::MemoryMapFixture::InitMemoryMap(buffer_size, ss.str())); + ASSERT_OK_AND_ASSIGN( + mmap_, io::MemoryMapFixture::InitMemoryMap(buffer_size, TempFile(ss.str()))); std::shared_ptr schema_result; DoSchemaRoundTrip(*batch.schema(), &schema_result); @@ -469,6 +478,7 @@ class IpcTestFixture : public io::MemoryMapFixture, public ExtensionTypesMixin { protected: std::shared_ptr mmap_; IpcWriteOptions options_; + std::unique_ptr temp_dir_; }; TEST(MetadataVersion, ForwardsCompatCheck) { @@ -644,10 +654,9 @@ TEST_F(TestWriteRecordBatch, SliceTruncatesBinaryOffsets) { auto batch = RecordBatch::Make(schema, array->length(), {array}); auto sliced_batch = batch->Slice(0, 5); - std::stringstream ss; - ss << "test-truncate-offsets"; ASSERT_OK_AND_ASSIGN( - mmap_, io::MemoryMapFixture::InitMemoryMap(/*buffer_size=*/1 << 20, ss.str())); + mmap_, io::MemoryMapFixture::InitMemoryMap(/*buffer_size=*/1 << 20, + TempFile("test-truncate-offsets"))); DictionaryMemo dictionary_memo; ASSERT_OK_AND_ASSIGN( auto result, @@ -732,10 +741,9 @@ TEST_F(TestWriteRecordBatch, RoundtripPreservesBufferSizes) { auto arr = rg.String(length, 0, 10, 0.1); auto batch = RecordBatch::Make(::arrow::schema({field("f0", utf8())}), length, {arr}); - std::stringstream ss; - ss << "test-roundtrip-buffer-sizes"; ASSERT_OK_AND_ASSIGN( - mmap_, io::MemoryMapFixture::InitMemoryMap(/*buffer_size=*/1 << 20, ss.str())); + mmap_, io::MemoryMapFixture::InitMemoryMap( + /*buffer_size=*/1 << 20, TempFile("test-roundtrip-buffer-sizes"))); DictionaryMemo dictionary_memo; ASSERT_OK_AND_ASSIGN( auto result, @@ -787,7 +795,15 @@ TEST_F(TestWriteRecordBatch, IntegerGetRecordBatchSize) { class RecursionLimits : public ::testing::Test, public io::MemoryMapFixture { public: - void SetUp() { pool_ = default_memory_pool(); } + void SetUp() { + pool_ = default_memory_pool(); + ASSERT_OK_AND_ASSIGN(temp_dir_, TemporaryDir::Make("ipc-recursion-limits-test-")); + } + + std::string TempFile(util::string_view file) { + return temp_dir_->path().Join(std::string(file)).ValueOrDie().ToString(); + } + void TearDown() { io::MemoryMapFixture::TearDown(); } Status WriteToMmap(int recursion_level, bool override_level, int32_t* metadata_length, @@ -813,8 +829,8 @@ class RecursionLimits : public ::testing::Test, public io::MemoryMapFixture { std::stringstream ss; ss << "test-write-past-max-recursion-" << g_file_number++; const int memory_map_size = 1 << 20; - ARROW_ASSIGN_OR_RAISE(mmap_, - io::MemoryMapFixture::InitMemoryMap(memory_map_size, ss.str())); + ARROW_ASSIGN_OR_RAISE( + mmap_, io::MemoryMapFixture::InitMemoryMap(memory_map_size, TempFile(ss.str()))); auto options = IpcWriteOptions::Defaults(); if (override_level) { @@ -826,6 +842,7 @@ class RecursionLimits : public ::testing::Test, public io::MemoryMapFixture { protected: std::shared_ptr mmap_; + std::unique_ptr temp_dir_; MemoryPool* pool_; }; diff --git a/cpp/src/arrow/util/io_util_test.cc b/cpp/src/arrow/util/io_util_test.cc index 74ee16dc690b7..d84a2b76e3992 100644 --- a/cpp/src/arrow/util/io_util_test.cc +++ b/cpp/src/arrow/util/io_util_test.cc @@ -1,4 +1,5 @@ // Licensed to the Apache Software Foundation (ASF) under one +// std::unique_ptr temp_dir; // or more contributor license agreements. See the NOTICE file // distributed with this work for additional information // regarding copyright ownership. The ASF licenses this file @@ -332,7 +333,10 @@ TEST(PlatformFilename, Parent) { } TEST(CreateDirDeleteDir, Basics) { - const std::string BASE = "xxx-io-util-test-dir"; + std::unique_ptr temp_dir; + ASSERT_OK_AND_ASSIGN(temp_dir, TemporaryDir::Make("deletedirtest-")); + const std::string BASE = + temp_dir->path().Join("xxx-io-util-test-dir2").ValueOrDie().ToString(); bool created, deleted; PlatformFilename parent, child; @@ -378,7 +382,10 @@ TEST(CreateDirDeleteDir, Basics) { } TEST(DeleteDirContents, Basics) { - const std::string BASE = "xxx-io-util-test-dir2"; + std::unique_ptr temp_dir; + ASSERT_OK_AND_ASSIGN(temp_dir, TemporaryDir::Make("deletedirtest-")); + const std::string BASE = + temp_dir->path().Join("xxx-io-util-test-dir2").ValueOrDie().ToString(); bool created, deleted; PlatformFilename parent, child1, child2;