Restructure $XML_OUTPUT_FILE logic

4 files changed, 32 insertions, 26 deletions

diff --git a/googletest/include/gtest/internal/gtest-port.h b/googletest/include/gtest/internal/gtest-port.h
index 2d10ccc..0094ed5 100644
--- a/googletest/include/gtest/internal/gtest-port.h
+++ b/googletest/include/gtest/internal/gtest-port.h
@@ -2546,8 +2546,7 @@ bool ParseInt32(const Message& src_text, const char* str, Int32* value);
 // corresponding to the given Google Test flag.
 bool BoolFromGTestEnv(const char* flag, bool default_val);
 GTEST_API_ Int32 Int32FromGTestEnv(const char* flag, Int32 default_val);
-const char* StringFromGTestEnv(const char* flag, const char* default_val);
-std::string OutputFromGTestEnv(const char* default_val);
+std::string StringFromGTestEnv(const char* flag, const char* default_val);
 
 }  // namespace internal
 }  // namespace testing
diff --git a/googletest/src/gtest-port.cc b/googletest/src/gtest-port.cc
index 05ffca0..e5bf3dd 100644
--- a/googletest/src/gtest-port.cc
+++ b/googletest/src/gtest-port.cc
@@ -1226,30 +1226,31 @@ Int32 Int32FromGTestEnv(const char* flag, Int32 default_value) {
 
 // Reads and returns the string environment variable corresponding to
 // the given flag; if it's not set, returns default_value.
-const char* StringFromGTestEnv(const char* flag, const char* default_value) {
+std::string StringFromGTestEnv(const char* flag, const char* default_value) {
 #if defined(GTEST_GET_STRING_FROM_ENV_)
   return GTEST_GET_STRING_FROM_ENV_(flag, default_value);
 #endif  // defined(GTEST_GET_STRING_FROM_ENV_)
   const std::string env_var = FlagToEnvVar(flag);
-  const char* const value = posix::GetEnv(env_var.c_str());
-  return value == NULL ? default_value : value;
-}
-
-// Reads and returns GTEST_OUTPUT and/or XML_OUTPUT_FILE environment
-// variables; if neither is set, returns default value.
-// XML_OUTPUT_FILE is set by the Bazel build system, and its format is
-// a filename without the "xml:" prefix of GTEST_OUTPUT.
-std::string OutputFromGTestEnv(const char* default_value) {
-#if defined(GTEST_GET_STRING_FROM_ENV_)
-  return GTEST_GET_STRING_FROM_ENV_("output", default_value);
-#endif  // defined(GTEST_GET_STRING_FROM_ENV_)
-  const char* value = StringFromGTestEnv("output", NULL);
-  if (value) {
+  const char* value = posix::GetEnv(env_var.c_str());
+  if (value != NULL) {
     return value;
   }
-  value = posix::GetEnv("XML_OUTPUT_FILE");
-  if (value) {
-    return std::string("xml:") + value;
+
+  // As a special case for the 'output' flag, if GTEST_OUTPUT is not
+  // set, we look for XML_OUTPUT_FILE, which is set by the Bazel build
+  // system.  The value of XML_OUTPUT_FILE is a filename without the
+  // "xml:" prefix of GTEST_OUTPUT.
+  //
+  // The net priority order after flag processing is thus:
+  //   --gtest_output command line flag
+  //   GTEST_OUTPUT environment variable
+  //   XML_OUTPUT_FILE environment variable
+  //   'default_value'
+  if (strcmp(flag, "output") == 0) {
+    value = posix::GetEnv("XML_OUTPUT_FILE");
+    if (value != NULL) {
+      return std::string("xml:") + value;
+    }
   }
   return default_value;
 }
diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc
index 5509f09..2bac245 100644
--- a/googletest/src/gtest.cc
+++ b/googletest/src/gtest.cc
@@ -237,7 +237,7 @@ GTEST_DEFINE_bool_(list_tests, false,
 
 GTEST_DEFINE_string_(
     output,
-    internal::OutputFromGTestEnv(""),
+    internal::StringFromGTestEnv("output", ""),
     "A format (currently must be \"xml\"), optionally followed "
     "by a colon and an output file name or directory. A directory "
     "is indicated by a trailing pathname separator. "
diff --git a/googletest/test/gtest_env_var_test.py b/googletest/test/gtest_env_var_test.py
index 13afb2b..81b5966 100755
--- a/googletest/test/gtest_env_var_test.py
+++ b/googletest/test/gtest_env_var_test.py
@@ -99,18 +99,24 @@ class GTestEnvVarTest(gtest_test_utils.TestCase):
       TestFlag('stack_trace_depth', '0', '100')
 
   def testXmlOutputFile(self):
-      """Test that $XML_OUTPUT_FILE affects the output flag."""
+    """Test that $XML_OUTPUT_FILE affects the output flag."""
 
-      # $XML_OUTPUT_FILE sets output flag
-      SetEnvVar('XML_OUTPUT_FILE', 'tmp/bar.xml')
-      AssertEq('xml:tmp/bar.xml', GetFlag('output'))
+    try:
       # $XML_OUTPUT_FILE is overridden by $GTEST_OUTPUT
       SetEnvVar('GTEST_OUTPUT', 'xml:tmp/foo.xml')
+      SetEnvVar('XML_OUTPUT_FILE', 'tmp/bar.xml')
       AssertEq('xml:tmp/foo.xml', GetFlag('output'))
+
+      # $XML_OUTPUT_FILE without $GTEST_OUTPUT sets output flag
+      SetEnvVar('GTEST_OUTPUT', None)
+      AssertEq('xml:tmp/bar.xml', GetFlag('output'))
+
       # If neither set, flag has default value
       SetEnvVar('XML_OUTPUT_FILE', None)
-      SetEnvVar('GTEST_OUTPUT', None)
       AssertEq('', GetFlag('output'))
+    finally:
+      SetEnvVar('GTEST_OUTPUT', None)
+      SetEnvVar('XML_OUTPUT_FILE', None)
 
 if __name__ == '__main__':
   gtest_test_utils.Main()