Make merging of shard test results handle test suites that are split across shards better.

BUG=136412
R=nsylvain@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10749018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@145952 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 129 insertions, 10 deletions

diff --git a/tools/sharding_supervisor/data/gtest_results.xml0 b/tools/sharding_supervisor/data/gtest_results.xml0
new file mode 100644
index 0000000..f6bf83a
--- /dev/null
+++ b/tools/sharding_supervisor/data/gtest_results.xml0
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuites name="AllTests" tests="" failures="" disabled="" errors="" time="">
+  <!-- Suite that is run entirely on shard 0 -->
+  <testsuite name="Suite0" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite0" />
+  </testsuite>
+
+  <!-- Suite that is run entirely on shard 1 -->
+  <testsuite name="Suite1" tests="1" failures="" disabled="" errors="" time="">
+  </testsuite>
+
+  <!-- Suite that has tests run on both shard 0 and shard 1 -->
+  <testsuite name="Suite2" tests="2" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite2" />
+  </testsuite>
+
+  <!-- Suite that has a test run on both shard 0 and shard 1 -->
+  <testsuite name="Suite3" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite3">
+      <failure message="" type="" ignored="true"></failure>
+    </testcase>
+  </testsuite>
+</testsuites>
diff --git a/tools/sharding_supervisor/data/gtest_results.xml1 b/tools/sharding_supervisor/data/gtest_results.xml1
new file mode 100644
index 0000000..6d8bef8
--- /dev/null
+++ b/tools/sharding_supervisor/data/gtest_results.xml1
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuites name="AllTests" tests="" failures="" disabled="" errors="" time="">
+  <!-- See comments in gtest_results.xml0 for what the different suites represent -->
+  <testsuite name="Suite0" tests="1" failures="" disabled="" errors="" time="">
+  </testsuite>
+  <testsuite name="Suite1" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="1" classname="Suite1">
+      <failure message="" type="" ignored="true"></failure>
+    </testcase>
+  </testsuite>
+  <testsuite name="Suite2" tests="2" failures="" disabled="" errors="" time="">
+    <testcase name="Test1" status="run" time="0" classname="Suite2" />
+  </testsuite>
+  <testsuite name="Suite3" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite3">
+      <failure message="" type="" ignored="true"></failure>
+    </testcase>
+  </testsuite>
+</testsuites>
diff --git a/tools/sharding_supervisor/data/gtest_results_expected.xml b/tools/sharding_supervisor/data/gtest_results_expected.xml
new file mode 100644
index 0000000..7af04d4
--- /dev/null
+++ b/tools/sharding_supervisor/data/gtest_results_expected.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuites name="AllTests" tests="" failures="" disabled="" errors="" time="">
+  <!-- See comments in gtest_results.xml0 for what the different suites represent -->
+  <testsuite name="Suite0" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite0" />
+  </testsuite>
+  <testsuite name="Suite1" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="1" classname="Suite1">
+      <failure message="" type="" ignored="true"></failure>
+    </testcase>
+  </testsuite>
+  <testsuite name="Suite2" tests="2" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite2" />
+    <testcase name="Test1" status="run" time="0" classname="Suite2" />
+  </testsuite>
+  <testsuite name="Suite3" tests="1" failures="" disabled="" errors="" time="">
+    <testcase name="Test0" status="run" time="0" classname="Suite3">
+      <failure message="" type="" ignored="true"></failure>
+      <failure message="" type="" ignored="true"></failure>
+    </testcase>
+  </testsuite>
+</testsuites>
diff --git a/tools/sharding_supervisor/sharding_supervisor.py b/tools/sharding_supervisor/sharding_supervisor.py
index 019ebf8..83f860c 100755
--- a/tools/sharding_supervisor/sharding_supervisor.py
+++ b/tools/sharding_supervisor/sharding_supervisor.py
@@ -123,6 +123,11 @@ def AppendToXML(final_xml, generic_path, shard):
 
   testcases = shard_node.getElementsByTagName('testcase')
   final_testcases = final_node.getElementsByTagName('testcase')
+
+  final_testsuites = final_node.getElementsByTagName('testsuite')
+  final_testsuites_by_name = dict(
+      (suite.getAttribute('name'), suite) for suite in final_testsuites)
+
   for testcase in testcases:
     name = testcase.getAttribute('name')
     classname = testcase.getAttribute('classname')
@@ -136,6 +141,7 @@ def AppendToXML(final_xml, generic_path, shard):
 
     # Look in our final xml to see if it's there.
     # There has to be a better way...
+    merged_into_final_testcase = False
     for final_testcase in final_testcases:
       final_name = final_testcase.getAttribute('name')
       final_classname = final_testcase.getAttribute('classname')
@@ -145,6 +151,14 @@ def AppendToXML(final_xml, generic_path, shard):
         final_testcase.setAttribute('time', elapsed)
         for failure in failures:
           final_testcase.appendChild(failure)
+        merged_into_final_testcase = True
+
+    # We couldn't find an existing testcase to merge the results into, so we
+    # copy the node into the existing test suite.
+    if not merged_into_final_testcase:
+      testsuite = testcase.parentNode
+      final_testsuite = final_testsuites_by_name[testsuite.getAttribute('name')]
+      final_testsuite.appendChild(testcase)
 
   return final_xml
 
