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

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

BUG=136412
R=nsylvain@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=145952

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

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
File tools/sharding_supervisor/sharding_supervisor.py (right):

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor.py:130:
final_testsuites_by_name[final_testsuite.getAttribute('name')] = \
final_testsuites_by_name = dict(
  (suite.getAttribute('name'), suite) for suite in final_testsuites)

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
File tools/sharding_supervisor/sharding_supervisor_unittest.py (right):

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:14: from xml.dom
import minidom
IMHO, xml.etree.ElementTree is much easier&cleaner to use.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:18:
SHARDING_SUPERVISOR = os.path.join(os.path.dirname(sys.argv[0]),
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.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:87: test_directory =
os.path.abspath(os.path.dirname(__file__))
Move that as a constant at the top of the file, since any test could have a
side-effect of calling os.chdir()

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:88: shard_xml_path =
os.path.join(test_directory, 'gtest_results.xml')
Please move all the test data files in a 'data' subdirectory to keep the
directory cleaner.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:89: expected_xml_path
= \
expected_xml_path = os.path.join(
   ...

we don't use \ in chromium's python

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:103: if child.nodeType
== minidom.Node.TEXT_NODE and \
same

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:123: self.fail('Did
not merge results XML correctly:\n' + '\n'.join(diff))
alignment

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:124: 
2 vertical lines between file-level symbols.

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

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
File tools/sharding_supervisor/sharding_supervisor.py (right):

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor.py:130:
final_testsuites_by_name[final_testsuite.getAttribute('name')] = \
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> final_testsuites_by_name = dict(
>   (suite.getAttribute('name'), suite) for suite in final_testsuites)

Done.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
File tools/sharding_supervisor/sharding_supervisor_unittest.py (right):

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:18:
SHARDING_SUPERVISOR = os.path.join(os.path.dirname(sys.argv[0]),
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> 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.
> 

Done.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:88: shard_xml_path =
os.path.join(test_directory, 'gtest_results.xml')
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> Please move all the test data files in a 'data' subdirectory to keep the
> directory cleaner.

Done.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:89: expected_xml_path
= \
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> expected_xml_path = os.path.join(
>    ...
> 
> we don't use \ in chromium's python

Done.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:103: if child.nodeType
== minidom.Node.TEXT_NODE and \
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> same

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

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:123: self.fail('Did
not merge results XML correctly:\n' + '\n'.join(diff))
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> alignment

Done.

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor_unittest.py:124: 
On 2012/07/10 14:26:50, Marc-Antoine Ruel wrote:
> 2 vertical lines between file-level symbols.

Done.

[nsylvain, 2012-07-10 20:16:08]

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
File tools/sharding_supervisor/sharding_supervisor.py (right):

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor.py:162: final_testsuite =
final_testsuites_by_name[testsuite.getAttribute('name')]
is it possible that the testsuite is not in the final xml either?

[Mihai Parparita -not on Chrome, 2012-07-10 20:18:57]

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
File tools/sharding_supervisor/sharding_supervisor.py (right):

https://chromiumcodereview.appspot.com/10749018/diff/1/tools/sharding_supervi...
tools/sharding_supervisor/sharding_supervisor.py:162: final_testsuite =
final_testsuites_by_name[testsuite.getAttribute('name')]
On 2012/07/10 20:16:08, nsylvain wrote:
> is it possible that the testsuite is not in the final xml either?

That didn't seem to be the case with the data that I copied from a bot (the same
top-level suites appear in each shard file).

[nsylvain, 2012-07-10 20:26:49]

LGTM! 

Thanks for doing this!