From 650d5bf3ba200ecbeccbfcec6e7b6cc6f40a1f60 Mon Sep 17 00:00:00 2001 From: "zhanyong.wan" Date: Mon, 26 Jan 2009 19:21:32 +0000 Subject: [PATCH] Fixes the bug where the XML output path is affected by test changing the current directory. By Stefan Weigand. --- include/gtest/internal/gtest-filepath.h | 9 ++ src/gtest-filepath.cc | 44 +++++-- src/gtest-internal-inl.h | 7 +- src/gtest.cc | 22 +++- test/gtest-filepath_test.cc | 84 +++++++++++- test/gtest-options_test.cc | 163 +++++++++++++++++++++--- 6 files changed, 288 insertions(+), 41 deletions(-) diff --git a/include/gtest/internal/gtest-filepath.h b/include/gtest/internal/gtest-filepath.h index 07fb86ae..1b2f5869 100644 --- a/include/gtest/internal/gtest-filepath.h +++ b/include/gtest/internal/gtest-filepath.h @@ -93,6 +93,12 @@ class FilePath { int number, const char* extension); + // Given directory = "dir", relative_path = "test.xml", + // returns "dir/test.xml". + // On Windows, uses \ as the separator rather than /. + static FilePath ConcatPaths(const FilePath& directory, + const FilePath& relative_path); + // Returns a pathname for a file that does not currently exist. The pathname // will be directory/base_name.extension or // directory/base_name_.extension if directory/base_name.extension @@ -164,6 +170,9 @@ class FilePath { // root directory per disk drive.) bool IsRootDirectory() const; + // Returns true if pathname describes an absolute path. + bool IsAbsolutePath() const; + private: // Replaces multiple consecutive separators with a single separator. // For example, "bar///foo" becomes "bar/foo". Does not eliminate other diff --git a/src/gtest-filepath.cc b/src/gtest-filepath.cc index 640c27c3..b21b7091 100644 --- a/src/gtest-filepath.cc +++ b/src/gtest-filepath.cc @@ -46,8 +46,8 @@ #include #else #include -#include -#include +#include // NOLINT +#include // NOLINT #endif // _WIN32_WCE or _WIN32 #ifdef GTEST_OS_WINDOWS @@ -144,13 +144,22 @@ FilePath FilePath::MakeFileName(const FilePath& directory, const FilePath& base_name, int number, const char* extension) { - FilePath dir(directory.RemoveTrailingPathSeparator()); - if (number == 0) { - return FilePath(String::Format("%s%c%s.%s", dir.c_str(), kPathSeparator, - base_name.c_str(), extension)); - } - return FilePath(String::Format("%s%c%s_%d.%s", dir.c_str(), kPathSeparator, - base_name.c_str(), number, extension)); + const FilePath file_name( + (number == 0) ? + String::Format("%s.%s", base_name.c_str(), extension) : + String::Format("%s_%d.%s", base_name.c_str(), number, extension)); + return ConcatPaths(directory, file_name); +} + +// Given directory = "dir", relative_path = "test.xml", returns "dir/test.xml". +// On Windows, uses \ as the separator rather than /. +FilePath FilePath::ConcatPaths(const FilePath& directory, + const FilePath& relative_path) { + if (directory.IsEmpty()) + return relative_path; + const FilePath dir(directory.RemoveTrailingPathSeparator()); + return FilePath(String::Format("%s%c%s", dir.c_str(), kPathSeparator, + relative_path.c_str())); } // Returns true if pathname describes something findable in the file-system, @@ -207,13 +216,26 @@ bool FilePath::DirectoryExists() const { bool FilePath::IsRootDirectory() const { #ifdef GTEST_OS_WINDOWS const char* const name = pathname_.c_str(); - return pathname_.GetLength() == 3 && + // TODO(wan@google.com): on Windows a network share like + // \\server\share can be a root directory, although it cannot be the + // current directory. Handle this properly. + return pathname_.GetLength() == 3 && IsAbsolutePath(); +#else + return pathname_ == kPathSeparatorString; +#endif +} + +// Returns true if pathname describes an absolute path. +bool FilePath::IsAbsolutePath() const { + const char* const name = pathname_.c_str(); +#ifdef GTEST_OS_WINDOWS + return pathname_.GetLength() >= 3 && ((name[0] >= 'a' && name[0] <= 'z') || (name[0] >= 'A' && name[0] <= 'Z')) && name[1] == ':' && name[2] == kPathSeparator; #else - return pathname_ == kPathSeparatorString; + return name[0] == kPathSeparator; #endif } diff --git a/src/gtest-internal-inl.h b/src/gtest-internal-inl.h index 5808a50c..28006a2f 100644 --- a/src/gtest-internal-inl.h +++ b/src/gtest-internal-inl.h @@ -799,9 +799,10 @@ class UnitTestOptions { // Returns the output format, or "" for normal printed output. static String GetOutputFormat(); - // Returns the name of the requested output file, or the default if none - // was explicitly specified. - static String GetOutputFile(); + // Returns the absolute path of the requested output file, or the + // default (test_detail.xml in the original working directory) if + // none was explicitly specified. + static String GetAbsolutePathToOutputFile(); // Functions for processing the gtest_filter flag. diff --git a/src/gtest.cc b/src/gtest.cc index 0e3115ba..d6be608a 100644 --- a/src/gtest.cc +++ b/src/gtest.cc @@ -69,7 +69,7 @@ #include // NOLINT // On z/OS we additionally need strings.h for strcasecmp. -#include +#include // NOLINT #elif defined(_WIN32_WCE) // We are on Windows CE. @@ -289,6 +289,7 @@ Mutex g_linked_ptr_mutex(Mutex::NO_CONSTRUCTOR_NEEDED_FOR_STATIC_MUTEX); // Application pathname gotten in InitGoogleTest. String g_executable_path; +String g_original_working_dir; // Returns the current application's name, removing directory path if that // is present. @@ -319,16 +320,27 @@ String UnitTestOptions::GetOutputFormat() { // Returns the name of the requested output file, or the default if none // was explicitly specified. -String UnitTestOptions::GetOutputFile() { +String UnitTestOptions::GetAbsolutePathToOutputFile() { const char* const gtest_output_flag = GTEST_FLAG(output).c_str(); if (gtest_output_flag == NULL) return String(""); const char* const colon = strchr(gtest_output_flag, ':'); if (colon == NULL) - return String(kDefaultOutputFile); + return String(internal::FilePath::ConcatPaths( + internal::FilePath(g_original_working_dir), + internal::FilePath(kDefaultOutputFile)).ToString() ); internal::FilePath output_name(colon + 1); + if (!output_name.IsAbsolutePath()) + // TODO(wan@google.com): on Windows \some\path is not an absolute + // path (as its meaning depends on the current drive), yet the + // following logic for turning it into an absolute path is wrong. + // Fix it. + output_name = internal::FilePath::ConcatPaths( + internal::FilePath(g_original_working_dir), + internal::FilePath(colon + 1)); + if (!output_name.IsDirectory()) return output_name.ToString(); @@ -3675,7 +3687,7 @@ UnitTestEventListenerInterface* UnitTestImpl::result_printer() { const String& output_format = internal::UnitTestOptions::GetOutputFormat(); if (output_format == "xml") { repeater->AddListener(new XmlUnitTestResultPrinter( - internal::UnitTestOptions::GetOutputFile().c_str())); + internal::UnitTestOptions::GetAbsolutePathToOutputFile().c_str())); } else if (output_format != "") { printf("WARNING: unrecognized output format \"%s\" ignored.\n", output_format.c_str()); @@ -3926,6 +3938,8 @@ void InitGoogleTestImpl(int* argc, CharType** argv) { if (*argc <= 0) return; internal::g_executable_path = internal::StreamableToString(argv[0]); + internal::g_original_working_dir = + internal::FilePath::GetCurrentDir().ToString(); #ifdef GTEST_HAS_DEATH_TEST g_argvs.clear(); diff --git a/test/gtest-filepath_test.cc b/test/gtest-filepath_test.cc index d87c7c8c..589442fe 100644 --- a/test/gtest-filepath_test.cc +++ b/test/gtest-filepath_test.cc @@ -52,9 +52,9 @@ #ifdef GTEST_OS_WINDOWS #ifdef _WIN32_WCE -#include +#include // NOLINT #else -#include +#include // NOLINT #endif // _WIN32_WCE #define PATH_SEP "\\" #else @@ -217,6 +217,65 @@ TEST(MakeFileNameTest, GenerateFileNameWithSlashNumberGtZero) { EXPECT_STREQ("foo" PATH_SEP "bar_12.xml", actual.c_str()); } +TEST(MakeFileNameTest, GenerateWhenNumberIsZeroAndDirIsEmpty) { + FilePath actual = FilePath::MakeFileName(FilePath(""), FilePath("bar"), + 0, "xml"); + EXPECT_STREQ("bar.xml", actual.c_str()); +} + +TEST(MakeFileNameTest, GenerateWhenNumberIsNotZeroAndDirIsEmpty) { + FilePath actual = FilePath::MakeFileName(FilePath(""), FilePath("bar"), + 14, "xml"); + EXPECT_STREQ("bar_14.xml", actual.c_str()); +} + +TEST(ConcatPathsTest, WorksWhenDirDoesNotEndWithPathSep) { + FilePath actual = FilePath::ConcatPaths(FilePath("foo"), + FilePath("bar.xml")); + EXPECT_STREQ("foo" PATH_SEP "bar.xml", actual.c_str()); +} + +TEST(ConcatPathsTest, WorksWhenPath1EndsWithPathSep) { + FilePath actual = FilePath::ConcatPaths(FilePath("foo" PATH_SEP), + FilePath("bar.xml")); + EXPECT_STREQ("foo" PATH_SEP "bar.xml", actual.c_str()); +} + +TEST(ConcatPathsTest, Path1BeingEmpty) { + FilePath actual = FilePath::ConcatPaths(FilePath(""), + FilePath("bar.xml")); + EXPECT_STREQ("bar.xml", actual.c_str()); +} + +TEST(ConcatPathsTest, Path2BeingEmpty) { + FilePath actual = FilePath::ConcatPaths(FilePath("foo"), + FilePath("")); + EXPECT_STREQ("foo" PATH_SEP, actual.c_str()); +} + +TEST(ConcatPathsTest, BothPathBeingEmpty) { + FilePath actual = FilePath::ConcatPaths(FilePath(""), + FilePath("")); + EXPECT_STREQ("", actual.c_str()); +} + +TEST(ConcatPathsTest, Path1ContainsPathSep) { + FilePath actual = FilePath::ConcatPaths(FilePath("foo" PATH_SEP "bar"), + FilePath("foobar.xml")); + EXPECT_STREQ("foo" PATH_SEP "bar" PATH_SEP "foobar.xml", actual.c_str()); +} + +TEST(ConcatPathsTest, Path2ContainsPathSep) { + FilePath actual = FilePath::ConcatPaths(FilePath("foo" PATH_SEP), + FilePath("bar" PATH_SEP "bar.xml")); + EXPECT_STREQ("foo" PATH_SEP "bar" PATH_SEP "bar.xml", actual.c_str()); +} + +TEST(ConcatPathsTest, Path2EndsWithPathSep) { + FilePath actual = FilePath::ConcatPaths(FilePath("foo"), + FilePath("bar" PATH_SEP)); + EXPECT_STREQ("foo" PATH_SEP "bar" PATH_SEP, actual.c_str()); +} // RemoveTrailingPathSeparator "" -> "" TEST(RemoveTrailingPathSeparatorTest, EmptyString) { @@ -251,7 +310,7 @@ TEST(RemoveTrailingPathSeparatorTest, ShouldReturnUnmodified) { TEST(DirectoryTest, RootDirectoryExists) { #ifdef GTEST_OS_WINDOWS // We are on Windows. - char current_drive[_MAX_PATH]; + char current_drive[_MAX_PATH]; // NOLINT current_drive[0] = _getdrive() + 'A' - 1; current_drive[1] = ':'; current_drive[2] = '\\'; @@ -268,7 +327,7 @@ TEST(DirectoryTest, RootOfWrongDriveDoesNotExists) { // Find a drive that doesn't exist. Start with 'Z' to avoid common ones. for (char drive = 'Z'; drive >= 'A'; drive--) if (_chdrive(drive - 'A' + 1) == -1) { - char non_drive[_MAX_PATH]; + char non_drive[_MAX_PATH]; // NOLINT non_drive[0] = drive; non_drive[1] = ':'; non_drive[2] = '\\'; @@ -278,14 +337,14 @@ TEST(DirectoryTest, RootOfWrongDriveDoesNotExists) { } _chdrive(saved_drive_); } -#endif // GTEST_OS_WINDOWS +#endif // GTEST_OS_WINDOWS #ifndef _WIN32_WCE // Windows CE _does_ consider an empty directory to exist. TEST(DirectoryTest, EmptyPathDirectoryDoesNotExist) { EXPECT_FALSE(FilePath("").DirectoryExists()); } -#endif // ! _WIN32_WCE +#endif // ! _WIN32_WCE TEST(DirectoryTest, CurrentDirectoryExists) { #ifdef GTEST_OS_WINDOWS // We are on Windows. @@ -529,6 +588,19 @@ TEST(FilePathTest, IsDirectory) { EXPECT_TRUE(FilePath("koala" PATH_SEP).IsDirectory()); } +TEST(FilePathTest, IsAbsolutePath) { + EXPECT_FALSE(FilePath("is" PATH_SEP "relative").IsAbsolutePath()); + EXPECT_FALSE(FilePath("").IsAbsolutePath()); +#ifdef GTEST_OS_WINDOWS + EXPECT_TRUE(FilePath("c:\\" PATH_SEP "is_not" PATH_SEP "relative") + .IsAbsolutePath()); + EXPECT_FALSE(FilePath("c:foo" PATH_SEP "bar").IsAbsolutePath()); +#else + EXPECT_TRUE(FilePath(PATH_SEP "is_not" PATH_SEP "relative") + .IsAbsolutePath()); +#endif // GTEST_OS_WINDOWS +} + } // namespace } // namespace internal } // namespace testing diff --git a/test/gtest-options_test.cc b/test/gtest-options_test.cc index 93f49e20..e3e7bd79 100644 --- a/test/gtest-options_test.cc +++ b/test/gtest-options_test.cc @@ -40,6 +40,12 @@ #include +#ifdef _WIN32_WCE +#include +#elif defined(GTEST_OS_WINDOWS) +#include +#endif // _WIN32_WCE + // Indicates that this translation unit is part of Google Test's // implementation. It must come before gtest-internal-inl.h is // included, or there will be a compiler error. This trick is to @@ -50,10 +56,14 @@ #undef GTEST_IMPLEMENTATION namespace testing { - namespace internal { namespace { +// Turns the given relative path into an absolute path. +FilePath GetAbsolutePathOf(const FilePath& relative_path) { + return FilePath::ConcatPaths(FilePath::GetCurrentDir(), relative_path); +} + // Testing UnitTestOptions::GetOutputFormat/GetOutputFile. TEST(XmlOutputTest, GetOutputFormatDefault) { @@ -68,36 +78,43 @@ TEST(XmlOutputTest, GetOutputFormat) { TEST(XmlOutputTest, GetOutputFileDefault) { GTEST_FLAG(output) = ""; - EXPECT_STREQ("test_detail.xml", - UnitTestOptions::GetOutputFile().c_str()); + EXPECT_STREQ(GetAbsolutePathOf(FilePath("test_detail.xml")).c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); } TEST(XmlOutputTest, GetOutputFileSingleFile) { GTEST_FLAG(output) = "xml:filename.abc"; - EXPECT_STREQ("filename.abc", - UnitTestOptions::GetOutputFile().c_str()); + EXPECT_STREQ(GetAbsolutePathOf(FilePath("filename.abc")).c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); } TEST(XmlOutputTest, GetOutputFileFromDirectoryPath) { #ifdef GTEST_OS_WINDOWS - GTEST_FLAG(output) = "xml:pathname\\"; - const String& output_file = UnitTestOptions::GetOutputFile(); - EXPECT_TRUE(_strcmpi(output_file.c_str(), - "pathname\\gtest-options_test.xml") == 0 || - _strcmpi(output_file.c_str(), - "pathname\\gtest-options-ex_test.xml") == 0) - << " output_file = " << output_file; + GTEST_FLAG(output) = "xml:path\\"; + const String& output_file = UnitTestOptions::GetAbsolutePathToOutputFile(); + EXPECT_TRUE( + _strcmpi(output_file.c_str(), + GetAbsolutePathOf( + FilePath("path\\gtest-options_test.xml")).c_str()) == 0 || + _strcmpi(output_file.c_str(), + GetAbsolutePathOf( + FilePath("path\\gtest-options-ex_test.xml")).c_str()) == 0) + << " output_file = " << output_file; #else - GTEST_FLAG(output) = "xml:pathname/"; - const String& output_file = UnitTestOptions::GetOutputFile(); + GTEST_FLAG(output) = "xml:path/"; + const String& output_file = UnitTestOptions::GetAbsolutePathToOutputFile(); // TODO(wan@google.com): libtool causes the test binary file to be // named lt-gtest-options_test. Therefore the output file may be // named .../lt-gtest-options_test.xml. We should remove this // hard-coded logic when Chandler Carruth's libtool replacement is // ready. - EXPECT_TRUE(output_file == "pathname/gtest-options_test.xml" || - output_file == "pathname/lt-gtest-options_test.xml") - << " output_file = " << output_file; + EXPECT_TRUE(output_file == + GetAbsolutePathOf( + FilePath("path/gtest-options_test.xml")).c_str() || + output_file == + GetAbsolutePathOf( + FilePath("path/lt-gtest-options_test.xml")).c_str()) + << " output_file = " << output_file; #endif } @@ -117,6 +134,118 @@ TEST(OutputFileHelpersTest, GetCurrentExecutableName) { #endif } +class XmlOutputChangeDirTest : public Test { + protected: + virtual void SetUp() { + original_working_dir_ = FilePath::GetCurrentDir(); + ChDir(".."); + // This will make the test fail if run from the root directory. + EXPECT_STRNE(original_working_dir_.c_str(), + FilePath::GetCurrentDir().c_str()); + } + + virtual void TearDown() { + ChDir(original_working_dir_.c_str()); + } + + void ChDir(const char* dir) { +#ifdef GTEST_OS_WINDOWS + _chdir(dir); +#else + chdir(dir); +#endif + } + + FilePath original_working_dir_; +}; + +TEST_F(XmlOutputChangeDirTest, PreserveOriginalWorkingDirWithDefault) { + GTEST_FLAG(output) = ""; + EXPECT_STREQ(FilePath::ConcatPaths(original_working_dir_, + FilePath("test_detail.xml")).c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); +} + +TEST_F(XmlOutputChangeDirTest, PreserveOriginalWorkingDirWithDefaultXML) { + GTEST_FLAG(output) = "xml"; + EXPECT_STREQ(FilePath::ConcatPaths(original_working_dir_, + FilePath("test_detail.xml")).c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); +} + +TEST_F(XmlOutputChangeDirTest, PreserveOriginalWorkingDirWithRelativeFile) { + GTEST_FLAG(output) = "xml:filename.abc"; + EXPECT_STREQ(FilePath::ConcatPaths(original_working_dir_, + FilePath("filename.abc")).c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); +} + +TEST_F(XmlOutputChangeDirTest, PreserveOriginalWorkingDirWithRelativePath) { +#ifdef GTEST_OS_WINDOWS + GTEST_FLAG(output) = "xml:path\\"; + const String& output_file = UnitTestOptions::GetAbsolutePathToOutputFile(); + EXPECT_TRUE( + _strcmpi(output_file.c_str(), + FilePath::ConcatPaths( + original_working_dir_, + FilePath("path\\gtest-options_test.xml")).c_str()) == 0 || + _strcmpi(output_file.c_str(), + FilePath::ConcatPaths( + original_working_dir_, + FilePath("path\\gtest-options-ex_test.xml")).c_str()) == 0) + << " output_file = " << output_file; +#else + GTEST_FLAG(output) = "xml:path/"; + const String& output_file = UnitTestOptions::GetAbsolutePathToOutputFile(); + // TODO(wan@google.com): libtool causes the test binary file to be + // named lt-gtest-options_test. Therefore the output file may be + // named .../lt-gtest-options_test.xml. We should remove this + // hard-coded logic when Chandler Carruth's libtool replacement is + // ready. + EXPECT_TRUE(output_file == FilePath::ConcatPaths(original_working_dir_, + FilePath("path/gtest-options_test.xml")).c_str() || + output_file == FilePath::ConcatPaths(original_working_dir_, + FilePath("path/lt-gtest-options_test.xml")).c_str()) + << " output_file = " << output_file; +#endif +} + +TEST_F(XmlOutputChangeDirTest, PreserveOriginalWorkingDirWithAbsoluteFile) { +#ifdef GTEST_OS_WINDOWS + GTEST_FLAG(output) = "xml:c:\\tmp\\filename.abc"; + EXPECT_STREQ(FilePath("c:\\tmp\\filename.abc").c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); +#else + GTEST_FLAG(output) ="xml:/tmp/filename.abc"; + EXPECT_STREQ(FilePath("/tmp/filename.abc").c_str(), + UnitTestOptions::GetAbsolutePathToOutputFile().c_str()); +#endif +} + +TEST_F(XmlOutputChangeDirTest, PreserveOriginalWorkingDirWithAbsolutePath) { +#ifdef GTEST_OS_WINDOWS + GTEST_FLAG(output) = "xml:c:\\tmp\\"; + const String& output_file = UnitTestOptions::GetAbsolutePathToOutputFile(); + EXPECT_TRUE( + _strcmpi(output_file.c_str(), + FilePath("c:\\tmp\\gtest-options_test.xml").c_str()) == 0 || + _strcmpi(output_file.c_str(), + FilePath("c:\\tmp\\gtest-options-ex_test.xml").c_str()) == 0) + << " output_file = " << output_file; +#else + GTEST_FLAG(output) = "xml:/tmp/"; + const String& output_file = UnitTestOptions::GetAbsolutePathToOutputFile(); + // TODO(wan@google.com): libtool causes the test binary file to be + // named lt-gtest-options_test. Therefore the output file may be + // named .../lt-gtest-options_test.xml. We should remove this + // hard-coded logic when Chandler Carruth's libtool replacement is + // ready. + EXPECT_TRUE(output_file == "/tmp/gtest-options_test.xml" || + output_file == "/tmp/lt-gtest-options_test.xml") + << " output_file = " << output_file; +#endif +} + } // namespace } // namespace internal } // namespace testing