Diff
Modified: trunk/Tools/ChangeLog (133327 => 133328)
--- trunk/Tools/ChangeLog 2012-11-02 19:02:28 UTC (rev 133327)
+++ trunk/Tools/ChangeLog 2012-11-02 19:02:30 UTC (rev 133328)
@@ -1,3 +1,39 @@
+2012-11-02 Dirk Pranke <dpra...@chromium.org>
+
+ webkit-patch analyze-baselines output is weak
+ https://bugs.webkit.org/show_bug.cgi?id=100998
+
+ Reviewed by Ojan Vafai.
+
+ Currently analyze-baselines prints a list of baselines that have
+ the same checksum per line, but the format is hard to read;
+ this patch cleans up the output to print by directory instead
+ and use the same format I recently added for debugging optimize-baselines,
+ then refactors the code so that we share and adds tests for
+ analyze-baselines (which was untested).
+
+ Also, I got rid of a couple of unnecessarily-hardcoded port names,
+ and modified the baseline optimizer to log the current world when
+ optimize fails.
+
+ * Scripts/webkitpy/common/checkout/baselineoptimizer.py:
+ (BaselineOptimizer.read_results_by_directory):
+ (BaselineOptimizer.write_by_directory):
+ (BaselineOptimizer.optimize):
+ * Scripts/webkitpy/tool/commands/rebaseline.py:
+ (OptimizeBaselines.execute):
+ (AnalyzeBaselines.__init__):
+ (AnalyzeBaselines._write):
+ (AnalyzeBaselines._analyze_baseline):
+ (AnalyzeBaselines.execute):
+ * Scripts/webkitpy/tool/commands/rebaseline_unittest.py:
+ (_FakeOptimizer):
+ (_FakeOptimizer.read_results_by_directory):
+ (TestAnalyzeBaselines):
+ (TestAnalyzeBaselines.setUp):
+ (TestAnalyzeBaselines.test_default):
+ (TestAnalyzeBaselines.test_missing_baselines):
+
2012-11-02 Adam Barth <aba...@webkit.org>
ENABLE(UNDO_MANAGER) is disabled everywhere and is not under active development
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py (133327 => 133328)
--- trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py 2012-11-02 19:02:28 UTC (rev 133327)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py 2012-11-02 19:02:30 UTC (rev 133328)
@@ -81,7 +81,7 @@
self._hypergraph = _baseline_search_hypergraph(host)
self._directories = reduce(set.union, map(set, self._hypergraph.values()))
- def _read_results_by_directory(self, baseline_name):
+ def read_results_by_directory(self, baseline_name):
results_by_directory = {}
for directory in self._directories:
path = self._filesystem.join(self._scm.checkout_root, directory, baseline_name)
@@ -122,7 +122,7 @@
results_by_directory[directory] = result
def _find_optimal_result_placement(self, baseline_name):
- results_by_directory = self._read_results_by_directory(baseline_name)
+ results_by_directory = self.read_results_by_directory(baseline_name)
results_by_port_name = self._results_by_port_name(results_by_directory)
port_names_by_result = _invert_dictionary(results_by_port_name)
@@ -181,7 +181,7 @@
return best_so_far
except KeyError as e:
# FIXME: KeyErrors get raised if we're missing baselines. We should handle this better.
- return results_by_directory
+ return {}
def _find_in_fallbackpath(self, fallback_path, current_result, results_by_directory):
for index, directory in enumerate(fallback_path):
@@ -240,9 +240,13 @@
_log.debug(" (Nothing to add)")
def directories_by_result(self, baseline_name):
- results_by_directory = self._read_results_by_directory(baseline_name)
+ results_by_directory = self.read_results_by_directory(baseline_name)
return _invert_dictionary(results_by_directory)
+ def write_by_directory(self, results_by_directory, writer, indent):
+ for path in sorted(results_by_directory):
+ writer("%s%s: %s" % (indent, self._platform(path), results_by_directory[path][0:6]))
+
def optimize(self, baseline_name):
basename = self._filesystem.basename(baseline_name)
results_by_directory, new_results_by_directory = self._find_optimal_result_placement(baseline_name)
@@ -250,25 +254,20 @@
if new_results_by_directory == results_by_directory:
if new_results_by_directory:
_log.debug(" %s: (already optimal)" % basename)
- for path in sorted(results_by_directory):
- result = results_by_directory[path]
- _log.debug(" %s: %s" % (self._platform(path), result[0:6]))
+ self.write_by_directory(results_by_directory, _log.debug, " ")
else:
_log.debug(" %s: (no baselines found)" % basename)
return True
if self._filtered_results_by_port_name(results_by_directory) != self._filtered_results_by_port_name(new_results_by_directory):
- _log.warning("Optimization failed")
+ _log.warning(" %s: optimization failed" % basename)
+ self.write_by_directory(results_by_directory, _log.warning, " ")
return False
_log.debug(" %s:" % basename)
_log.debug(" Before: ")
- for path in sorted(results_by_directory):
- result = results_by_directory[path]
- _log.debug(" %s: %s" % (self._platform(path), result[0:6]))
+ self.write_by_directory(results_by_directory, _log.debug, " ")
_log.debug(" After: ")
- for path in sorted(new_results_by_directory):
- result = new_results_by_directory[path]
- _log.debug(" %s: %s" % (self._platform(path), result[0:6]))
+ self.write_by_directory(new_results_by_directory, _log.debug, " ")
self._move_baselines(baseline_name, results_by_directory, new_results_by_directory)
return True
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py (133327 => 133328)
--- trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py 2012-11-02 19:02:28 UTC (rev 133327)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py 2012-11-02 19:02:30 UTC (rev 133328)
@@ -42,7 +42,7 @@
# We override this method for testing so we don't have to construct an
# elaborate mock file system.
- def _read_results_by_directory(self, baseline_name):
+ def read_results_by_directory(self, baseline_name):
return self._mock_results_by_directory
def _move_baselines(self, baseline_name, results_by_directory, new_results_by_directory):
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline.py (133327 => 133328)
--- trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline.py 2012-11-02 19:02:28 UTC (rev 133327)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline.py 2012-11-02 19:02:30 UTC (rev 133328)
@@ -208,8 +208,7 @@
def execute(self, options, args, tool):
self._baseline_suffix_list = options.suffixes.split(',')
self._baseline_optimizer = BaselineOptimizer(tool)
- self._port = tool.port_factory.get("chromium-win-win7") # FIXME: This should be selectable.
-
+ self._port = tool.port_factory.get()
for test_name in self._port.tests(args):
_log.info("Optimizing %s" % test_name)
self._optimize_baseline(test_name)
@@ -221,28 +220,31 @@
argument_names = "TEST_NAMES"
def __init__(self):
- return super(AnalyzeBaselines, self).__init__(options=[self.suffixes_option])
+ super(AnalyzeBaselines, self).__init__(options=[
+ self.suffixes_option,
+ optparse.make_option('--missing', action='', default=False, help='show missing baselines as well'),
+ ])
+ self._optimizer_class = BaselineOptimizer # overridable for testing
- def _print(self, baseline_name, directories_by_result):
- for result, directories in directories_by_result.items():
- if len(directories) <= 1:
- continue
- results_names = [self._tool.filesystem.join(directory, baseline_name) for directory in directories]
- print ' '.join(results_names)
+ def _write(self, msg):
+ print msg
- def _analyze_baseline(self, test_name):
+ def _analyze_baseline(self, options, test_name):
for suffix in self._baseline_suffix_list:
baseline_name = _baseline_name(self._tool.filesystem, test_name, suffix)
- directories_by_result = self._baseline_optimizer.directories_by_result(baseline_name)
- self._print(baseline_name, directories_by_result)
+ results_by_directory = self._baseline_optimizer.read_results_by_directory(baseline_name)
+ if results_by_directory:
+ self._write("%s:" % baseline_name)
+ self._baseline_optimizer.write_by_directory(results_by_directory, self._write, " ")
+ elif options.missing:
+ self._write("%s: (no baselines found)" % baseline_name)
def execute(self, options, args, tool):
self._baseline_suffix_list = options.suffixes.split(',')
- self._baseline_optimizer = BaselineOptimizer(tool)
- self._port = tool.port_factory.get("chromium-win-win7") # FIXME: This should be selectable.
-
+ self._baseline_optimizer = self._optimizer_class(tool)
+ self._port = tool.port_factory.get()
for test_name in self._port.tests(args):
- self._analyze_baseline(test_name)
+ self._analyze_baseline(options, test_name)
class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py (133327 => 133328)
--- trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py 2012-11-02 19:02:28 UTC (rev 133327)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py 2012-11-02 19:02:30 UTC (rev 133328)
@@ -29,11 +29,12 @@
import unittest
from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.common.checkout.baselineoptimizer import BaselineOptimizer
+from webkitpy.common.net.buildbot.buildbot_mock import MockBuilder
+from webkitpy.common.system.executive_mock import MockExecutive2
from webkitpy.thirdparty.mock import Mock
from webkitpy.tool.commands.rebaseline import *
from webkitpy.tool.mocktool import MockTool, MockOptions
-from webkitpy.common.net.buildbot.buildbot_mock import MockBuilder
-from webkitpy.common.system.executive_mock import MockExecutive2
class _BaseTestCase(unittest.TestCase):
@@ -360,3 +361,36 @@
self.assertEquals(self._read(self.lion_expectations_path), '')
+class _FakeOptimizer(BaselineOptimizer):
+ def read_results_by_directory(self, baseline_name):
+ if baseline_name.endswith('txt'):
+ return {'LayoutTests/passes/text.html': '123456',
+ 'LayoutTests/platform/test-mac-leopard/passes/text.html': 'abcdef'}
+ return {}
+
+
+class TestAnalyzeBaselines(_BaseTestCase):
+ command_constructor = AnalyzeBaselines
+
+ def setUp(self):
+ super(TestAnalyzeBaselines, self).setUp()
+ self.port = self.tool.port_factory.get('test')
+ self.tool.port_factory.get = lambda port_name=None, options=None: self.port
+ self.lines = []
+ self.command._optimizer_class = _FakeOptimizer
+ self.command._write = lambda msg: self.lines.append(msg)
+
+ def test_default(self):
+ self.command.execute(MockOptions(suffixes='txt', missing=False), ['passes/text.html'], self.tool)
+ self.assertEquals(self.lines,
+ ['passes/text-expected.txt:',
+ ' (generic): 123456',
+ ' test-mac-leopard: abcdef'])
+
+ def test_missing_baselines(self):
+ self.command.execute(MockOptions(suffixes='png,txt', missing=True), ['passes/text.html'], self.tool)
+ self.assertEquals(self.lines,
+ ['passes/text-expected.png: (no baselines found)',
+ 'passes/text-expected.txt:',
+ ' (generic): 123456',
+ ' test-mac-leopard: abcdef'])
