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

tools/sharding_supervisor/sharding_supervisor_unittest.py

Index: tools/sharding_supervisor/sharding_supervisor_unittest.py
diff --git a/tools/sharding_supervisor/sharding_supervisor_unittest.py b/tools/sharding_supervisor/sharding_supervisor_unittest.py
index 216d606fbfa241a45d15c6520fb781782775e136..5bdc0cff64359dc9b3d4773d91235bb54a5e9d00 100755
--- a/tools/sharding_supervisor/sharding_supervisor_unittest.py
+++ b/tools/sharding_supervisor/sharding_supervisor_unittest.py
@@ -5,11 +5,14 @@
 
 """Verify basic usage of sharding_supervisor."""
 
+import difflib
 import os
 import subprocess
 import sys
 import unittest
 
+from xml.dom import minidom

[M-A Ruel, 2012/07/10 14:26:50]

IMHO, xml.etree.ElementTree is much easier&cleaner to use.

+
 import sharding_supervisor
 
 SHARDING_SUPERVISOR = os.path.join(os.path.dirname(sys.argv[0]),

[M-A Ruel, 2012/07/10 14:26:50]

Add
ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
SHARDING_SUPERVISOR = os.path.join(ROOT_DIR, 'sharding_supervisor.py')

note that abspath must be called *before* dirname.

[Mihai Parparita -not on Chrome, 2012/07/10 20:15:16]

Done.

@@ -80,6 +83,44 @@ class ShardingSupervisorUnittest(unittest.TestCase):
       self.assertEqual(expected_err, err)
       self.assertEqual(0, p.returncode)
 
+  def test_append_to_xml(self):
+    test_directory = os.path.abspath(os.path.dirname(__file__))

[M-A Ruel, 2012/07/10 14:26:50]

Move that as a constant at the top of the file, since any test could have a
side-effect of calling os.chdir()

+    shard_xml_path = os.path.join(test_directory, 'gtest_results.xml')

[M-A Ruel, 2012/07/10 14:26:50]

Please move all the test data files in a 'data' subdirectory to keep the
directory cleaner.

[Mihai Parparita -not on Chrome, 2012/07/10 20:15:16]

Done.

+    expected_xml_path = \

[M-A Ruel, 2012/07/10 14:26:50]

expected_xml_path = os.path.join(
   ...

we don't use \ in chromium's python

[Mihai Parparita -not on Chrome, 2012/07/10 20:15:16]

Done.

+        os.path.join(test_directory, '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 \

[M-A Ruel, 2012/07/10 14:26:50]

same

[Mihai Parparita -not on Chrome, 2012/07/10 20:15:16]

Done (and a bunch more that were already in the file).

+              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))

[M-A Ruel, 2012/07/10 14:26:50]

alignment

[Mihai Parparita -not on Chrome, 2012/07/10 20:15:16]

Done.

[M-A Ruel, 2012/07/10 14:26:50]

2 vertical lines between file-level symbols.

[Mihai Parparita -not on Chrome, 2012/07/10 20:15:16]

Done.

 if __name__ == '__main__':
   unittest.main()