Changeset 121821 in webkit

Timestamp:

Jul 3, 2012 7:13:13 PM (12 years ago)

Author:

ojan@chromium.org

Message:

Improve webkit-patch rebaseline to work for more cases
​https://bugs.webkit.org/show_bug.cgi?id=90504

Reviewed by Dirk Pranke.

-Makes it work for the build.chromium.org bots.
-Lets you rebaseline all builders instead of just one.
-Lets you pass in the builder or test to rebaseline.

• Scripts/webkitpy/common/net/buildbot/buildbot.py:

(Build.results_url):
Make this work for build.chromium.org builders as well.

• Scripts/webkitpy/common/net/buildbot/chromiumbuildbot.py:

(ChromiumBuilder):
(ChromiumBuilder.latest_cached_build):
ChromiumBuilder doesn't support large parts of the Builder interface.
This provides the bare minimum for this patch to work. Eventually,
we should create a single interface that can be supported for both
sets of buildbots.

• Scripts/webkitpy/layout_tests/port/builders.py:

(builder_path_from_name):
(all_builder_names):
memoizing here is incorrect because the test override _exact_matches,
so these can return different values. In either case, I'm pretty sure these
are not remotely hot codepaths.

• Scripts/webkitpy/tool/commands/rebaseline.py:

(AbstractParallelRebaselineCommand.init):
(Rebaseline):
(Rebaseline.init):
(Rebaseline._builders_to_pull_from):
(Rebaseline._builder_with_name):
(Rebaseline._tests_to_update):
(Rebaseline.execute):

• Scripts/webkitpy/tool/commands/rebaseline_unittest.py:

(test_rebaseline.mock_builders_to_pull_from):
(test_rebaseline):
(test_rebaseline_command_line_flags):
(test_rebaseline_multiple_builders):
(test_rebaseline_multiple_builders.mock_builders_to_pull_from):
(test_rebaseline_multiple_builders.mock_tests_to_update):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/net/buildbot/buildbot.py (modified) (1 diff)
• Scripts/webkitpy/common/net/buildbot/chromiumbuildbot.py (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/builders.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/rebaseline.py (modified) (2 diffs)
• Scripts/webkitpy/tool/commands/rebaseline_unittest.py (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-07-03  Ojan Vafai  <ojan@chromium.org>
+
+        Improve webkit-patch rebaseline to work for more cases
+        https://bugs.webkit.org/show_bug.cgi?id=90504
+
+        Reviewed by Dirk Pranke.
+
+        -Makes it work for the build.chromium.org bots.
+        -Lets you rebaseline all builders instead of just one.
+        -Lets you pass in the builder or test to rebaseline.
+
+        * Scripts/webkitpy/common/net/buildbot/buildbot.py:
+        (Build.results_url):
+        Make this work for build.chromium.org builders as well.
+
+        * Scripts/webkitpy/common/net/buildbot/chromiumbuildbot.py:
+        (ChromiumBuilder):
+        (ChromiumBuilder.latest_cached_build):
+        ChromiumBuilder doesn't support large parts of the Builder interface.
+        This provides the bare minimum for this patch to work. Eventually,
+        we should create a single interface that can be supported for both
+        sets of buildbots.
+
+        * Scripts/webkitpy/layout_tests/port/builders.py:
+        (builder_path_from_name):
+        (all_builder_names):
+        memoizing here is incorrect because the test override _exact_matches,
+        so these can return different values. In either case, I'm pretty sure these
+        are not remotely hot codepaths.
+
+        * Scripts/webkitpy/tool/commands/rebaseline.py:
+        (AbstractParallelRebaselineCommand.__init__):
+        (Rebaseline):
+        (Rebaseline.__init__):
+        (Rebaseline._builders_to_pull_from):
+        (Rebaseline._builder_with_name):
+        (Rebaseline._tests_to_update):
+        (Rebaseline.execute):
+        * Scripts/webkitpy/tool/commands/rebaseline_unittest.py:
+        (test_rebaseline.mock_builders_to_pull_from):
+        (test_rebaseline):
+        (test_rebaseline_command_line_flags):
+        (test_rebaseline_multiple_builders):
+        (test_rebaseline_multiple_builders.mock_builders_to_pull_from):
+        (test_rebaseline_multiple_builders.mock_tests_to_update):
+
 2012-07-03  Christophe Dumez  <christophe.dumez@intel.com>
 

trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py

@@ -240 +240 @@
 
     def results_url(self):
-        results_directory = "r%s (%s)" % (self.revision(), self._number)
+        # FIXME: We should probably have a better way of determining chromium builders than checking if the name starts with 'Webkit'.
+        results_directory = "results/layout-test-results" if self._builder.name().startswith('Webkit') else "r%s (%s)" % (self.revision(), self._number)
         return "%s/%s" % (self._builder.results_url(), urllib.quote(results_directory))
 

trunk/Tools/Scripts/webkitpy/common/net/buildbot/chromiumbuildbot.py

@@ -42 +42 @@
         return self.results_url() + "/results/layout-test-results"
 
+    # We override Builder.latest_cached_build because it relies on functions that assume too much about
+    # the information provided on the buildbot pages.
+    # FIXME: Have Builder and ChromiumBuilder actually provide a unified inteface that they both support.
+    def latest_cached_build(self):
+        for builder_status in self._buildbot.builder_statuses():
+            if builder_status["name"] == self.name():
+                build_number = builder_status["build_number"]
+                break
+        return self.build(build_number)
+
 
 class ChromiumBuildBot(BuildBot):

trunk/Tools/Scripts/webkitpy/layout_tests/port/builders.py

@@ -97 +97 @@
 
 
-@memoized
 def all_builder_names():
     return sorted(set(_exact_matches.keys()))
 
 
-@memoized
 def all_port_names():
     return sorted(set(map(lambda x: x["port_name"], _exact_matches.values()) + _ports_without_builders))

trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline.py

@@ -230 +230 @@
 
 class AbstractParallelRebaselineCommand(AbstractDeclarativeCommand):
-    def __init__(self):
-        options = [
+    def __init__(self, options=None):
+        options = options or []
+        options.extend([
             optparse.make_option('--no-optimize', dest='optimize', action='store_false', default=True,
                 help=('Do not optimize/de-dup the expectations after rebaselining '
                       '(default is to de-dup automatically). '
-                      'You can use "webkit-patch optimize-baselines" to optimize separately.')),
-        ]
+                      'You can use "webkit-patch optimize-baselines" to optimize separately.'))])
         AbstractDeclarativeCommand.__init__(self, options=options)
 
