Adds type_param and value_param as <testcase> attributes to the XML

report; also removes the comment() and test_case_comment() fields of
TestInfo.  Proposed and initally implemented by Joey Oravec.
Re-implemented by Vlad Losev.

5 files changed, 100 insertions, 51 deletions

diff --git a/test/gtest-param-test_test.cc b/test/gtest-param-test_test.cc
index acd269bc..5a681d83 100644
--- a/test/gtest-param-test_test.cc
+++ b/test/gtest-param-test_test.cc
@@ -792,19 +792,17 @@ INSTANTIATE_TEST_CASE_P(FourElemSequence, SeparateInstanceTest, Range(1, 4));
 // sequence element used to instantiate the test.
 class NamingTest : public TestWithParam<int> {};
 
-TEST_P(NamingTest, TestsAreNamedAndCommentedCorrectly) {
+TEST_P(NamingTest, TestsReportCorrectNamesAndParameters) {
   const ::testing::TestInfo* const test_info =
      ::testing::UnitTest::GetInstance()->current_test_info();
 
   EXPECT_STREQ("ZeroToFiveSequence/NamingTest", test_info->test_case_name());
 
   Message index_stream;
-  index_stream << "TestsAreNamedAndCommentedCorrectly/" << GetParam();
+  index_stream << "TestsReportCorrectNamesAndParameters/" << GetParam();
   EXPECT_STREQ(index_stream.GetString().c_str(), test_info->name());
 
-  const ::std::string comment =
-      "GetParam() = " + ::testing::PrintToString(GetParam());
-  EXPECT_EQ(comment, test_info->comment());
+  EXPECT_EQ(::testing::PrintToString(GetParam()), test_info->value_param());
 }
 
 INSTANTIATE_TEST_CASE_P(ZeroToFiveSequence, NamingTest, Range(0, 5));
@@ -823,13 +821,11 @@ class Unstreamable {
 
 class CommentTest : public TestWithParam<Unstreamable> {};
 
-TEST_P(CommentTest, TestsWithUnstreamableParamsCommentedCorrectly) {
+TEST_P(CommentTest, TestsCorrectlyReportUnstreamableParams) {
   const ::testing::TestInfo* const test_info =
      ::testing::UnitTest::GetInstance()->current_test_info();
 
-  const ::std::string comment =
-      "GetParam() = " + ::testing::PrintToString(GetParam());
-  EXPECT_EQ(comment, test_info->comment());
+  EXPECT_EQ(::testing::PrintToString(GetParam()), test_info->value_param());
 }
 
 INSTANTIATE_TEST_CASE_P(InstantiationWithComments,
diff --git a/test/gtest-unittest-api_test.cc b/test/gtest-unittest-api_test.cc
index ed5dea83..f53e2744 100644
--- a/test/gtest-unittest-api_test.cc
+++ b/test/gtest-unittest-api_test.cc
@@ -37,6 +37,7 @@
 
 #include <string.h>  // For strcmp.
 #include <algorithm>
+#include <sstream>
 
 using ::testing::InitGoogleTest;
 
@@ -103,12 +104,6 @@ TYPED_TEST(TestCaseWithCommentTest, Dummy) {}
 
 const int kTypedTestCases = 1;
 const int kTypedTests = 1;
-
-String GetExpectedTestCaseComment() {
-  Message comment;
-  comment << "TypeParam = " << GetTypeName<int>().c_str();
-  return comment.GetString();
-}
 #else
 const int kTypedTestCases = 0;
 const int kTypedTests = 0;
@@ -143,12 +138,19 @@ TEST(ApiTest, UnitTestImmutableAccessorsWork) {
   RecordProperty("key", "value");
 }
 
+AssertionResult IsNull(const char* str) {
+  if (str != NULL) {
+    return testing::AssertionFailure() << "argument is " << str;
+  }
+  return AssertionSuccess();
+}
+
 TEST(ApiTest, TestCaseImmutableAccessorsWork) {
   const TestCase* test_case = UnitTestHelper::FindTestCase("ApiTest");
   ASSERT_TRUE(test_case != NULL);
 
   EXPECT_STREQ("ApiTest", test_case->name());
-  EXPECT_STREQ("", test_case->comment());
+  EXPECT_TRUE(IsNull(test_case->type_param()));
   EXPECT_TRUE(test_case->should_run());
   EXPECT_EQ(1, test_case->disabled_test_count());
   EXPECT_EQ(3, test_case->test_to_run_count());
@@ -158,26 +160,26 @@ TEST(ApiTest, TestCaseImmutableAccessorsWork) {
 
   EXPECT_STREQ("DISABLED_Dummy1", tests[0]->name());
   EXPECT_STREQ("ApiTest", tests[0]->test_case_name());
-  EXPECT_STREQ("", tests[0]->comment());
-  EXPECT_STREQ("", tests[0]->test_case_comment());
+  EXPECT_TRUE(IsNull(tests[0]->value_param()));
+  EXPECT_TRUE(IsNull(tests[0]->type_param()));
   EXPECT_FALSE(tests[0]->should_run());
 
   EXPECT_STREQ("TestCaseDisabledAccessorsWork", tests[1]->name());
   EXPECT_STREQ("ApiTest", tests[1]->test_case_name());
-  EXPECT_STREQ("", tests[1]->comment());
-  EXPECT_STREQ("", tests[1]->test_case_comment());
+  EXPECT_TRUE(IsNull(tests[1]->value_param()));
+  EXPECT_TRUE(IsNull(tests[1]->type_param()));
   EXPECT_TRUE(tests[1]->should_run());
 
   EXPECT_STREQ("TestCaseImmutableAccessorsWork", tests[2]->name());
   EXPECT_STREQ("ApiTest", tests[2]->test_case_name());
-  EXPECT_STREQ("", tests[2]->comment());
-  EXPECT_STREQ("", tests[2]->test_case_comment());
+  EXPECT_TRUE(IsNull(tests[2]->value_param()));
+  EXPECT_TRUE(IsNull(tests[2]->type_param()));
   EXPECT_TRUE(tests[2]->should_run());
 
   EXPECT_STREQ("UnitTestImmutableAccessorsWork", tests[3]->name());
   EXPECT_STREQ("ApiTest", tests[3]->test_case_name());
-  EXPECT_STREQ("", tests[3]->comment());
-  EXPECT_STREQ("", tests[3]->test_case_comment());
+  EXPECT_TRUE(IsNull(tests[3]->value_param()));
+  EXPECT_TRUE(IsNull(tests[3]->type_param()));
   EXPECT_TRUE(tests[3]->should_run());
 
   delete[] tests;
@@ -188,7 +190,7 @@ TEST(ApiTest, TestCaseImmutableAccessorsWork) {
   ASSERT_TRUE(test_case != NULL);
 
   EXPECT_STREQ("TestCaseWithCommentTest/0", test_case->name());
-  EXPECT_STREQ(GetExpectedTestCaseComment().c_str(), test_case->comment());
+  EXPECT_STREQ(GetTypeName<int>().c_str(), test_case->type_param());
   EXPECT_TRUE(test_case->should_run());
   EXPECT_EQ(0, test_case->disabled_test_count());
   EXPECT_EQ(1, test_case->test_to_run_count());
@@ -198,9 +200,8 @@ TEST(ApiTest, TestCaseImmutableAccessorsWork) {
 
   EXPECT_STREQ("Dummy", tests[0]->name());
   EXPECT_STREQ("TestCaseWithCommentTest/0", tests[0]->test_case_name());
-  EXPECT_STREQ("", tests[0]->comment());
-  EXPECT_STREQ(GetExpectedTestCaseComment().c_str(),
-               tests[0]->test_case_comment());
+  EXPECT_TRUE(IsNull(tests[0]->value_param()));
+  EXPECT_STREQ(GetTypeName<int>().c_str(), tests[0]->type_param());
   EXPECT_TRUE(tests[0]->should_run());
 
   delete[] tests;
@@ -212,7 +213,7 @@ TEST(ApiTest, TestCaseDisabledAccessorsWork) {
   ASSERT_TRUE(test_case != NULL);
 
   EXPECT_STREQ("DISABLED_Test", test_case->name());
-  EXPECT_STREQ("", test_case->comment());
+  EXPECT_TRUE(IsNull(test_case->type_param()));
   EXPECT_FALSE(test_case->should_run());
   EXPECT_EQ(1, test_case->disabled_test_count());
   EXPECT_EQ(0, test_case->test_to_run_count());
@@ -221,8 +222,8 @@ TEST(ApiTest, TestCaseDisabledAccessorsWork) {
   const TestInfo* const test_info = test_case->GetTestInfo(0);
   EXPECT_STREQ("Dummy2", test_info->name());
   EXPECT_STREQ("DISABLED_Test", test_info->test_case_name());
-  EXPECT_STREQ("", test_info->comment());
-  EXPECT_STREQ("", test_info->test_case_comment());
+  EXPECT_TRUE(IsNull(test_info->value_param()));
+  EXPECT_TRUE(IsNull(test_info->type_param()));
   EXPECT_FALSE(test_info->should_run());
 }
 
@@ -247,7 +248,7 @@ class FinalSuccessChecker : public Environment {
     const TestCase** const test_cases = UnitTestHelper::GetSortedTestCases();
 
     EXPECT_STREQ("ApiTest", test_cases[0]->name());
-    EXPECT_STREQ("", test_cases[0]->comment());
+    EXPECT_TRUE(IsNull(test_cases[0]->type_param()));
     EXPECT_TRUE(test_cases[0]->should_run());
     EXPECT_EQ(1, test_cases[0]->disabled_test_count());
     ASSERT_EQ(4, test_cases[0]->total_test_count());
@@ -257,7 +258,7 @@ class FinalSuccessChecker : public Environment {
     EXPECT_FALSE(test_cases[0]->Failed());
 
     EXPECT_STREQ("DISABLED_Test", test_cases[1]->name());
-    EXPECT_STREQ("", test_cases[1]->comment());
+    EXPECT_TRUE(IsNull(test_cases[1]->type_param()));
     EXPECT_FALSE(test_cases[1]->should_run());
     EXPECT_EQ(1, test_cases[1]->disabled_test_count());
     ASSERT_EQ(1, test_cases[1]->total_test_count());
@@ -266,8 +267,7 @@ class FinalSuccessChecker : public Environment {
 
 #if GTEST_HAS_TYPED_TEST
     EXPECT_STREQ("TestCaseWithCommentTest/0", test_cases[2]->name());
-    EXPECT_STREQ(GetExpectedTestCaseComment().c_str(),
-                 test_cases[2]->comment());
+    EXPECT_STREQ(GetTypeName<int>().c_str(), test_cases[2]->type_param());
     EXPECT_TRUE(test_cases[2]->should_run());
     EXPECT_EQ(0, test_cases[2]->disabled_test_count());
     ASSERT_EQ(1, test_cases[2]->total_test_count());
@@ -285,24 +285,24 @@ class FinalSuccessChecker : public Environment {
 
     EXPECT_STREQ("TestCaseDisabledAccessorsWork", tests[1]->name());
     EXPECT_STREQ("ApiTest", tests[1]->test_case_name());
-    EXPECT_STREQ("", tests[1]->comment());
-    EXPECT_STREQ("", tests[1]->test_case_comment());
+    EXPECT_TRUE(IsNull(tests[1]->value_param()));
+    EXPECT_TRUE(IsNull(tests[1]->type_param()));
     EXPECT_TRUE(tests[1]->should_run());
     EXPECT_TRUE(tests[1]->result()->Passed());
     EXPECT_EQ(0, tests[1]->result()->test_property_count());
 
     EXPECT_STREQ("TestCaseImmutableAccessorsWork", tests[2]->name());
     EXPECT_STREQ("ApiTest", tests[2]->test_case_name());
-    EXPECT_STREQ("", tests[2]->comment());
-    EXPECT_STREQ("", tests[2]->test_case_comment());
+    EXPECT_TRUE(IsNull(tests[2]->value_param()));
+    EXPECT_TRUE(IsNull(tests[2]->type_param()));
     EXPECT_TRUE(tests[2]->should_run());
     EXPECT_TRUE(tests[2]->result()->Passed());
     EXPECT_EQ(0, tests[2]->result()->test_property_count());
 
     EXPECT_STREQ("UnitTestImmutableAccessorsWork", tests[3]->name());
     EXPECT_STREQ("ApiTest", tests[3]->test_case_name());
-    EXPECT_STREQ("", tests[3]->comment());
-    EXPECT_STREQ("", tests[3]->test_case_comment());
+    EXPECT_TRUE(IsNull(tests[3]->value_param()));
+    EXPECT_TRUE(IsNull(tests[3]->type_param()));
     EXPECT_TRUE(tests[3]->should_run());
     EXPECT_TRUE(tests[3]->result()->Passed());
     EXPECT_EQ(1, tests[3]->result()->test_property_count());
@@ -318,9 +318,8 @@ class FinalSuccessChecker : public Environment {
 
     EXPECT_STREQ("Dummy", tests[0]->name());
     EXPECT_STREQ("TestCaseWithCommentTest/0", tests[0]->test_case_name());
-    EXPECT_STREQ("", tests[0]->comment());
-    EXPECT_STREQ(GetExpectedTestCaseComment().c_str(),
-                 tests[0]->test_case_comment());
+    EXPECT_TRUE(IsNull(tests[0]->value_param()));
+    EXPECT_STREQ(GetTypeName<int>().c_str(), tests[0]->type_param());
     EXPECT_TRUE(tests[0]->should_run());
     EXPECT_TRUE(tests[0]->result()->Passed());
     EXPECT_EQ(0, tests[0]->result()->test_property_count());
diff --git a/test/gtest_xml_output_unittest.py b/test/gtest_xml_output_unittest.py
index 6d44929c..bdd50353 100755
--- a/test/gtest_xml_output_unittest.py
+++ b/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="15" failures="4" disabled="2" errors="0" time="*" name="AllTests">
+<testsuites tests="23" 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>
@@ -105,6 +105,24 @@ Invalid characters in brackets []%(stack)s]]></failure>
      <testcase name="ExternalUtilityThatCallsRecordIntValuedProperty" status="run" time="*" classname="NoFixtureTest" key_for_utility_int="1"/>
      <testcase name="ExternalUtilityThatCallsRecordStringValuedProperty" status="run" time="*" classname="NoFixtureTest" key_for_utility_string="1"/>
   </testsuite>
+  <testsuite name="Single/ValueParamTest" tests="4" failures="0" disabled="0" errors="0" time="*">
+    <testcase name="HasValueParamAttribute/0" value_param="33" status="run" time="*" classname="Single/ValueParamTest" />
+    <testcase name="HasValueParamAttribute/1" value_param="42" status="run" time="*" classname="Single/ValueParamTest" />
+    <testcase name="AnotherTestThatHasValueParamAttribute/0" value_param="33" status="run" time="*" classname="Single/ValueParamTest" />
+    <testcase name="AnotherTestThatHasValueParamAttribute/1" value_param="42" status="run" time="*" classname="Single/ValueParamTest" />
+  </testsuite>
+  <testsuite name="TypedTest/0" tests="1" failures="0" disabled="0" errors="0" time="*">
+    <testcase name="HasTypeParamAttribute" type_param="*" status="run" time="*" classname="TypedTest/0" />
+  </testsuite>
+  <testsuite name="TypedTest/1" tests="1" failures="0" disabled="0" errors="0" time="*">
+    <testcase name="HasTypeParamAttribute" type_param="*" status="run" time="*" classname="TypedTest/1" />
+  </testsuite>
+  <testsuite name="Single/TypeParameterizedTestCase/0" tests="1" failures="0" disabled="0" errors="0" time="*">
+    <testcase name="HasTypeParamAttribute" type_param="*" status="run" time="*" classname="Single/TypeParameterizedTestCase/0" />
+  </testsuite>
+  <testsuite name="Single/TypeParameterizedTestCase/1" tests="1" failures="0" disabled="0" errors="0" time="*">
+    <testcase name="HasTypeParamAttribute" type_param="*" status="run" time="*" classname="Single/TypeParameterizedTestCase/1" />
+  </testsuite>
 </testsuites>""" % {'stack': STACK_TRACE_TEMPLATE}
 
 
diff --git a/test/gtest_xml_output_unittest_.cc b/test/gtest_xml_output_unittest_.cc
index 693ffb99..741a8874 100644
--- a/test/gtest_xml_output_unittest_.cc
+++ b/test/gtest_xml_output_unittest_.cc
@@ -42,9 +42,13 @@
 
 using ::testing::InitGoogleTest;
 using ::testing::TestEventListeners;
+using ::testing::TestWithParam;
 using ::testing::UnitTest;
+using ::testing::Test;
+using ::testing::Types;
+using ::testing::Values;
 
-class SuccessfulTest : public testing::Test {
+class SuccessfulTest : public Test {
 };
 
 TEST_F(SuccessfulTest, Succeeds) {
@@ -52,14 +56,14 @@ TEST_F(SuccessfulTest, Succeeds) {
   ASSERT_EQ(1, 1);
 }
 
-class FailedTest : public testing::Test {
+class FailedTest : public Test {
 };
 
 TEST_F(FailedTest, Fails) {
   ASSERT_EQ(1, 2);
 }
 
-class DisabledTest : public testing::Test {
+class DisabledTest : public Test {
 };
 
 TEST_F(DisabledTest, DISABLED_test_not_run) {
@@ -91,7 +95,7 @@ TEST(InvalidCharactersTest, InvalidCharactersInMessage) {
   FAIL() << "Invalid characters in brackets [\x1\x2]";
 }
 
-class PropertyRecordingTest : public testing::Test {
+class PropertyRecordingTest : public Test {
 };
 
 TEST_F(PropertyRecordingTest, OneProperty) {
@@ -134,6 +138,31 @@ TEST(NoFixtureTest, ExternalUtilityThatCallsRecordStringValuedProperty) {
   ExternalUtilityThatCallsRecordProperty("key_for_utility_string", "1");
 }
 
+// Verifies that the test parameter value is output in the 'value_param'
+// XML attribute for value-parameterized tests.
+class ValueParamTest : public TestWithParam<int> {};
+TEST_P(ValueParamTest, HasValueParamAttribute) {}
+TEST_P(ValueParamTest, AnotherTestThatHasValueParamAttribute) {}
+INSTANTIATE_TEST_CASE_P(Single, ValueParamTest, Values(33, 42));
+
+// Verifies that the type parameter name is output in the 'type_param'
+// XML attribute for typed tests.
+template <typename T> class TypedTest : public Test {};
+typedef Types<int, long> TypedTestTypes;
+TYPED_TEST_CASE(TypedTest, TypedTestTypes);
+TYPED_TEST(TypedTest, HasTypeParamAttribute) {}
+
+// Verifies that the type parameter name is output in the 'type_param'
+// XML attribute for type-parameterized tests.
+template <typename T> class TypeParameterizedTestCase : public Test {};
+TYPED_TEST_CASE_P(TypeParameterizedTestCase);
+TYPED_TEST_P(TypeParameterizedTestCase, HasTypeParamAttribute) {}
+REGISTER_TYPED_TEST_CASE_P(TypeParameterizedTestCase, HasTypeParamAttribute);
+typedef Types<int, long> TypeParameterizedTestCaseTypes;
+INSTANTIATE_TYPED_TEST_CASE_P(Single,
+                              TypeParameterizedTestCase,
+                              TypeParameterizedTestCaseTypes);
+
 int main(int argc, char** argv) {
   InitGoogleTest(&argc, argv);
 
diff --git a/test/gtest_xml_test_utils.py b/test/gtest_xml_test_utils.py
index c83c3b7e..0f55c164 100755
--- a/test/gtest_xml_test_utils.py
+++ b/test/gtest_xml_test_utils.py
@@ -58,8 +58,9 @@ class GTestXMLTestCase(gtest_test_utils.TestCase):
     *  It has the same tag name as expected_node.
     *  It has the same set of attributes as expected_node, each with
        the same value as the corresponding attribute of expected_node.
-       An exception is any attribute named "time", which needs only be
-       convertible to a floating-point number.
+       Exceptions are any attribute named "time", which needs only be
+       convertible to a floating-point number and any attribute named
+       "type_param" which only has to be non-empty.
     *  It has an equivalent set of child nodes (including elements and
        CDATA sections) as expected_node.  Note that we ignore the
        order of the children as they are not guaranteed to be in any
@@ -150,6 +151,9 @@ class GTestXMLTestCase(gtest_test_utils.TestCase):
     *  The "time" attribute of <testsuites>, <testsuite> and <testcase>
        elements is replaced with a single asterisk, if it contains
        only digit characters.
+    *  The "type_param" attribute of <testcase> elements is replaced with a
+       single asterisk (if it sn non-empty) as it is the type name returned
+       by the compiler and is platform dependent.
     *  The line number reported in the first line of the "message"
        attribute of <failure> elements is replaced with a single asterisk.
     *  The directory names in file paths are removed.
@@ -159,6 +163,9 @@ class GTestXMLTestCase(gtest_test_utils.TestCase):
     if element.tagName in ("testsuites", "testsuite", "testcase"):
       time = element.getAttributeNode("time")
       time.value = re.sub(r"^\d+(\.\d+)?$", "*", time.value)
+      type_param = element.getAttributeNode("type_param")
+      if type_param and type_param.value:
+        type_param.value = "*"
     elif element.tagName == "failure":
       for child in element.childNodes:
         if child.nodeType == Node.CDATA_SECTION_NODE: