Fixes the bug where the XML output path is affected by test changing the...

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

@@ -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_<number>.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

src/gtest-filepath.cc

@@ -46,8 +46,8 @@
 #include <unistd.h>
 #else
 #include <limits.h>
-#include <sys/stat.h>
-#include <unistd.h>
+#include <sys/stat.h>  // NOLINT
+#include <unistd.h>  // 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
 }
 

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.
 

src/gtest.cc

@@ -69,7 +69,7 @@
 #include <sys/time.h>  // NOLINT
 
 // On z/OS we additionally need strings.h for strcasecmp.
-#include <strings.h>
+#include <strings.h>  // 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();

test/gtest-filepath_test.cc

@@ -52,9 +52,9 @@
 
 #ifdef GTEST_OS_WINDOWS
 #ifdef _WIN32_WCE
-#include <windows.h>
+#include <windows.h>  // NOLINT
 #else
-#include <direct.h>
+#include <direct.h>  // 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

test/gtest-options_test.cc

@@ -40,6 +40,12 @@
 
 #include <gtest/gtest.h>
 
+#ifdef _WIN32_WCE
+#include <windows.h>
+#elif defined(GTEST_OS_WINDOWS)
+#include <direct.h>
+#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