@@ -371 +371 @@
     name = "rebaseline"
     help_text = "Replaces local expected.txt files with new results from build bots"
-
-    # FIXME: This should share more code with FailureReason._builder_to_explain
-    def _builder_to_pull_from(self):
-        builder_statuses = self._tool.buildbot.builder_statuses()
-        red_statuses = [status for status in builder_statuses if not status["is_green"]]
-        _log.info("%s failing" % (pluralize("builder", len(red_statuses))))
-        builder_choices = [status["name"] for status in red_statuses]
-        chosen_name = self._tool.user.prompt_with_list("Which builder to pull results from:", builder_choices)
-        # FIXME: prompt_with_list should really take a set of objects and a set of names and then return the object.
-        for status in red_statuses:
-            if status["name"] == chosen_name:
-                return (self._tool.buildbot.builder_with_name(chosen_name), status["build_number"])
-
-    def _tests_to_update(self, build):
-        failing_tests = build.layout_test_results().tests_matching_failure_types([test_failures.FailureTextMismatch])
-        return self._tool.user.prompt_with_list("Which test(s) to rebaseline:", failing_tests, can_choose_multiple=True)
-
-    def execute(self, options, args, tool):
-        builder, build_number = self._builder_to_pull_from()
-        build = builder.build(build_number)
-
-        builder_name = builder.name()
+    _chromium_prefix = 'Chromium - '
+
+    def __init__(self):
+        options = [
+            # FIXME: Make this builders/tests and have it take a comma-separated list.
+            optparse.make_option("--builder", default=None, help="Builder to pull new baselines from"),
+            optparse.make_option("--test", help="Test to rebaseline"),
+        ]
+        AbstractParallelRebaselineCommand.__init__(self, options=options)
+
+    def _builders_to_pull_from(self):
+        # FIXME: Instead of prefixing the chromium builder names, show a build.webkit.org list and a build.chromium.org list.
+        builder_names = [self._chromium_prefix + name if name.startswith('Webkit') else name for name in builders.all_builder_names()]
+        chosen_names = self._tool.user.prompt_with_list("Which builder to pull results from:", builder_names, can_choose_multiple=True)
+        return [self._builder_with_name(name) for name in chosen_names]
+
+    def _builder_with_name(self, name):
+        if name.startswith(self._chromium_prefix):
+            name = name[len(self._chromium_prefix):]
+
+        # FIXME: Share this code with the code in RebaselineTest._results_url.
+        port = self._tool.port_factory.get_from_builder_name(name)
+        if port.name().startswith('chromium-'):
+            return self._tool.chromium_buildbot().builder_with_name(name)
+        return self._tool.buildbot.builder_with_name(name)
+
+    def _tests_to_update(self, builder):
+        failing_tests = builder.latest_cached_build().layout_test_results().tests_matching_failure_types([test_failures.FailureTextMismatch])
+        return self._tool.user.prompt_with_list("Which test(s) to rebaseline for %s:" % builder.name(), failing_tests, can_choose_multiple=True)
+
+    def execute(self, options, args, tool):
+        if options.builder:
+            builders = [self._builder_with_name(options.builder)]
+        else:
+            builders = self._builders_to_pull_from()
+
         test_list = {}
-        for test in self._tests_to_update(build):
-            test_list[test] = {builder_name: ['txt']}
+
+        for builder in builders:
+            tests = [options.test] if options.test else self._tests_to_update(builder)
+            for test in tests:
+                if test not in test_list:
+                    test_list[test] = {}
+                # FIXME: Allow for choosing the suffixes.
+                test_list[test][builder.name()] = ['txt']
+
+        if options.verbose:
+            print "rebaseline-json: " + str(test_list)
+
         self._rebaseline(options, test_list)

trunk/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py

@@ -352 +352 @@
             tool.executive = MockExecutive(should_log=True)
 
-            def mock_builder_to_pull_from():
-                return MockBuilder('MOCK builder'), 1234
+            def mock_builders_to_pull_from():
+                return [MockBuilder('MOCK builder')]
 
             def mock_tests_to_update(build):
                 return ['mock/path/to/test.html']
 
-            command._builder_to_pull_from = mock_builder_to_pull_from
+            command._builders_to_pull_from = mock_builders_to_pull_from
             command._tests_to_update = mock_tests_to_update
+
+            expected_stdout = """rebaseline-json: {'mock/path/to/test.html': {'MOCK builder': ['txt']}}
+"""
 
             expected_stderr = """MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt', '--builder', 'MOCK builder', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout
@@ -365 +368 @@
 """
 
-            OutputCapture().assert_outputs(self, command.execute, [MockOptions(optimize=True), [], tool], expected_stderr=expected_stderr)
+            OutputCapture().assert_outputs(self, command.execute, [MockOptions(optimize=True, builder=None, test=None, verbose=True), [], tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr)
 
         finally:
             builders._exact_matches = old_exact_matches
+
+    def test_rebaseline_command_line_flags(self):
+        old_exact_matches = builders._exact_matches
+        try:
+            builders._exact_matches = {
+                "MOCK builder": {"port_name": "test-mac-leopard", "specifiers": set(["mock-specifier"])},
+            }
+
+            command = Rebaseline()
+            tool = MockTool()
+            command.bind_to_tool(tool)
+
+            for port_name in tool.port_factory.all_port_names():
+                port = tool.port_factory.get(port_name)
+                for path in port.expectations_files():
+                    tool.filesystem.write_text_file(path, '')
+
+            tool.executive = MockExecutive(should_log=True)
+
+            expected_stdout = """rebaseline-json: {'mock/path/to/test.html': {'MOCK builder': ['txt']}}
+"""
+
+            expected_stderr = """MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt', '--builder', 'MOCK builder', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout
+MOCK run_command: ['echo', 'optimize-baselines', '--suffixes', 'txt', 'mock/path/to/test.html'], cwd=/mock-checkout
+"""
+
+            builder = "MOCK builder"
+            test = "mock/path/to/test.html"
+            OutputCapture().assert_outputs(self, command.execute, [MockOptions(optimize=True, builder=builder, test=test, verbose=True), [], tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr)
+
+        finally:
+            builders._exact_matches = old_exact_matches
+
+    def test_rebaseline_multiple_builders(self):
+        old_exact_matches = builders._exact_matches
+        try:
+            builders._exact_matches = {
+                "MOCK builder": {"port_name": "test-mac-leopard", "specifiers": set(["mock-specifier"])},
+                "MOCK builder2": {"port_name": "test-mac-snowleopard", "specifiers": set(["mock-specifier2"])},
+            }
+
+            command = Rebaseline()
+            tool = MockTool()
+            command.bind_to_tool(tool)
+
+            for port_name in tool.port_factory.all_port_names():
+                port = tool.port_factory.get(port_name)
+                for path in port.expectations_files():
+                    tool.filesystem.write_text_file(path, '')
+
+            tool.executive = MockExecutive(should_log=True)
+
+            def mock_builders_to_pull_from():
+                return [MockBuilder('MOCK builder'), MockBuilder('MOCK builder2')]
+
+            def mock_tests_to_update(build):
+                return ['mock/path/to/test.html']
+
+            command._builders_to_pull_from = mock_builders_to_pull_from
+            command._tests_to_update = mock_tests_to_update
+
+            expected_stdout = """rebaseline-json: {'mock/path/to/test.html': {'MOCK builder2': ['txt'], 'MOCK builder': ['txt']}}
+"""
+
+            expected_stderr = """MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt', '--builder', 'MOCK builder2', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout
+MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt', '--builder', 'MOCK builder', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout
+MOCK run_command: ['echo', 'optimize-baselines', '--suffixes', 'txt', 'mock/path/to/test.html'], cwd=/mock-checkout
+"""
+
+            OutputCapture().assert_outputs(self, command.execute, [MockOptions(optimize=True, builder=None, test=None, verbose=True), [], tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr)
+
+        finally:
+            builders._exact_matches = old_exact_matches