@@ -288,7 +302,7 @@ class ShardingSupervisor(object):
   case the number of shards to execute will be the same, but they will be
   smaller, as the total number of shards in the test suite will be multiplied
   by 'total_slaves'.
- 
+
   For example, if you are on a quad core machine, the sharding supervisor by
   default will use 20 shards for the whole suite. However, if you set
   total_slaves to 2, it will split the suite in 40 shards and will only run
diff --git a/tools/sharding_supervisor/sharding_supervisor_unittest.py b/tools/sharding_supervisor/sharding_supervisor_unittest.py
index 216d606..b6e9ca5 100755
--- a/tools/sharding_supervisor/sharding_supervisor_unittest.py
+++ b/tools/sharding_supervisor/sharding_supervisor_unittest.py
@@ -5,16 +5,19 @@
 
 """Verify basic usage of sharding_supervisor."""
 
+import difflib
 import os
 import subprocess
 import sys
 import unittest
 
+from xml.dom import minidom
+
 import sharding_supervisor
 
-SHARDING_SUPERVISOR = os.path.join(os.path.dirname(sys.argv[0]),
-                                   'sharding_supervisor.py')
-DUMMY_TEST = os.path.join(os.path.dirname(sys.argv[0]), 'dummy_test.py')
+ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
+SHARDING_SUPERVISOR = os.path.join(ROOT_DIR, 'sharding_supervisor.py')
+DUMMY_TEST = os.path.join(ROOT_DIR, 'dummy_test.py')
 NUM_CORES = sharding_supervisor.DetectNumCores()
 SHARDS_PER_CORE = sharding_supervisor.SS_DEFAULT_SHARDS_PER_CORE
 
@@ -34,8 +37,8 @@ class ShardingSupervisorUnittest(unittest.TestCase):
   def test_basic_run(self):
     # Default test.
     expected_shards = NUM_CORES * SHARDS_PER_CORE
-    (expected_out, expected_err) = \
-        generate_expected_output(0, expected_shards, expected_shards)
+    (expected_out, expected_err) = generate_expected_output(
+        0, expected_shards, expected_shards)
     p = subprocess.Popen([SHARDING_SUPERVISOR, '--no-color', DUMMY_TEST],
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
@@ -47,8 +50,8 @@ class ShardingSupervisorUnittest(unittest.TestCase):
   def test_shard_per_core(self):
     """Test the --shards_per_core parameter."""
     expected_shards = NUM_CORES * 25
-    (expected_out, expected_err) = \
-        generate_expected_output(0, expected_shards, expected_shards)
+    (expected_out, expected_err) = generate_expected_output(
+        0, expected_shards, expected_shards)
     p = subprocess.Popen([SHARDING_SUPERVISOR, '--no-color',
                           '--shards_per_core', '25', DUMMY_TEST],
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -67,8 +70,8 @@ class ShardingSupervisorUnittest(unittest.TestCase):
     for index in range(total_shards):
       begin = NUM_CORES * SHARDS_PER_CORE * index
       end = begin + NUM_CORES * SHARDS_PER_CORE
-      (expected_out, expected_err) = \
-          generate_expected_output(begin, end, expected_shards)
+      (expected_out, expected_err) = generate_expected_output(
+          begin, end, expected_shards)
       p = subprocess.Popen([SHARDING_SUPERVISOR, '--no-color',
                             '--total-slaves', str(total_shards),
                             '--slave-index', str(index),
@@ -80,6 +83,44 @@ class ShardingSupervisorUnittest(unittest.TestCase):
       self.assertEqual(expected_err, err)
       self.assertEqual(0, p.returncode)
 
+  def test_append_to_xml(self):
+    shard_xml_path = os.path.join(ROOT_DIR, 'data', 'gtest_results.xml')
+    expected_xml_path = os.path.join(
+        ROOT_DIR, 'data', 'gtest_results_expected.xml')
+    merged_xml = sharding_supervisor.AppendToXML(None, shard_xml_path, 0)
+    merged_xml = sharding_supervisor.AppendToXML(merged_xml, shard_xml_path, 1)
+
+    with open(expected_xml_path) as expected_xml_file:
+      expected_xml = minidom.parse(expected_xml_file)
+
+    # Serialize XML to a list of strings that is consistently formatted
+    # (ignoring whitespace between elements) so that it may be compared.
+    def _serialize_xml(xml):
+      def _remove_whitespace_and_comments(xml):
+        children_to_remove = []
+        for child in xml.childNodes:
+          if (child.nodeType == minidom.Node.TEXT_NODE and
+              not child.data.strip()):
+            children_to_remove.append(child)
+          elif child.nodeType == minidom.Node.COMMENT_NODE:
+            children_to_remove.append(child)
+          elif child.nodeType == minidom.Node.ELEMENT_NODE:
+            _remove_whitespace_and_comments(child)
+
+        for child in children_to_remove:
+          xml.removeChild(child)
+
+      _remove_whitespace_and_comments(xml)
+      return xml.toprettyxml(indent='  ').splitlines()
+
+    diff = list(difflib.unified_diff(
+        _serialize_xml(expected_xml),
+        _serialize_xml(merged_xml),
+        fromfile='gtest_results_expected.xml',
+        tofile='gtest_results_actual.xml'))
+    if diff:
+      self.fail('Did not merge results XML correctly:\n' + '\n'.join(diff))
+
 
 if __name__ == '__main__':
   unittest.main()