Refactoring: Extract some logic from RebaselineCL.execute.

third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A command to fetch new baselines from try jobs for the current CL."""
 
 import json
 import logging
 import optparse
 
@@ skipping 45 matching lines @@
                 _log.error('  %s', path)
             return 1
 
         issue_number = self._get_issue_number()
         if issue_number is None:
             _log.error('No issue number for current branch.')
             return 1
         _log.debug('Issue number for current branch: %s', issue_number)
 
         builds = self.git_cl().latest_try_jobs(self._try_bots())
-
-        builders_with_pending_builds = self.builders_with_pending_builds(builds)
-        if builders_with_pending_builds:
-            _log.info('There are existing pending builds for:')
-            for builder in sorted(builders_with_pending_builds):
-                _log.info('  %s', builder)
+        self._log_pending_builds(builds)
         builders_with_no_results = self.builders_with_no_results(builds)
 
-        if options.trigger_jobs and builders_with_no_results:
-            self.trigger_builds(builders_with_no_results)
-            _log.info('Please re-run webkit-patch rebaseline-cl once all pending try jobs have finished.')
-            return 1
+        if options.trigger_jobs:
+            triggered = self.trigger_builds(builders_with_no_results)
+            if triggered:

[jeffcarp, 2017/05/19 20:21:45]

Could these two lines be collapsed?

[qyearsley, 2017/05/19 20:39:03]

Yep, and that would be a bit nicer I think; done.

+                return 1
 
         if builders_with_no_results and not options.fill_missing:
             _log.error('The following builders have no results:')
             for builder in builders_with_no_results:
                 _log.error('  %s', builder)
             return 1
 
         _log.debug('Getting results for issue %d.', issue_number)
         builds_to_results = self._fetch_results(builds)
         if not options.fill_missing and len(builds_to_results) < len(builds):
             return 1
 
-        test_baseline_set = TestBaselineSet(tool)
         if args:
-            for test in args:
-                for build in builds:
-                    if not builds_to_results.get(build):
-                        continue
-                    test_baseline_set.add(test, build)
+            test_baseline_set = self._make_test_baseline_set_for_tests(
+                args, builds_to_results)
         else:
             test_baseline_set = self._make_test_baseline_set(
-                builds_to_results,
-                only_changed_tests=options.only_changed_tests)
+                builds_to_results, options.only_changed_tests)
 
         if options.fill_missing:
             self.fill_in_missing_results(test_baseline_set)
 
         _log.debug('Rebaselining: %s', test_baseline_set)
 
         if not options.dry_run:
             self.rebaseline(options, test_baseline_set)
         return 0
 
     def _get_issue_number(self):
         """Returns the current CL issue number, or None."""
         issue = self.git_cl().get_issue_number()
         if not issue.isdigit():
             return None
         return int(issue)
 
     def git_cl(self):
         """Returns a GitCL instance. Can be overridden for tests."""
         return GitCL(self._tool)
 
     def trigger_builds(self, builders):
+        """Triggers try jobs if necessary; returns whether builds were triggered."""
+        if builders is None:
+            return False
         _log.info('Triggering try jobs for:')
         for builder in sorted(builders):
             _log.info('  %s', builder)
         self.git_cl().trigger_try_jobs(builders)
+        _log.info('Once all pending try jobs have finished, please re-run\n'
+                  'webkit-patch rebaseline-cl to fetch new baselines.')
+        return True
 
     def builders_with_no_results(self, builds):
         """Returns the set of builders that don't have finished results."""
         builders_with_no_builds = set(self._try_bots()) - {b.builder_name for b in builds}
         return builders_with_no_builds | self.builders_with_pending_builds(builds)
 
+    def _log_pending_builds(self, builds):
+        builders = self.builders_with_pending_builds(builds)
+        if not builders:
+            return
+        _log.info('There are existing pending builds for:')
+        for builder in sorted(builders):
+            _log.info('  %s', builder)
+
     def builders_with_pending_builds(self, builds):
         """Returns the set of builders that have pending builds."""
         return {b.builder_name for b in builds if b.build_number is None}
 
     def _try_bots(self):
         """Returns a collection of try bot builders to fetch results for."""
         return self._tool.builders.all_try_builder_names()
 
     def _fetch_results(self, builds):
         """Fetches results for all of the given builds.
@@ skipping 17 matching lines @@
             results_url = buildbot.results_url(build.builder_name, build.build_number)
             layout_test_results = buildbot.fetch_results(build)
             if layout_test_results is None:
                 _log.info('Failed to fetch results for %s', build)
                 _log.info('Results URL: %s/results.html', results_url)
                 _log.info('Retry job by running: git cl try -b %s', build.builder_name)
                 continue
             results[build] = layout_test_results
         return results
 
+    def _make_test_baseline_set_for_tests(self, tests, builds_to_results):
+        test_baseline_set = TestBaselineSet(self._tool)
+        for test in tests:
+            for build in builds_to_results:
+                test_baseline_set.add(test, build)
+        return test_baseline_set
+
     def _make_test_baseline_set(self, builds_to_results, only_changed_tests):
         """Returns a dict which lists the set of baselines to fetch.
 
         The dict that is returned is a dict of tests to Build objects
         to baseline file extensions.
 
         Args:
             builds_to_results: A dict mapping Builds to LayoutTestResults.
             only_changed_tests: Whether to only include baselines for tests that
                are changed in this CL. If False, all new baselines for failing
@@ skipping 86 matching lines @@
 
         # If any Build exists with the same OS, use the first one.
         target_os = os_name(target_port)
         same_os_builds = sorted(b for b, p in build_port_pairs if os_name(p) == target_os)
         if same_os_builds:
             return same_os_builds[0]
 
         # Otherwise, perhaps any build will do, for example if the results are
         # the same on all platforms. In this case, just return the first build.
         return sorted(build_port_pairs)[0][0]