Makes gtest compile cleanly with MSVC's /W4 (by Zhanyong Wan).

Renames EventListenrs to TestEventListeners (by Zhanyong Wan).
Fixes invalid characters in XML report (by Vlad Losev).
Refacotrs SConscript (by Vlad Losev).

include/gtest/gtest-death-test.h

@@ -245,10 +245,10 @@ class KilledBySignal {
 #ifdef NDEBUG
 
 #define EXPECT_DEBUG_DEATH(statement, regex) \
-  do { statement; } while (false)
+  do { statement; } while (::testing::internal::AlwaysFalse())
 
 #define ASSERT_DEBUG_DEATH(statement, regex) \
-  do { statement; } while (false)
+  do { statement; } while (::testing::internal::AlwaysFalse())
 
 #else
 

include/gtest/gtest-spi.h

@@ -150,7 +150,7 @@ class SingleFailureChecker {
           INTERCEPT_ONLY_CURRENT_THREAD, &gtest_failures);\
       GTestExpectFatalFailureHelper::Execute();\
     }\
-  } while (false)
+  } while (::testing::internal::AlwaysFalse())
 
 #define EXPECT_FATAL_FAILURE_ON_ALL_THREADS(statement, substr) \
   do { \
@@ -167,7 +167,7 @@ class SingleFailureChecker {
           INTERCEPT_ALL_THREADS, &gtest_failures);\
       GTestExpectFatalFailureHelper::Execute();\
     }\
-  } while (false)
+  } while (::testing::internal::AlwaysFalse())
 
 // A macro for testing Google Test assertions or code that's expected to
 // generate Google Test non-fatal failures.  It asserts that the given
@@ -190,8 +190,17 @@ class SingleFailureChecker {
 // Note that even though the implementations of the following two
 // macros are much alike, we cannot refactor them to use a common
 // helper macro, due to some peculiarity in how the preprocessor
-// works.  The AcceptsMacroThatExpandsToUnprotectedComma test in
+// works.  If we do that, the code won't compile when the user gives
-// gtest_unittest.cc will fail to compile if we do that.
+// EXPECT_NONFATAL_FAILURE() a statement that contains a macro that
+// expands to code containing an unprotected comma.  The
+// AcceptsMacroThatExpandsToUnprotectedComma test in gtest_unittest.cc
+// catches that.
+//
+// For the same reason, we have to write
+//   if (::testing::internal::AlwaysTrue()) { statement; }
+// instead of
+//   GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement)
+// to avoid an MSVC warning on unreachable code.
 #define EXPECT_NONFATAL_FAILURE(statement, substr) \
   do {\
     ::testing::TestPartResultArray gtest_failures;\
@@ -202,9 +211,9 @@ class SingleFailureChecker {
       ::testing::ScopedFakeTestPartResultReporter gtest_reporter(\
           ::testing::ScopedFakeTestPartResultReporter:: \
           INTERCEPT_ONLY_CURRENT_THREAD, &gtest_failures);\
-      statement;\
+      if (::testing::internal::AlwaysTrue()) { statement; }\
     }\
-  } while (false)
+  } while (::testing::internal::AlwaysFalse())
 
 #define EXPECT_NONFATAL_FAILURE_ON_ALL_THREADS(statement, substr) \
   do {\
@@ -216,8 +225,8 @@ class SingleFailureChecker {
       ::testing::ScopedFakeTestPartResultReporter gtest_reporter(\
           ::testing::ScopedFakeTestPartResultReporter::INTERCEPT_ALL_THREADS,\
           &gtest_failures);\
-      statement;\
+      if (::testing::internal::AlwaysTrue()) { statement; }\
     }\
-  } while (false)
+  } while (::testing::internal::AlwaysFalse())
 
 #endif  // GTEST_INCLUDE_GTEST_GTEST_SPI_H_

include/gtest/gtest.h

