This is an automated email from the ASF dual-hosted git repository. uwe pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push: new b492975 ARROW-5449: [C++] Test extended-length paths on Windows b492975 is described below commit b492975faa68913a91c7a4758f125b4678f06a8b Author: Antoine Pitrou <anto...@python.org> AuthorDate: Thu Jun 6 10:27:31 2019 -0400 ARROW-5449: [C++] Test extended-length paths on Windows This helps validate that UNC paths (e.g. //server/share/...) should work. Backslashes are still untested. Author: Antoine Pitrou <anto...@python.org> Closes #4487 from pitrou/ARROW-5449-win-paths and squashes the following commits: 539ef4ab <Antoine Pitrou> ARROW-5449: Test extended-length paths on Windows --- cpp/src/arrow/filesystem/localfs-test.cc | 87 +++++++++++++++++++++++++------- cpp/src/arrow/filesystem/test-util.h | 44 +++++++++------- 2 files changed, 94 insertions(+), 37 deletions(-) diff --git a/cpp/src/arrow/filesystem/localfs-test.cc b/cpp/src/arrow/filesystem/localfs-test.cc index ef36ea4..cd2dfc5 100644 --- a/cpp/src/arrow/filesystem/localfs-test.cc +++ b/cpp/src/arrow/filesystem/localfs-test.cc @@ -33,6 +33,7 @@ namespace arrow { namespace fs { namespace internal { +using ::arrow::internal::PlatformFilename; using ::arrow::internal::TemporaryDir; TimePoint CurrentTimePoint() { @@ -41,51 +42,101 @@ TimePoint CurrentTimePoint() { std::chrono::duration_cast<TimePoint::duration>(now.time_since_epoch())); } +class LocalFSTestMixin : public ::testing::Test { + public: + void SetUp() override { ASSERT_OK(TemporaryDir::Make("test-localfs-", &temp_dir_)); } + + protected: + std::unique_ptr<TemporaryDir> temp_dir_; +}; + +struct CommonPathFormatter { + std::string operator()(const PlatformFilename& fn) { return fn.ToString(); } +}; + +#ifdef _WIN32 +struct ExtendedLengthPathFormatter { + std::string operator()(const PlatformFilename& fn) { return "//?/" + fn.ToString(); } +}; + +using PathFormatters = ::testing::Types<CommonPathFormatter, ExtendedLengthPathFormatter>; +#else +using PathFormatters = ::testing::Types<CommonPathFormatter>; +#endif + //////////////////////////////////////////////////////////////////////////// // Generic LocalFileSystem tests -class TestLocalFSGeneric : public ::testing::Test, public GenericFileSystemTest { +template <typename PathFormatter> +class TestLocalFSGeneric : public LocalFSTestMixin, public GenericFileSystemTest { public: void SetUp() override { - ASSERT_OK(TemporaryDir::Make("test-localfs-", &temp_dir_)); + LocalFSTestMixin::SetUp(); local_fs_ = std::make_shared<LocalFileSystem>(); - fs_ = std::make_shared<SubTreeFileSystem>(temp_dir_->path().ToString(), local_fs_); + auto path = PathFormatter()(temp_dir_->path()); + fs_ = std::make_shared<SubTreeFileSystem>(path, local_fs_); } protected: std::shared_ptr<FileSystem> GetEmptyFileSystem() override { return fs_; } - std::unique_ptr<TemporaryDir> temp_dir_; std::shared_ptr<LocalFileSystem> local_fs_; std::shared_ptr<FileSystem> fs_; }; -GENERIC_FS_TEST_FUNCTIONS(TestLocalFSGeneric); +TYPED_TEST_CASE(TestLocalFSGeneric, PathFormatters); + +GENERIC_FS_TYPED_TEST_FUNCTIONS(TestLocalFSGeneric); //////////////////////////////////////////////////////////////////////////// // Concrete LocalFileSystem tests -class TestLocalFS : public ::testing::Test { +template <typename PathFormatter> +class TestLocalFS : public LocalFSTestMixin { public: void SetUp() { - ASSERT_OK(TemporaryDir::Make("test-localfs-", &temp_dir_)); + LocalFSTestMixin::SetUp(); local_fs_ = std::make_shared<LocalFileSystem>(); - fs_ = std::make_shared<SubTreeFileSystem>(temp_dir_->path().ToString(), local_fs_); + auto path = PathFormatter()(temp_dir_->path()); + fs_ = std::make_shared<SubTreeFileSystem>(path, local_fs_); } protected: - std::unique_ptr<TemporaryDir> temp_dir_; std::shared_ptr<LocalFileSystem> local_fs_; std::shared_ptr<FileSystem> fs_; }; -TEST_F(TestLocalFS, DirectoryMTime) { +TYPED_TEST_CASE(TestLocalFS, PathFormatters); + +TYPED_TEST(TestLocalFS, CorrectPathExists) { + // Test that the right location on disk is accessed + std::shared_ptr<io::OutputStream> stream; + ASSERT_OK(this->fs_->OpenOutputStream("abc", &stream)); + std::string data = "some data"; + auto data_size = static_cast<int64_t>(data.size()); + ASSERT_OK(stream->Write(data.data(), data_size)); + ASSERT_OK(stream->Close()); + + // Now check the file's existence directly, bypassing the FileSystem abstraction + auto path = this->temp_dir_->path().ToString() + "/abc"; + PlatformFilename fn; + int fd; + int64_t size = -1; + ASSERT_OK(PlatformFilename::FromString(path, &fn)); + ASSERT_OK(::arrow::internal::FileOpenReadable(fn, &fd)); + Status st = ::arrow::internal::FileGetSize(fd, &size); + ASSERT_OK(::arrow::internal::FileClose(fd)); + ASSERT_OK(st); + ASSERT_EQ(size, data_size); +} + +TYPED_TEST(TestLocalFS, DirectoryMTime) { TimePoint t1 = CurrentTimePoint(); - ASSERT_OK(fs_->CreateDir("AB/CD/EF")); + ASSERT_OK(this->fs_->CreateDir("AB/CD/EF")); TimePoint t2 = CurrentTimePoint(); std::vector<FileStats> stats; - ASSERT_OK(fs_->GetTargetStats({"AB", "AB/CD/EF"}, &stats)); + ASSERT_OK(this->fs_->GetTargetStats({"AB", "AB/CD/EF"}, &stats)); ASSERT_EQ(stats.size(), 2); AssertFileStats(stats[0], "AB", FileType::Directory); AssertFileStats(stats[1], "AB/CD/EF", FileType::Directory); @@ -100,14 +151,14 @@ TEST_F(TestLocalFS, DirectoryMTime) { AssertDurationBetween(t2 - stats[1].mtime(), -kTimeSlack, kTimeSlack); } -TEST_F(TestLocalFS, FileMTime) { +TYPED_TEST(TestLocalFS, FileMTime) { TimePoint t1 = CurrentTimePoint(); - ASSERT_OK(fs_->CreateDir("AB/CD")); - CreateFile(fs_.get(), "AB/CD/ab", "data"); + ASSERT_OK(this->fs_->CreateDir("AB/CD")); + CreateFile(this->fs_.get(), "AB/CD/ab", "data"); TimePoint t2 = CurrentTimePoint(); std::vector<FileStats> stats; - ASSERT_OK(fs_->GetTargetStats({"AB", "AB/CD/ab"}, &stats)); + ASSERT_OK(this->fs_->GetTargetStats({"AB", "AB/CD/ab"}, &stats)); ASSERT_EQ(stats.size(), 2); AssertFileStats(stats[0], "AB", FileType::Directory); AssertFileStats(stats[1], "AB/CD/ab", FileType::File, 4); @@ -117,8 +168,8 @@ TEST_F(TestLocalFS, FileMTime) { AssertDurationBetween(t2 - stats[1].mtime(), -kTimeSlack, kTimeSlack); } -// TODO check UNC paths on Windows, e.g. "\\server\share\path\file" (networked) -// or "\\?\C:\path\file" (extended-length paths) +// TODO Should we test backslash paths on Windows? +// SubTreeFileSystem isn't compatible with them. } // namespace internal } // namespace fs diff --git a/cpp/src/arrow/filesystem/test-util.h b/cpp/src/arrow/filesystem/test-util.h index 179b08c..ee2e67d 100644 --- a/cpp/src/arrow/filesystem/test-util.h +++ b/cpp/src/arrow/filesystem/test-util.h @@ -102,25 +102,31 @@ class ARROW_EXPORT GenericFileSystemTest { void TestOpenInputFile(FileSystem* fs); }; -#define GENERIC_FS_TEST_FUNCTION(TEST_CLASS, NAME) \ - TEST_F(TEST_CLASS, NAME) { Test##NAME(); } - -#define GENERIC_FS_TEST_FUNCTIONS(TEST_CLASS) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, Empty) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, CreateDir) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, DeleteDir) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, DeleteFile) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, DeleteFiles) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, MoveFile) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, MoveDir) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, CopyFile) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, GetTargetStatsSingle) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, GetTargetStatsVector) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, GetTargetStatsSelector) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, OpenOutputStream) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, OpenAppendStream) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, OpenInputStream) \ - GENERIC_FS_TEST_FUNCTION(TEST_CLASS, OpenInputFile) +#define GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, NAME) \ + TEST_MACRO(TEST_CLASS, NAME) { this->Test##NAME(); } + +#define GENERIC_FS_TEST_FUNCTIONS_MACROS(TEST_MACRO, TEST_CLASS) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, Empty) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, CreateDir) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, DeleteDir) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, DeleteFile) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, DeleteFiles) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, MoveFile) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, MoveDir) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, CopyFile) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, GetTargetStatsSingle) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, GetTargetStatsVector) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, GetTargetStatsSelector) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, OpenOutputStream) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, OpenAppendStream) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, OpenInputStream) \ + GENERIC_FS_TEST_FUNCTION(TEST_MACRO, TEST_CLASS, OpenInputFile) + +#define GENERIC_FS_TEST_FUNCTIONS(TEST_CLASS) \ + GENERIC_FS_TEST_FUNCTIONS_MACROS(TEST_F, TEST_CLASS) + +#define GENERIC_FS_TYPED_TEST_FUNCTIONS(TEST_CLASS) \ + GENERIC_FS_TEST_FUNCTIONS_MACROS(TYPED_TEST, TEST_CLASS) } // namespace fs } // namespace arrow