@@ -146,13 +146,13 @@ namespace internal {
 
 class AssertHelper;
 class DefaultGlobalTestPartResultReporter;
-class EventListenersAccessor;
 class ExecDeathTest;
 class NoExecDeathTest;
 class FinalSuccessChecker;
 class GTestFlagSaver;
 class TestInfoImpl;
 class TestResultAccessor;
+class TestEventListenersAccessor;
 class TestEventRepeater;
 class WindowsDeathTest;
 class UnitTestImpl* GetUnitTestImpl();
@@ -832,11 +832,11 @@ class EmptyTestEventListener : public TestEventListener {
   virtual void OnTestProgramEnd(const UnitTest& /*unit_test*/) {}
 };
 
-// EventListeners lets users add listeners to track events in Google Test.
+// TestEventListeners lets users add listeners to track events in Google Test.
-class EventListeners {
+class TestEventListeners {
  public:
-  EventListeners();
+  TestEventListeners();
-  ~EventListeners();
+  ~TestEventListeners();
 
   // Appends an event listener to the end of the list. Google Test assumes
   // the ownership of the listener (i.e. it will delete the listener when
@@ -871,8 +871,8 @@ class EventListeners {
  private:
   friend class TestCase;
   friend class internal::DefaultGlobalTestPartResultReporter;
-  friend class internal::EventListenersAccessor;
   friend class internal::NoExecDeathTest;
+  friend class internal::TestEventListenersAccessor;
   friend class internal::TestInfoImpl;
   friend class internal::UnitTestImpl;
 
@@ -906,8 +906,8 @@ class EventListeners {
   // Listener responsible for the creation of the XML output file.
   TestEventListener* default_xml_generator_;
 
-  // We disallow copying EventListeners.
+  // We disallow copying TestEventListeners.
-  GTEST_DISALLOW_COPY_AND_ASSIGN_(EventListeners);
+  GTEST_DISALLOW_COPY_AND_ASSIGN_(TestEventListeners);
 };
 
 // A UnitTest consists of a vector of TestCases.
@@ -1002,7 +1002,7 @@ class UnitTest {
 
   // Returns the list of event listeners that can be used to track events
   // inside Google Test.
-  EventListeners& listeners();
+  TestEventListeners& listeners();
 
  private:
   // Registers and returns a global test environment.  When a test

include/gtest/internal/gtest-death-test-internal.h

@@ -153,7 +153,7 @@ bool ExitedUnsuccessfully(int exit_status);
 // ASSERT_EXIT*, and EXPECT_EXIT*.
 #define GTEST_DEATH_TEST_(statement, predicate, regex, fail) \
   GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
-  if (true) { \
+  if (::testing::internal::AlwaysTrue()) { \
     const ::testing::internal::RE& gtest_regex = (regex); \
     ::testing::internal::DeathTest* gtest_dt; \
     if (!::testing::internal::DeathTest::Create(#statement, &gtest_regex, \
@@ -259,7 +259,7 @@ InternalRunDeathTestFlag* ParseInternalRunDeathTestFlag();
       GTEST_LOG_(WARNING) \
           << "Death tests are not supported on this platform.\n" \
           << "Statement '" #statement "' cannot be verified."; \
-    } else if (!::testing::internal::AlwaysTrue()) { \
+    } else if (::testing::internal::AlwaysFalse()) { \
       ::testing::internal::RE::PartialMatch(".*", (regex)); \
       GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \
       terminator; \

include/gtest/internal/gtest-internal.h

@@ -745,9 +745,15 @@ class TypeParameterizedTestCase<Fixture, Templates0, Types> {
 // the trace but Bar() and GetCurrentOsStackTraceExceptTop() won't.
 String GetCurrentOsStackTraceExceptTop(UnitTest* unit_test, int skip_count);
 
-// A helper for suppressing warnings on unreachable code in some macros.
+// Helpers for suppressing warnings on unreachable code or constant
+// condition.
+
+// Always returns true.
 bool AlwaysTrue();
 
+// Always returns false.
+inline bool AlwaysFalse() { return !AlwaysTrue(); }
+
 // A simple Linear Congruential Generator for generating random
 // numbers with a uniform distribution.  Unlike rand() and srand(), it
 // doesn't use global state (and therefore can't interfere with user
@@ -854,7 +860,7 @@ class Random {
 
 #define GTEST_TEST_BOOLEAN_(boolexpr, booltext, actual, expected, fail) \
   GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
-  if (boolexpr) \
+  if (::testing::internal::IsTrue(boolexpr)) \
     ; \
   else \
     fail("Value of: " booltext "\n  Actual: " #actual "\nExpected: " #expected)

include/gtest/internal/gtest-port.h

@@ -577,6 +577,10 @@ typedef ::std::stringstream StrStream;
 typedef ::std::strstream StrStream;
 #endif  // GTEST_HAS_STD_STRING
 
+// A helper for suppressing warnings on constant condition.  It just
+// returns 'condition'.
+bool IsTrue(bool condition);
+
 // Defines scoped_ptr.
 
 // This implementation of scoped_ptr is PARTIAL - it only contains
@@ -599,7 +603,7 @@ class scoped_ptr {
 
   void reset(T* p = NULL) {
     if (p != ptr_) {
-      if (sizeof(T) > 0) {  // Makes sure T is a complete type.
+      if (IsTrue(sizeof(T) > 0)) {  // Makes sure T is a complete type.
         delete ptr_;
       }
       ptr_ = p;
@@ -1037,7 +1041,7 @@ typedef TypeWithSize<8>::Int TimeInMillis;  // Represents time in milliseconds.
 //    whether it is built in the debug mode or not.
 #define GTEST_CHECK_(condition) \
     GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
-    if (condition) \
+    if (::testing::internal::IsTrue(condition)) \
       ; \
     else \
       GTEST_LOG_(FATAL) << "Condition " #condition " failed. "

samples/sample10_unittest.cc

@@ -37,10 +37,10 @@
 #include <gtest/gtest.h>
 
 using ::testing::EmptyTestEventListener;
-using ::testing::EventListeners;
 using ::testing::InitGoogleTest;
 using ::testing::Test;
 using ::testing::TestCase;
+using ::testing::TestEventListeners;
 using ::testing::TestInfo;
 using ::testing::TestPartResult;
 using ::testing::UnitTest;
@@ -123,7 +123,7 @@ int main(int argc, char **argv) {
   // If we are given the --check_for_leaks command line flag, installs the
   // leak checker.
   if (check_for_leaks) {
-    EventListeners& listeners = UnitTest::GetInstance()->listeners();
+    TestEventListeners& listeners = UnitTest::GetInstance()->listeners();
 
     // Adds the leak checker to the end of the test event listener list,
     // after the default text output printer and the default XML report

samples/sample9_unittest.cc

@@ -37,10 +37,10 @@
 #include <gtest/gtest.h>
 
 using ::testing::EmptyTestEventListener;
-using ::testing::EventListeners;
 using ::testing::InitGoogleTest;
 using ::testing::Test;
 using ::testing::TestCase;
+using ::testing::TestEventListeners;
 using ::testing::TestInfo;
 using ::testing::TestPartResult;
 using ::testing::UnitTest;
@@ -120,7 +120,7 @@ int main(int argc, char **argv) {
   // If we are given the --terse_output command line flag, suppresses the
   // standard output and attaches own result printer.
   if (terse_output) {
-    EventListeners& listeners = unit_test.listeners();
+    TestEventListeners& listeners = unit_test.listeners();
 
     // Removes the default console output listener from the list so it will
     // not receive events from Google Test and won't print any output. Since

scons/SConscript

@@ -96,29 +96,119 @@ import os
 ############################################################
 # Environments for building the targets, sorted by name.
 
-def NewEnvironment(env, type):
-  """Copies environment and gives it suffix for names of targets built in it."""
 
-  if type:
+class EnvCreator:
-    suffix = '_' + type
+  """Creates new customized environments from a base one."""
-  else:
-    suffix = ''
 
-  new_env = env.Clone()
+  @staticmethod
-  new_env['OBJ_SUFFIX'] = suffix
+  def _Remove(env, attribute, value):
-  return new_env;
+    """Removes the given attribute value from the environment."""
 
+    attribute_values = env[attribute]
+    if value in attribute_values:
+      attribute_values.remove(value)
 
-def Remove(env, attribute, value):
+  @staticmethod
-  """Removes the given attribute value from the environment."""
+  def Create(base_env, modifier=None):
+    # User should NOT create more than one environment with the same
+    # modifier (including None).
+    new_env = env.Clone()
+    if modifier:
+      modifier(new_env)
+    else:
+      new_env['OBJ_SUFFIX'] = ''  # Default suffix for unchanged environment.
 
-  attribute_values = env[attribute]
+    return new_env;
-  if value in attribute_values:
+
-    attribute_values.remove(value)
+  # Each of the following methods modifies the environment for a particular
+  # purpose and can be used by clients for creating new environments.  Each
+  # one needs to set the OBJ_SUFFIX variable to a unique suffix to
+  # differentiate targets built with that environment.  Otherwise, SCons may
+  # complain about same target built with different settings.
+
+  @staticmethod
+  def UseOwnTuple(env):
+    """Instructs Google Test to use its internal implementation of tuple."""
+
+    env['OBJ_SUFFIX'] = '_use_own_tuple'
+    env.Append(CPPDEFINES = 'GTEST_USE_OWN_TR1_TUPLE=1')
+
+  @staticmethod
+  def WarningOk(env):
+    """Does not treat warnings as errors.
+
+    Necessary for compiling gtest_unittest.cc, which triggers a gcc
+    warning when testing EXPECT_EQ(NULL, ptr)."""
+
+    env['OBJ_SUFFIX'] = '_warning_ok'
+    if env['PLATFORM'] == 'win32':
+      EnvCreator._Remove(env, 'CCFLAGS', '-WX')
+    else:
+      EnvCreator._Remove(env, 'CCFLAGS', '-Werror')
+
+  @staticmethod
+  def WithExceptions(env):
+    """Re-enables exceptions."""
+
+    # We compile gtest_unittest in this environment which means we need to
+    # allow warnings here as well.
+    EnvCreator.WarningOk(env)
+    env['OBJ_SUFFIX'] = '_ex'  # Overrides the suffix supplied by WarningOK.
+    if env['PLATFORM'] == 'win32':
+      env.Append(CCFLAGS=['/EHsc'])
+      env.Append(CPPDEFINES='_HAS_EXCEPTIONS=1')
+      # Undoes the _TYPEINFO_ hack, which is unnecessary and only creates
+      # trouble when exceptions are enabled.
+      EnvCreator._Remove(env, 'CPPDEFINES', '_TYPEINFO_')
+      EnvCreator._Remove(env, 'CPPDEFINES', '_HAS_EXCEPTIONS=0')
+    else:
+      env.Append(CCFLAGS='-fexceptions')
+      EnvCreator._Remove(env, 'CCFLAGS', '-fno-exceptions')
+
+  @staticmethod
+  def LessOptimized(env):
+    """Disables certain optimizations on Windows.
+
+    We need to disable some optimization flags for some tests on
+    Windows; otherwise the redirection of stdout does not work
+    (apparently because of a compiler bug)."""
+
+    env['OBJ_SUFFIX'] = '_less_optimized'
+    if env['PLATFORM'] == 'win32':
+      for flag in ['/O1', '/Os', '/Og', '/Oy']:
+        EnvCreator._Remove(env, 'LINKFLAGS', flag)
+
+  @staticmethod
+  def WithThreads(env):
+    """Allows use of threads.
+
+    Currently only enables pthreads under GCC."""
+
+    env['OBJ_SUFFIX'] = '_with_threads'
+    if env['PLATFORM'] != 'win32':
+      # Assuming POSIX-like environment with GCC.
+      # TODO(vladl@google.com): sniff presence of pthread_atfork instead of
+      # selecting on a platform.
+      env.Append(CCFLAGS=['-pthread'])
+      env.Append(LINKFLAGS=['-pthread'])
+
+  @staticmethod
+  def NoRtti(env):
+    """Disables RTTI support."""
+
+    # We compile gtest_unittest in this environment which means we need to
+    # allow warnings here as well.
+    EnvCreator.WarningOk(env)
+    env['OBJ_SUFFIX'] = '_no_rtti'  # Overrides suffix supplied by WarningOK.
+    if env['PLATFORM'] == 'win32':
+      env.Append(CCFLAGS=['/GR-'])
+    else:
+      env.Append(CCFLAGS=['-fno-rtti'])
+      env.Append(CPPDEFINES='GTEST_HAS_RTTI=0')
 
 
 Import('env')
-env = NewEnvironment(env, '')
+env = EnvCreator.Create(env)
 
 # Note: The relative paths in SConscript files are relative to the location
 # of the SConscript file itself. To make a path relative to the location of
@@ -133,51 +223,12 @@ env = NewEnvironment(env, '')
 # file is one directory deeper than the gtest directory.
 env.Prepend(CPPPATH = ['..', '../include'])
 
-env_use_own_tuple = NewEnvironment(env, 'use_own_tuple')
+env_use_own_tuple = EnvCreator.Create(env, EnvCreator.UseOwnTuple)
-env_use_own_tuple.Append(CPPDEFINES = 'GTEST_USE_OWN_TR1_TUPLE=1')
+env_warning_ok = EnvCreator.Create(env, EnvCreator.WarningOk)
-
+env_with_exceptions = EnvCreator.Create(env, EnvCreator.WithExceptions)
-# Needed to allow gtest_unittest.cc, which triggers a gcc warning when
+env_less_optimized = EnvCreator.Create(env, EnvCreator.LessOptimized)
-# testing EXPECT_EQ(NULL, ptr), to compile.
+env_with_threads = EnvCreator.Create(env, EnvCreator.WithThreads)
-env_warning_ok = NewEnvironment(env, 'warning_ok')
+env_without_rtti = EnvCreator.Create(env, EnvCreator.NoRtti)
-if env_warning_ok['PLATFORM'] == 'win32':
-  Remove(env_warning_ok, 'CCFLAGS', '-WX')
-else:
-  Remove(env_warning_ok, 'CCFLAGS', '-Werror')
-
-env_with_exceptions = NewEnvironment(env_warning_ok, 'ex')
-if env_with_exceptions['PLATFORM'] == 'win32':
-  env_with_exceptions.Append(CCFLAGS=['/EHsc'])
-  env_with_exceptions.Append(CPPDEFINES='_HAS_EXCEPTIONS=1')
-  # Undoes the _TYPEINFO_ hack, which is unnecessary and only creates
-  # trouble when exceptions are enabled.
-  Remove(env_with_exceptions, 'CPPDEFINES', '_TYPEINFO_')
-  Remove(env_with_exceptions, 'CPPDEFINES', '_HAS_EXCEPTIONS=0')
-else:
-  env_with_exceptions.Append(CCFLAGS='-fexceptions')
-  Remove(env_with_exceptions, 'CCFLAGS', '-fno-exceptions')
-
-# We need to disable some optimization flags for some tests on
-# Windows; otherwise the redirection of stdout does not work
-# (apparently because of a compiler bug).
-env_less_optimized = NewEnvironment(env, 'less_optimized')
-if env_less_optimized['PLATFORM'] == 'win32':
-  for flag in ['/O1', '/Os', '/Og', '/Oy']:
-    Remove(env_less_optimized, 'LINKFLAGS', flag)
-
-# Assuming POSIX-like environment with GCC.
-# TODO(vladl@google.com): sniff presence of pthread_atfork instead of
-# selecting on a platform.
-env_with_threads = NewEnvironment(env, 'with_threads')
-if env_with_threads['PLATFORM'] != 'win32':
-  env_with_threads.Append(CCFLAGS=['-pthread'])
-  env_with_threads.Append(LINKFLAGS=['-pthread'])
-
-env_without_rtti = NewEnvironment(env_warning_ok, 'no_rtti')
-if env_without_rtti['PLATFORM'] == 'win32':
-  env_without_rtti.Append(CCFLAGS=['/GR-'])
-else:
-  env_without_rtti.Append(CCFLAGS=['-fno-rtti'])
-  env_without_rtti.Append(CPPDEFINES='GTEST_HAS_RTTI=0')
 
 ############################################################
 # Helpers for creating build targets.
@@ -372,7 +423,7 @@ gtest_exports = {'gtest': gtest,
                  'gtest_ex': gtest_ex,
                  'gtest_no_rtti': gtest_no_rtti,
                  'gtest_use_own_tuple': gtest_use_own_tuple,
-                 'NewEnvironment': NewEnvironment,
+                 'EnvCreator': EnvCreator,
                  'GtestObject': GtestObject,
                  'GtestBinary': GtestBinary,
                  'GtestTest': GtestTest}

scons/SConstruct.common

@@ -99,11 +99,6 @@ class SConstructHelper:
                  # Disables warnings that are either uninteresting or
                  # hard to fix.
 
-                 '/wd4127',
-                 # constant conditional expression.  The macro
-                 # GTEST_IS_NULL_LITERAL_() triggers it and I cannot find
-                 # a fix.
-
                  '-WX',             # Treat warning as errors
                  #'-GR-',           # Disable runtime type information
                  '-RTCs',           # Enable stack-frame run-time error checks

src/gtest-death-test.cc

@@ -220,12 +220,12 @@ void DeathTestAbort(const String& message) {
 // fails.
 #define GTEST_DEATH_TEST_CHECK_(expression) \
   do { \
-    if (!(expression)) { \
+    if (!::testing::internal::IsTrue(expression)) { \
-      DeathTestAbort(::testing::internal::String::Format(\
+      DeathTestAbort(::testing::internal::String::Format( \
           "CHECK failed: File %s, line %d: %s", \
           __FILE__, __LINE__, #expression)); \
     } \
-  } while (0)
+  } while (::testing::internal::AlwaysFalse())
 
 // This macro is similar to GTEST_DEATH_TEST_CHECK_, but it is meant for
 // evaluating any system call that fulfills two conditions: it must return
@@ -241,11 +241,11 @@ void DeathTestAbort(const String& message) {
       gtest_retval = (expression); \
     } while (gtest_retval == -1 && errno == EINTR); \
     if (gtest_retval == -1) { \
-      DeathTestAbort(::testing::internal::String::Format(\
+      DeathTestAbort(::testing::internal::String::Format( \
           "CHECK failed: File %s, line %d: %s != -1", \
           __FILE__, __LINE__, #expression)); \
     } \
-  } while (0)
+  } while (::testing::internal::AlwaysFalse())
 
 // Returns the message describing the last system error in errno.
 String GetLastErrnoDescription() {
@@ -581,8 +581,8 @@ int WindowsDeathTest::Wait() {
       WAIT_OBJECT_0 == ::WaitForSingleObject(child_handle_.Get(),
                                              INFINITE));
   DWORD status;
-  GTEST_DEATH_TEST_CHECK_(::GetExitCodeProcess(child_handle_.Get(),
+  GTEST_DEATH_TEST_CHECK_(::GetExitCodeProcess(child_handle_.Get(), &status)
-                                               &status));
+                          != FALSE);
   child_handle_.Reset();
   set_status(static_cast<int>(status));
   return this->status();
@@ -612,9 +612,10 @@ DeathTest::TestRole WindowsDeathTest::AssumeRole() {
   SECURITY_ATTRIBUTES handles_are_inheritable = {
     sizeof(SECURITY_ATTRIBUTES), NULL, TRUE };
   HANDLE read_handle, write_handle;
-  GTEST_DEATH_TEST_CHECK_(::CreatePipe(&read_handle, &write_handle,
+  GTEST_DEATH_TEST_CHECK_(
-                                       &handles_are_inheritable,
+      ::CreatePipe(&read_handle, &write_handle, &handles_are_inheritable,
-                                       0));  // Default buffer size.
+                   0)  // Default buffer size.
+      != FALSE);
   set_read_fd(::_open_osfhandle(reinterpret_cast<intptr_t>(read_handle),
                                 O_RDONLY));
   write_handle_.Reset(write_handle);
@@ -677,7 +678,7 @@ DeathTest::TestRole WindowsDeathTest::AssumeRole() {
       NULL,   // Inherit the parent's environment.
       UnitTest::GetInstance()->original_working_dir(),
       &startup_info,
-      &process_info));
+      &process_info) != FALSE);
   child_handle_.Reset(process_info.hProcess);
   ::CloseHandle(process_info.hThread);
   set_spawned(true);
@@ -1042,7 +1043,7 @@ static void SplitString(const ::std::string& str, char delimiter,
                         ::std::vector< ::std::string>* dest) {
   ::std::vector< ::std::string> parsed;
   ::std::string::size_type pos = 0;
-  while (true) {
+  while (::testing::internal::AlwaysTrue()) {
     const ::std::string::size_type colon = str.find(delimiter, pos);
     if (colon == ::std::string::npos) {
       parsed.push_back(str.substr(pos));

src/gtest-internal-inl.h

@@ -742,7 +742,7 @@ class UnitTestImpl {
   }
 
   // Provides access to the event listener list.
-  EventListeners* listeners() { return &listeners_; }
+  TestEventListeners* listeners() { return &listeners_; }
 
   // Returns the TestResult for the test that's currently running, or
   // the TestResult for the ad hoc test if no test is running.
@@ -1002,7 +1002,7 @@ class UnitTestImpl {
 
   // The list of event listeners that can be used to track events inside
   // Google Test.
-  EventListeners listeners_;
+  TestEventListeners listeners_;
 
   // The OS stack trace getter.  Will be deleted when the UnitTest
   // object is destructed.  By default, an OsStackTraceGetter is used,

src/gtest.cc

@@ -42,6 +42,8 @@
 #include <wchar.h>
 #include <wctype.h>
 
+#include <ostream>
+
 #if GTEST_OS_LINUX
 
 // TODO(kenton@google.com): Use autoconf to detect availability of
@@ -2966,6 +2968,9 @@ class XmlUnitTestResultPrinter : public EmptyTestEventListener {
   // with character references.
   static String EscapeXml(const char* str, bool is_attribute);
 
+  // Returns the given string with all characters invalid in XML removed.
+  static String RemoveInvalidXmlCharacters(const char* str);
+
   // Convenience wrapper around EscapeXml when str is an attribute value.
   static String EscapeXmlAttribute(const char* str) {
     return EscapeXml(str, true);
@@ -2974,10 +2979,13 @@ class XmlUnitTestResultPrinter : public EmptyTestEventListener {
   // Convenience wrapper around EscapeXml when str is not an attribute value.
   static String EscapeXmlText(const char* str) { return EscapeXml(str, false); }
 
-  // Prints an XML representation of a TestInfo object.
+  // Streams an XML CDATA section, escaping invalid CDATA sequences as needed.
-  static void PrintXmlTestInfo(FILE* out,
+  static void OutputXmlCDataSection(::std::ostream* stream, const char* data);
-                               const char* test_case_name,
+
-                               const TestInfo& test_info);
+  // Streams an XML representation of a TestInfo object.
+  static void OutputXmlTestInfo(::std::ostream* stream,
+                                const char* test_case_name,
+                                const TestInfo& test_info);
 
   // Prints an XML representation of a TestCase object
   static void PrintXmlTestCase(FILE* out, const TestCase& test_case);
@@ -3092,6 +3100,22 @@ String XmlUnitTestResultPrinter::EscapeXml(const char* str, bool is_attribute) {
   return m.GetString();
 }
 
+// Returns the given string with all characters invalid in XML removed.
+// Currently invalid characters are dropped from the string. An
+// alternative is to replace them with certain characters such as . or ?.
+String XmlUnitTestResultPrinter::RemoveInvalidXmlCharacters(const char* str) {
+  char* const output = new char[strlen(str) + 1];
+  char* appender = output;
+  for (char ch = *str; ch != '\0'; ch = *++str)
+    if (IsValidXmlCharacter(ch))
+      *appender++ = ch;
+  *appender = '\0';
+
+  String ret_value(output);
+  delete[] output;
+  return ret_value;
+}
+
 // The following routines generate an XML representation of a UnitTest
 // object.
 //
@@ -3118,40 +3142,62 @@ const char* FormatTimeInMillisAsSeconds(TimeInMillis ms) {
   return str.c_str();
 }
 
+// Streams an XML CDATA section, escaping invalid CDATA sequences as needed.
+void XmlUnitTestResultPrinter::OutputXmlCDataSection(::std::ostream* stream,
+                                                     const char* data) {
+  const char* segment = data;
+  *stream << "<![CDATA[";
+  for (;;) {
+    const char* const next_segment = strstr(segment, "]]>");
+    if (next_segment != NULL) {
+      stream->write(segment, next_segment - segment);
+      *stream << "]]>]]&gt;<![CDATA[";
+      segment = next_segment + strlen("]]>");
+    } else {
+      *stream << segment;
+      break;
+    }
+  }
+  *stream << "]]>";
+}
+
 // Prints an XML representation of a TestInfo object.
 // TODO(wan): There is also value in printing properties with the plain printer.
-void XmlUnitTestResultPrinter::PrintXmlTestInfo(FILE* out,
+void XmlUnitTestResultPrinter::OutputXmlTestInfo(::std::ostream* stream,
-                                                const char* test_case_name,
+                                                 const char* test_case_name,
-                                                const TestInfo& test_info) {
+                                                 const TestInfo& test_info) {
   const TestResult& result = *test_info.result();
-  fprintf(out,
+  *stream << "    <testcase name=\""
-          "    <testcase name=\"%s\" status=\"%s\" time=\"%s\" "
+          << EscapeXmlAttribute(test_info.name()).c_str()
-          "classname=\"%s\"%s",
+          << "\" status=\""
-          EscapeXmlAttribute(test_info.name()).c_str(),
+          << (test_info.should_run() ? "run" : "notrun")
-          test_info.should_run() ? "run" : "notrun",
+          << "\" time=\""
-          FormatTimeInMillisAsSeconds(result.elapsed_time()),
+          << FormatTimeInMillisAsSeconds(result.elapsed_time())
-          EscapeXmlAttribute(test_case_name).c_str(),
+          << "\" classname=\"" << EscapeXmlAttribute(test_case_name).c_str()
-          TestPropertiesAsXmlAttributes(result).c_str());
+          << "\"" << TestPropertiesAsXmlAttributes(result).c_str();
 
   int failures = 0;
   for (int i = 0; i < result.total_part_count(); ++i) {
     const TestPartResult& part = result.GetTestPartResult(i);
     if (part.failed()) {
-      const String message = String::Format("%s:%d\n%s", part.file_name(),
-                                            part.line_number(), part.message());
       if (++failures == 1)
-        fprintf(out, ">\n");
+        *stream << ">\n";
-      fprintf(out,
+      *stream << "      <failure message=\""
-              "      <failure message=\"%s\" type=\"\"><![CDATA[%s]]>"
+              << EscapeXmlAttribute(part.summary()).c_str()
-              "</failure>\n",
+              << "\" type=\"\">";
-              EscapeXmlAttribute(part.summary()).c_str(), message.c_str());
+      const String message = RemoveInvalidXmlCharacters(String::Format(
+          "%s:%d\n%s",
+          part.file_name(), part.line_number(),
+          part.message()).c_str());
+      OutputXmlCDataSection(stream, message.c_str());
+      *stream << "</failure>\n";
     }
   }
 
   if (failures == 0)
-    fprintf(out, " />\n");
+    *stream << " />\n";
   else
-    fprintf(out, "    </testcase>\n");
+    *stream << "    </testcase>\n";
 }
 
 // Prints an XML representation of a TestCase object
@@ -3167,8 +3213,11 @@ void XmlUnitTestResultPrinter::PrintXmlTestCase(FILE* out,
   fprintf(out,
           "errors=\"0\" time=\"%s\">\n",
           FormatTimeInMillisAsSeconds(test_case.elapsed_time()));
-  for (int i = 0; i < test_case.total_test_count(); ++i)
+  for (int i = 0; i < test_case.total_test_count(); ++i) {
-    PrintXmlTestInfo(out, test_case.name(), *test_case.GetTestInfo(i));
+    StrStream stream;
+    OutputXmlTestInfo(&stream, test_case.name(), *test_case.GetTestInfo(i));
+    fprintf(out, "%s", StrStreamToString(&stream).c_str());
+  }
   fprintf(out, "  </testsuite>\n");
 }
 
@@ -3253,28 +3302,28 @@ OsStackTraceGetter::kElidedFramesMarker =
 
 }  // namespace internal
 
-// class EventListeners
+// class TestEventListeners
 
-EventListeners::EventListeners()
+TestEventListeners::TestEventListeners()
     : repeater_(new internal::TestEventRepeater()),
       default_result_printer_(NULL),
       default_xml_generator_(NULL) {
 }
 
-EventListeners::~EventListeners() { delete repeater_; }
+TestEventListeners::~TestEventListeners() { delete repeater_; }
 
 // Returns the standard listener responsible for the default console
 // output.  Can be removed from the listeners list to shut down default
 // console output.  Note that removing this object from the listener list
 // with Release transfers its ownership to the user.
-void EventListeners::Append(TestEventListener* listener) {
+void TestEventListeners::Append(TestEventListener* listener) {
   repeater_->Append(listener);
 }
 
 // Removes the given event listener from the list and returns it.  It then
 // becomes the caller's responsibility to delete the listener. Returns
 // NULL if the listener is not found in the list.
-TestEventListener* EventListeners::Release(TestEventListener* listener) {
+TestEventListener* TestEventListeners::Release(TestEventListener* listener) {
   if (listener == default_result_printer_)
     default_result_printer_ = NULL;
   else if (listener == default_xml_generator_)
@@ -3284,14 +3333,14 @@ TestEventListener* EventListeners::Release(TestEventListener* listener) {
 
 // Returns repeater that broadcasts the TestEventListener events to all
 // subscribers.
-TestEventListener* EventListeners::repeater() { return repeater_; }
+TestEventListener* TestEventListeners::repeater() { return repeater_; }
 
 // Sets the default_result_printer attribute to the provided listener.
 // The listener is also added to the listener list and previous
 // default_result_printer is removed from it and deleted. The listener can
 // also be NULL in which case it will not be added to the list. Does
 // nothing if the previous and the current listener objects are the same.
-void EventListeners::SetDefaultResultPrinter(TestEventListener* listener) {
+void TestEventListeners::SetDefaultResultPrinter(TestEventListener* listener) {
   if (default_result_printer_ != listener) {
     // It is an error to pass this method a listener that is already in the
     // list.
@@ -3307,7 +3356,7 @@ void EventListeners::SetDefaultResultPrinter(TestEventListener* listener) {
 // default_xml_generator is removed from it and deleted. The listener can
 // also be NULL in which case it will not be added to the list. Does
 // nothing if the previous and the current listener objects are the same.
-void EventListeners::SetDefaultXmlGenerator(TestEventListener* listener) {
+void TestEventListeners::SetDefaultXmlGenerator(TestEventListener* listener) {
   if (default_xml_generator_ != listener) {
     // It is an error to pass this method a listener that is already in the
     // list.
@@ -3320,11 +3369,11 @@ void EventListeners::SetDefaultXmlGenerator(TestEventListener* listener) {
 
 // Controls whether events will be forwarded by the repeater to the
 // listeners in the list.
-bool EventListeners::EventForwardingEnabled() const {
+bool TestEventListeners::EventForwardingEnabled() const {
   return repeater_->forwarding_enabled();
 }
 
-void EventListeners::SuppressEventForwarding() {
+void TestEventListeners::SuppressEventForwarding() {
   repeater_->set_forwarding_enabled(false);
 }
 
@@ -3418,7 +3467,7 @@ const TestCase* UnitTest::GetTestCase(int i) const {
 
 // Returns the list of event listeners that can be used to track events
 // inside Google Test.
-EventListeners& UnitTest::listeners() {
+TestEventListeners& UnitTest::listeners() {
   return *impl()->listeners();
 }
 
@@ -4187,11 +4236,13 @@ namespace {
 class ClassUniqueToAlwaysTrue {};
 }
 
+bool IsTrue(bool condition) { return condition; }
+
 bool AlwaysTrue() {
 #if GTEST_HAS_EXCEPTIONS
   // This condition is always false so AlwaysTrue() never actually throws,
   // but it makes the compiler think that it may throw.
-  if (atoi("42") == 36)  // NOLINT
+  if (IsTrue(false))
     throw ClassUniqueToAlwaysTrue();
 #endif  // GTEST_HAS_EXCEPTIONS
   return true;

test/gtest-death-test_test.cc

@@ -35,6 +35,9 @@
 #include <gtest/gtest.h>
 #include <gtest/internal/gtest-filepath.h>
 
+using testing::internal::AlwaysFalse;
+using testing::internal::AlwaysTrue;
+
 #if GTEST_HAS_DEATH_TEST
 
 #if GTEST_OS_WINDOWS
@@ -271,21 +274,21 @@ TEST(ExitStatusPredicateTest, KilledBySignal) {
 // be followed by operator<<, and that in either case the complete text
 // comprises only a single C++ statement.
 TEST_F(TestForDeathTest, SingleStatement) {
-  if (false)
+  if (AlwaysFalse())
     // This would fail if executed; this is a compilation test only
     ASSERT_DEATH(return, "");
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_DEATH(_exit(1), "");
   else
     // This empty "else" branch is meant to ensure that EXPECT_DEATH
     // doesn't expand into an "if" statement without an "else"
     ;
 
-  if (false)
+  if (AlwaysFalse())
     ASSERT_DEATH(return, "") << "did not die";
 
-  if (false)
+  if (AlwaysFalse())
     ;
   else
     EXPECT_DEATH(_exit(1), "") << 1 << 2 << 3;
@@ -1188,21 +1191,21 @@ TEST(ConditionalDeathMacrosTest, AssertDeatDoesNotReturnhIfUnsupported) {
 //
 // The syntax should work whether death tests are available or not.
 TEST(ConditionalDeathMacrosSyntaxDeathTest, SingleStatement) {
-  if (false)
+  if (AlwaysFalse())
     // This would fail if executed; this is a compilation test only
     ASSERT_DEATH_IF_SUPPORTED(return, "");
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_DEATH_IF_SUPPORTED(_exit(1), "");
   else
     // This empty "else" branch is meant to ensure that EXPECT_DEATH
     // doesn't expand into an "if" statement without an "else"
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     ASSERT_DEATH_IF_SUPPORTED(return, "") << "did not die";
 
-  if (false)
+  if (AlwaysFalse())
     ;  // NOLINT
   else
     EXPECT_DEATH_IF_SUPPORTED(_exit(1), "") << 1 << 2 << 3;

test/gtest-port_test.cc

@@ -54,16 +54,16 @@ namespace testing {
 namespace internal {
 
 TEST(GtestCheckSyntaxTest, BehavesLikeASingleStatement) {
-  if (false)
+  if (AlwaysFalse())
     GTEST_CHECK_(false) << "This should never be executed; "
                            "It's a compilation test only.";
 
-  if (true)
+  if (AlwaysTrue())
     GTEST_CHECK_(true);
   else
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     ;  // NOLINT
   else
     GTEST_CHECK_(true) << "";

test/gtest-typed-test_test.cc

@@ -29,8 +29,8 @@
 //
 // Author: wan@google.com (Zhanyong Wan)
 
-#include <list>
 #include <set>
+#include <vector>
 
 #include "test/gtest-typed-test_test.h"
 #include <gtest/gtest.h>
@@ -57,7 +57,9 @@ class CommonTest : public Test {
   // This 'protected:' is optional.  There's no harm in making all
   // members of this fixture class template public.
  protected:
-  typedef std::list<T> List;
+  // We used to use std::list here, but switched to std::vector since
+  // MSVC's <list> doesn't compile cleanly with /W4.
+  typedef std::vector<T> Vector;
   typedef std::set<int> IntSet;
 
   CommonTest() : value_(1) {}
@@ -99,7 +101,7 @@ TYPED_TEST(CommonTest, ValuesAreCorrect) {
 
   // Typedefs in the fixture class template can be visited via the
   // "typename TestFixture::" prefix.
-  typename TestFixture::List empty;
+  typename TestFixture::Vector empty;
   EXPECT_EQ(0U, empty.size());
 
   typename TestFixture::IntSet empty2;
@@ -314,7 +316,7 @@ INSTANTIATE_TYPED_TEST_CASE_P(Double, TypedTestP2, Types<double>);
 // Tests that the same type-parameterized test case can be
 // instantiated in different translation units linked together.
 // (ContainerTest is also instantiated in gtest-typed-test_test.cc.)
-typedef Types<std::list<double>, std::set<char> > MyContainers;
+typedef Types<std::vector<double>, std::set<char> > MyContainers;
 INSTANTIATE_TYPED_TEST_CASE_P(My, ContainerTest, MyContainers);
 
 // Tests that a type-parameterized test case can be defined and

test/gtest_repeat_test.cc

@@ -64,14 +64,14 @@ namespace {
   do {\
     const int expected_val = (expected);\
     const int actual_val = (actual);\
-    if (expected_val != actual_val) {\
+    if (::testing::internal::IsTrue(expected_val != actual_val)) {\
       ::std::cout << "Value of: " #actual "\n"\
                   << "  Actual: " << actual_val << "\n"\
                   << "Expected: " #expected "\n"\
                   << "Which is: " << expected_val << "\n";\
       abort();\
     }\
-  } while(false)
+  } while(::testing::internal::AlwaysFalse())
 
 
 // Used for verifying that global environment set-up and tear-down are

test/gtest_unittest.cc

@@ -80,32 +80,33 @@ TEST(CommandLineFlagsTest, CanBeAccessedInCodeOnceGTestHIsIncluded) {
 
 namespace testing {
 namespace internal {
-const char* FormatTimeInMillisAsSeconds(TimeInMillis ms);
 
+bool ShouldUseColor(bool stdout_is_tty);
+const char* FormatTimeInMillisAsSeconds(TimeInMillis ms);
 bool ParseInt32Flag(const char* str, const char* flag, Int32* value);
 
-// Provides access to otherwise private parts of the EventListeners class
+// Provides access to otherwise private parts of the TestEventListeners class
 // that are needed to test it.
-class EventListenersAccessor {
+class TestEventListenersAccessor {
  public:
-  static TestEventListener* GetRepeater(EventListeners* listeners) {
+  static TestEventListener* GetRepeater(TestEventListeners* listeners) {
     return listeners->repeater();
   }
 
-  static void SetDefaultResultPrinter(EventListeners* listeners,
+  static void SetDefaultResultPrinter(TestEventListeners* listeners,
                                       TestEventListener* listener) {
     listeners->SetDefaultResultPrinter(listener);
   }
-  static void SetDefaultXmlGenerator(EventListeners* listeners,
+  static void SetDefaultXmlGenerator(TestEventListeners* listeners,
                                      TestEventListener* listener) {
     listeners->SetDefaultXmlGenerator(listener);
   }
 
-  static bool EventForwardingEnabled(const EventListeners& listeners) {
+  static bool EventForwardingEnabled(const TestEventListeners& listeners) {
     return listeners.EventForwardingEnabled();
   }
 
-  static void SuppressEventForwarding(EventListeners* listeners) {
+  static void SuppressEventForwarding(TestEventListeners* listeners) {
     listeners->SuppressEventForwarding();
   }
 };
@@ -113,26 +114,11 @@ class EventListenersAccessor {
 }  // namespace internal
 }  // namespace testing
 
-using testing::internal::FormatTimeInMillisAsSeconds;
-using testing::internal::ParseInt32Flag;
-using testing::internal::EventListenersAccessor;
-
-namespace testing {
-
-GTEST_DECLARE_string_(output);
-GTEST_DECLARE_string_(color);
-
-namespace internal {
-bool ShouldUseColor(bool stdout_is_tty);
-}  // namespace internal
-}  // namespace testing
-
 using testing::AssertionFailure;
 using testing::AssertionResult;
 using testing::AssertionSuccess;
 using testing::DoubleLE;
 using testing::EmptyTestEventListener;
-using testing::EventListeners;
 using testing::FloatLE;
 using testing::GTEST_FLAG(also_run_disabled_tests);
 using testing::GTEST_FLAG(break_on_failure);
@@ -155,16 +141,20 @@ using testing::Message;
 using testing::ScopedFakeTestPartResultReporter;
 using testing::StaticAssertTypeEq;
 using testing::Test;
+using testing::TestEventListeners;
 using testing::TestCase;
 using testing::TestPartResult;
 using testing::TestPartResultArray;
 using testing::TestProperty;
 using testing::TestResult;
 using testing::UnitTest;
+using testing::internal::AlwaysFalse;
+using testing::internal::AlwaysTrue;
 using testing::internal::AppendUserMessage;
 using testing::internal::CodePointToUtf8;
 using testing::internal::EqFailure;
 using testing::internal::FloatingPoint;
+using testing::internal::FormatTimeInMillisAsSeconds;
 using testing::internal::GTestFlagSaver;
 using testing::internal::GetCurrentOsStackTraceExceptTop;
 using testing::internal::GetNextRandomSeed;
@@ -174,11 +164,13 @@ using testing::internal::GetTypeId;
 using testing::internal::GetUnitTestImpl;
 using testing::internal::Int32;
 using testing::internal::Int32FromEnvOrDie;
+using testing::internal::ParseInt32Flag;
 using testing::internal::ShouldRunTestOnShard;
 using testing::internal::ShouldShard;
 using testing::internal::ShouldUseColor;
 using testing::internal::StreamableToString;
 using testing::internal::String;
+using testing::internal::TestEventListenersAccessor;
 using testing::internal::TestResultAccessor;
 using testing::internal::ThreadLocal;
 using testing::internal::UInt32;
@@ -3880,19 +3872,19 @@ TEST(HRESULTAssertionTest, Streaming) {
 
 // Tests that the assertion macros behave like single statements.
 TEST(AssertionSyntaxTest, BasicAssertionsBehavesLikeSingleStatement) {
-  if (false)
+  if (AlwaysFalse())
     ASSERT_TRUE(false) << "This should never be executed; "
                           "It's a compilation test only.";
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_FALSE(false);
   else
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     ASSERT_LT(1, 3);
 
-  if (false)
+  if (AlwaysFalse())
     ;  // NOLINT
   else
     EXPECT_GT(3, 2) << "";
@@ -3914,26 +3906,26 @@ TEST(ExpectThrowTest, DoesNotGenerateUnreachableCodeWarning) {
 }
 
 TEST(AssertionSyntaxTest, ExceptionAssertionsBehavesLikeSingleStatement) {
-  if (false)
+  if (AlwaysFalse())
     EXPECT_THROW(ThrowNothing(), bool);
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_THROW(ThrowAnInteger(), int);
   else
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     EXPECT_NO_THROW(ThrowAnInteger());
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_NO_THROW(ThrowNothing());
   else
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     EXPECT_ANY_THROW(ThrowNothing());
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_ANY_THROW(ThrowAnInteger());
   else
     ;  // NOLINT
@@ -3941,23 +3933,23 @@ TEST(AssertionSyntaxTest, ExceptionAssertionsBehavesLikeSingleStatement) {
 #endif  // GTEST_HAS_EXCEPTIONS
 
 TEST(AssertionSyntaxTest, NoFatalFailureAssertionsBehavesLikeSingleStatement) {
-  if (false)
+  if (AlwaysFalse())
     EXPECT_NO_FATAL_FAILURE(FAIL()) << "This should never be executed. "
                                     << "It's a compilation test only.";
   else
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     ASSERT_NO_FATAL_FAILURE(FAIL()) << "";
   else
     ;  // NOLINT
 
-  if (true)
+  if (AlwaysTrue())
     EXPECT_NO_FATAL_FAILURE(SUCCEED());
   else
     ;  // NOLINT
 
-  if (false)
+  if (AlwaysFalse())
     ;  // NOLINT
   else
     ASSERT_NO_FATAL_FAILURE(SUCCEED());
@@ -6272,17 +6264,17 @@ class TestListener : public EmptyTestEventListener {
 };
 
 // Tests the constructor.
-TEST(EventListenersTest, ConstructionWorks) {
+TEST(TestEventListenersTest, ConstructionWorks) {
-  EventListeners listeners;
+  TestEventListeners listeners;
 
-  EXPECT_TRUE(EventListenersAccessor::GetRepeater(&listeners) != NULL);
+  EXPECT_TRUE(TestEventListenersAccessor::GetRepeater(&listeners) != NULL);
   EXPECT_TRUE(listeners.default_result_printer() == NULL);
   EXPECT_TRUE(listeners.default_xml_generator() == NULL);
 }
 
-// Tests that the EventListeners destructor deletes all the listeners it
+// Tests that the TestEventListeners destructor deletes all the listeners it
 // owns.
-TEST(EventListenersTest, DestructionWorks) {
+TEST(TestEventListenersTest, DestructionWorks) {
   bool default_result_printer_is_destroyed = false;
   bool default_xml_printer_is_destroyed = false;
   bool extra_listener_is_destroyed = false;
@@ -6294,11 +6286,11 @@ TEST(EventListenersTest, DestructionWorks) {
       NULL, &extra_listener_is_destroyed);
 
   {
-    EventListeners listeners;
+    TestEventListeners listeners;
-    EventListenersAccessor::SetDefaultResultPrinter(&listeners,
+    TestEventListenersAccessor::SetDefaultResultPrinter(&listeners,
-                                                    default_result_printer);
+                                                        default_result_printer);
-    EventListenersAccessor::SetDefaultXmlGenerator(&listeners,
+    TestEventListenersAccessor::SetDefaultXmlGenerator(&listeners,
-                                                   default_xml_printer);
+                                                       default_xml_printer);
     listeners.Append(extra_listener);
   }
   EXPECT_TRUE(default_result_printer_is_destroyed);
@@ -6306,16 +6298,16 @@ TEST(EventListenersTest, DestructionWorks) {
   EXPECT_TRUE(extra_listener_is_destroyed);
 }
 
-// Tests that a listener Append'ed to an EventListeners list starts
+// Tests that a listener Append'ed to a TestEventListeners list starts
 // receiving events.
-TEST(EventListenersTest, Append) {
+TEST(TestEventListenersTest, Append) {
   int on_start_counter = 0;
   bool is_destroyed = false;
   TestListener* listener = new TestListener(&on_start_counter, &is_destroyed);
   {
-    EventListeners listeners;
+    TestEventListeners listeners;
     listeners.Append(listener);
-    EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+    TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
         *UnitTest::GetInstance());
     EXPECT_EQ(1, on_start_counter);
   }
@@ -6364,12 +6356,12 @@ class SequenceTestingListener : public EmptyTestEventListener {
 
 TEST(EventListenerTest, AppendKeepsOrder) {
   Vector<String> vec;
-  EventListeners listeners;
+  TestEventListeners listeners;
   listeners.Append(new SequenceTestingListener(&vec, "1st"));
   listeners.Append(new SequenceTestingListener(&vec, "2nd"));
   listeners.Append(new SequenceTestingListener(&vec, "3rd"));
 
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
       *UnitTest::GetInstance());
   ASSERT_EQ(3, vec.size());
   EXPECT_STREQ("1st.OnTestProgramStart", vec.GetElement(0).c_str());
@@ -6377,7 +6369,7 @@ TEST(EventListenerTest, AppendKeepsOrder) {
   EXPECT_STREQ("3rd.OnTestProgramStart", vec.GetElement(2).c_str());
 
   vec.Clear();
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramEnd(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramEnd(
       *UnitTest::GetInstance());
   ASSERT_EQ(3, vec.size());
   EXPECT_STREQ("3rd.OnTestProgramEnd", vec.GetElement(0).c_str());
@@ -6385,7 +6377,7 @@ TEST(EventListenerTest, AppendKeepsOrder) {
   EXPECT_STREQ("1st.OnTestProgramEnd", vec.GetElement(2).c_str());
 
   vec.Clear();
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestIterationStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestIterationStart(
       *UnitTest::GetInstance(), 0);
   ASSERT_EQ(3, vec.size());
   EXPECT_STREQ("1st.OnTestIterationStart", vec.GetElement(0).c_str());
@@ -6393,7 +6385,7 @@ TEST(EventListenerTest, AppendKeepsOrder) {
   EXPECT_STREQ("3rd.OnTestIterationStart", vec.GetElement(2).c_str());
 
   vec.Clear();
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestIterationEnd(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestIterationEnd(
       *UnitTest::GetInstance(), 0);
   ASSERT_EQ(3, vec.size());
   EXPECT_STREQ("3rd.OnTestIterationEnd", vec.GetElement(0).c_str());
@@ -6401,9 +6393,9 @@ TEST(EventListenerTest, AppendKeepsOrder) {
   EXPECT_STREQ("1st.OnTestIterationEnd", vec.GetElement(2).c_str());
 }
 
-// Tests that a listener removed from an EventListeners list stops receiving
+// Tests that a listener removed from a TestEventListeners list stops receiving
 // events and is not deleted when the list is destroyed.
-TEST(EventListenersTest, Release) {
+TEST(TestEventListenersTest, Release) {
   int on_start_counter = 0;
   bool is_destroyed = false;
   // Although Append passes the ownership of this object to the list,
@@ -6411,10 +6403,10 @@ TEST(EventListenersTest, Release) {
   // test ends.
   TestListener* listener = new TestListener(&on_start_counter, &is_destroyed);
   {
-    EventListeners listeners;
+    TestEventListeners listeners;
     listeners.Append(listener);
     EXPECT_EQ(listener, listeners.Release(listener));
-    EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+    TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
         *UnitTest::GetInstance());
     EXPECT_TRUE(listeners.Release(listener) == NULL);
   }
@@ -6428,12 +6420,12 @@ TEST(EventListenerTest, SuppressEventForwarding) {
   int on_start_counter = 0;
   TestListener* listener = new TestListener(&on_start_counter, NULL);
 
-  EventListeners listeners;
+  TestEventListeners listeners;
   listeners.Append(listener);
-  ASSERT_TRUE(EventListenersAccessor::EventForwardingEnabled(listeners));
+  ASSERT_TRUE(TestEventListenersAccessor::EventForwardingEnabled(listeners));
-  EventListenersAccessor::SuppressEventForwarding(&listeners);
+  TestEventListenersAccessor::SuppressEventForwarding(&listeners);
-  ASSERT_FALSE(EventListenersAccessor::EventForwardingEnabled(listeners));
+  ASSERT_FALSE(TestEventListenersAccessor::EventForwardingEnabled(listeners));
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
       *UnitTest::GetInstance());
   EXPECT_EQ(0, on_start_counter);
 }
@@ -6442,7 +6434,7 @@ TEST(EventListenerTest, SuppressEventForwarding) {
 // death test subprocesses.
 TEST(EventListenerDeathTest, EventsNotForwardedInDeathTestSubprecesses) {
   EXPECT_DEATH_IF_SUPPORTED({
-      GTEST_CHECK_(EventListenersAccessor::EventForwardingEnabled(
+      GTEST_CHECK_(TestEventListenersAccessor::EventForwardingEnabled(
           *GetUnitTestImpl()->listeners())) << "expected failure";},
       "expected failure");
 }
@@ -6455,26 +6447,26 @@ TEST(EventListenerTest, default_result_printer) {
   bool is_destroyed = false;
   TestListener* listener = new TestListener(&on_start_counter, &is_destroyed);
 
-  EventListeners listeners;
+  TestEventListeners listeners;
-  EventListenersAccessor::SetDefaultResultPrinter(&listeners, listener);
+  TestEventListenersAccessor::SetDefaultResultPrinter(&listeners, listener);
 
   EXPECT_EQ(listener, listeners.default_result_printer());
 
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
       *UnitTest::GetInstance());
 
   EXPECT_EQ(1, on_start_counter);
 
   // Replacing default_result_printer with something else should remove it
   // from the list and destroy it.
-  EventListenersAccessor::SetDefaultResultPrinter(&listeners, NULL);
+  TestEventListenersAccessor::SetDefaultResultPrinter(&listeners, NULL);
 
   EXPECT_TRUE(listeners.default_result_printer() == NULL);
   EXPECT_TRUE(is_destroyed);
 
   // After broadcasting an event the counter is still the same, indicating
   // the listener is not in the list anymore.
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
       *UnitTest::GetInstance());
   EXPECT_EQ(1, on_start_counter);
 }
@@ -6489,15 +6481,15 @@ TEST(EventListenerTest, RemovingDefaultResultPrinterWorks) {
   // test ends.
   TestListener* listener = new TestListener(&on_start_counter, &is_destroyed);
   {
-    EventListeners listeners;
+    TestEventListeners listeners;
-    EventListenersAccessor::SetDefaultResultPrinter(&listeners, listener);
+    TestEventListenersAccessor::SetDefaultResultPrinter(&listeners, listener);
 
     EXPECT_EQ(listener, listeners.Release(listener));
     EXPECT_TRUE(listeners.default_result_printer() == NULL);
     EXPECT_FALSE(is_destroyed);
 
     // Broadcasting events now should not affect default_result_printer.
-    EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+    TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
         *UnitTest::GetInstance());
     EXPECT_EQ(0, on_start_counter);
   }
@@ -6514,26 +6506,26 @@ TEST(EventListenerTest, default_xml_generator) {
   bool is_destroyed = false;
   TestListener* listener = new TestListener(&on_start_counter, &is_destroyed);
 
-  EventListeners listeners;
+  TestEventListeners listeners;
-  EventListenersAccessor::SetDefaultXmlGenerator(&listeners, listener);
+  TestEventListenersAccessor::SetDefaultXmlGenerator(&listeners, listener);
 
   EXPECT_EQ(listener, listeners.default_xml_generator());
 
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
       *UnitTest::GetInstance());
 
   EXPECT_EQ(1, on_start_counter);
 
   // Replacing default_xml_generator with something else should remove it
   // from the list and destroy it.
-  EventListenersAccessor::SetDefaultXmlGenerator(&listeners, NULL);
+  TestEventListenersAccessor::SetDefaultXmlGenerator(&listeners, NULL);
 
   EXPECT_TRUE(listeners.default_xml_generator() == NULL);
   EXPECT_TRUE(is_destroyed);
 
   // After broadcasting an event the counter is still the same, indicating
   // the listener is not in the list anymore.
-  EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+  TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
       *UnitTest::GetInstance());
   EXPECT_EQ(1, on_start_counter);
 }
@@ -6548,15 +6540,15 @@ TEST(EventListenerTest, RemovingDefaultXmlGeneratorWorks) {
   // test ends.
   TestListener* listener = new TestListener(&on_start_counter, &is_destroyed);
   {
-    EventListeners listeners;
+    TestEventListeners listeners;
-    EventListenersAccessor::SetDefaultXmlGenerator(&listeners, listener);
+    TestEventListenersAccessor::SetDefaultXmlGenerator(&listeners, listener);
 
     EXPECT_EQ(listener, listeners.Release(listener));
     EXPECT_TRUE(listeners.default_xml_generator() == NULL);
     EXPECT_FALSE(is_destroyed);
 
     // Broadcasting events now should not affect default_xml_generator.
-    EventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
+    TestEventListenersAccessor::GetRepeater(&listeners)->OnTestProgramStart(
         *UnitTest::GetInstance());
     EXPECT_EQ(0, on_start_counter);
   }

test/gtest_xml_output_unittest.py

@@ -54,7 +54,7 @@ else:
   STACK_TRACE_TEMPLATE = ""
 
 EXPECTED_NON_EMPTY_XML = """<?xml version="1.0" encoding="UTF-8"?>
-<testsuites tests="13" failures="2" disabled="2" errors="0" time="*" name="AllTests">
+<testsuites tests="15" failures="4" disabled="2" errors="0" time="*" name="AllTests">
   <testsuite name="SuccessfulTest" tests="1" failures="0" disabled="0" errors="0" time="*">
     <testcase name="Succeeds" status="run" time="*" classname="SuccessfulTest"/>
   </testsuite>
@@ -77,6 +77,20 @@ Expected: 2%(stack)s]]></failure>
     </testcase>
     <testcase name="DISABLED_test" status="notrun" time="*" classname="MixedResultTest"/>
   </testsuite>
+  <testsuite name="XmlQuotingTest" tests="1" failures="1" disabled="0" errors="0" time="*">
+    <testcase name="OutputsCData" status="run" time="*" classname="XmlQuotingTest">
+      <failure message="Failed&#x0A;XML output: &lt;?xml encoding=&quot;utf-8&quot;&gt;&lt;top&gt;&lt;![CDATA[cdata text]]&gt;&lt;/top&gt;" type=""><![CDATA[gtest_xml_output_unittest_.cc:*
+Failed
+XML output: <?xml encoding="utf-8"><top><![CDATA[cdata text]]>]]&gt;<![CDATA[</top>%(stack)s]]></failure>
+    </testcase>
+  </testsuite>
+  <testsuite name="InvalidCharactersTest" tests="1" failures="1" disabled="0" errors="0" time="*">
+    <testcase name="InvalidCharactersInMessage" status="run" time="*" classname="InvalidCharactersTest">
+      <failure message="Failed&#x0A;Invalid characters in brackets []" type=""><![CDATA[gtest_xml_output_unittest_.cc:*
+Failed
+Invalid characters in brackets []%(stack)s]]></failure>
+    </testcase>
+  </testsuite>
   <testsuite name="DisabledTest" tests="1" failures="0" disabled="1" errors="0" time="*">
     <testcase name="DISABLED_test_not_run" status="notrun" time="*" classname="DisabledTest"/>
   </testsuite>

test/gtest_xml_output_unittest_.cc

@@ -40,8 +40,8 @@
 
 #include <gtest/gtest.h>
 
-using ::testing::EventListeners;
 using ::testing::InitGoogleTest;
+using ::testing::TestEventListeners;
 using ::testing::UnitTest;
 
 class SuccessfulTest : public testing::Test {
@@ -80,6 +80,17 @@ TEST(MixedResultTest, DISABLED_test) {
   FAIL() << "Unexpected failure: Disabled test should not be run";
 }
 
+TEST(XmlQuotingTest, OutputsCData) {
+  FAIL() << "XML output: "
+            "<?xml encoding=\"utf-8\"><top><![CDATA[cdata text]]></top>";
+}
+
+// Helps to test that invalid characters produced by test code do not make
+// it into the XML file.
+TEST(InvalidCharactersTest, InvalidCharactersInMessage) {
+  FAIL() << "Invalid characters in brackets [\x1\x2]";
+}
+
 class PropertyRecordingTest : public testing::Test {
 };
 
@@ -127,7 +138,7 @@ int main(int argc, char** argv) {
   InitGoogleTest(&argc, argv);
 
   if (argc > 1 && strcmp(argv[1], "--shut_down_xml") == 0) {
-    EventListeners& listeners = UnitTest::GetInstance()->listeners();
+    TestEventListeners& listeners = UnitTest::GetInstance()->listeners();
     delete listeners.Release(listeners.default_xml_generator());
   }
   return RUN_ALL_TESTS();

test/gtest_xml_test_utils.py

@@ -77,19 +77,29 @@ class GTestXMLTestCase(gtest_test_utils.TestCase):
 
     expected_attributes = expected_node.attributes
     actual_attributes   = actual_node  .attributes
-    self.assertEquals(expected_attributes.length, actual_attributes.length)
+    self.assertEquals(
+        expected_attributes.length, actual_attributes.length,
+        "attribute numbers differ in element " + actual_node.tagName)
     for i in range(expected_attributes.length):
       expected_attr = expected_attributes.item(i)
       actual_attr   = actual_attributes.get(expected_attr.name)
-      self.assert_(actual_attr is not None)
+      self.assert_(
-      self.assertEquals(expected_attr.value, actual_attr.value)
+          actual_attr is not None,
+          "expected attribute %s not found in element %s" %
+          (expected_attr.name, actual_node.tagName))
+      self.assertEquals(expected_attr.value, actual_attr.value,
+                        " values of attribute %s in element %s differ" %
+                        (expected_attr.name, actual_node.tagName))
 
     expected_children = self._GetChildren(expected_node)
     actual_children = self._GetChildren(actual_node)
-    self.assertEquals(len(expected_children), len(actual_children))
+    self.assertEquals(
+        len(expected_children), len(actual_children),
+        "number of child elements differ in element " + actual_node.tagName)
     for child_id, child in expected_children.iteritems():
       self.assert_(child_id in actual_children,
-                   '<%s> is not in <%s>' % (child_id, actual_children))
+                   '<%s> is not in <%s> (in element %s)' %
+                   (child_id, actual_children, actual_node.tagName))
       self.AssertEquivalentNodes(child, actual_children[child_id])
 
   identifying_attribute = {
@@ -103,14 +113,13 @@ class GTestXMLTestCase(gtest_test_utils.TestCase):
     """
     Fetches all of the child nodes of element, a DOM Element object.
     Returns them as the values of a dictionary keyed by the IDs of the
-    children.  For <testsuites>, <testsuite> and <testcase> elements,
+    children.  For <testsuites>, <testsuite> and <testcase> elements, the ID
-    the ID is the value of their "name" attribute; for <failure>
+    is the value of their "name" attribute; for <failure> elements, it is
-    elements, it is the value of the "message" attribute; for CDATA
+    the value of the "message" attribute; CDATA sections and non-whitespace
-    section node, it is "detail".  An exception is raised if any
+    text nodes are concatenated into a single CDATA section with ID
-    element other than the above four is encountered, if two child
+    "detail".  An exception is raised if any element other than the above
-    elements with the same identifying attributes are encountered, or
+    four is encountered, if two child elements with the same identifying
-    if any other type of node is encountered, other than Text nodes
+    attributes are encountered, or if any other type of node is encountered.
-    containing only whitespace.
     """
 
     children = {}
@@ -121,11 +130,14 @@ class GTestXMLTestCase(gtest_test_utils.TestCase):
         childID = child.getAttribute(self.identifying_attribute[child.tagName])
         self.assert_(childID not in children)
         children[childID] = child
-      elif child.nodeType == Node.TEXT_NODE:
+      elif child.nodeType in [Node.TEXT_NODE, Node.CDATA_SECTION_NODE]:
-        self.assert_(child.nodeValue.isspace())
+        if "detail" not in children:
-      elif child.nodeType == Node.CDATA_SECTION_NODE:
+          if (child.nodeType == Node.CDATA_SECTION_NODE or
-        self.assert_("detail" not in children)
+              not child.nodeValue.isspace()):
-        children["detail"] = child
+            children["detail"] = child.ownerDocument.createCDATASection(
+                child.nodeValue)
+        else:
+          children["detail"].nodeValue += child.nodeValue
       else:
         self.fail("Encountered unexpected node type %d" % child.nodeType)
     return children