New issue
Advanced search Search tips

Issue 654919 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

rebaseline-cl should not trigger new try jobs if try jobs are already triggered.

Project Member Reported by qyears...@chromium.org, Oct 11 2016

Issue description

Currently, if I run webkit-patch rebaseline-cl several times in a row when no try jobs results are ready, it starts multiple try jobs per builder.

Ideally, it should tell me which try jobs we're still waiting for, and start jobs only for builders that have no jobs started yet.
 
Labels: -OS-Linux -Type-Bug -Pri-3 Pri-2 Type-Feature
Note: adding triggering of try jobs before was  issue 639533 .
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b2243fab21e96c0cb75fc342a7a49ea7274f9d0

commit 6b2243fab21e96c0cb75fc342a7a49ea7274f9d0
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Oct 12 16:33:31 2016

Change Rietveld.latest_try_jobs to return try job result details.

In this CL:
 - Make latest_try_jobs return a dict mapping builds to result dicts
   and return that instead of just a list of builds.
 - Move and refactor filter_latest_builds and mark as protected.
 - Remove unused method get_latest_try_job_results.
 - Update tests and other callers.

Reason: In order to make rebaseline-cl not start new try jobs for
builders that already have try jobs started ( http://crbug.com/654919 ),
we need access to the current try job state/result information.

Possible changes to consider before submitting:
 - Maybe we don't want to pass on the result dicts from rietveld as-is,
   but perhaps instead we just want to return a couple pieces of
   information about the job (e.g. is it started? is it finished?)

BUG= 654919 

Review-Url: https://codereview.chromium.org/2406153003
Cr-Commit-Position: refs/heads/master@{#424761}

[modify] https://crrev.com/6b2243fab21e96c0cb75fc342a7a49ea7274f9d0/third_party/WebKit/Tools/Scripts/webkitpy/common/net/rietveld.py
[modify] https://crrev.com/6b2243fab21e96c0cb75fc342a7a49ea7274f9d0/third_party/WebKit/Tools/Scripts/webkitpy/common/net/rietveld_unittest.py
[modify] https://crrev.com/6b2243fab21e96c0cb75fc342a7a49ea7274f9d0/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
[modify] https://crrev.com/6b2243fab21e96c0cb75fc342a7a49ea7274f9d0/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e

commit 9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e
Author: qyearsley <qyearsley@chromium.org>
Date: Fri Oct 21 17:51:34 2016

rebaseline-cl: Don't trigger new try jobs for builders that already have jobs started.

In the preliminary CL http://crrev.com/2406153003, I changed Rietveld.latest_try_jobs to Rietveld.latest_try_job_results and made it return a dict of Builds (named tuple objects with builder_name and build_number) to result info dicts.

The purpose of this was to make it so that rebaseline-cl could look through the result info dicts to see whether each build is completed or just started, to avoid starting extra builds.

After looking at it some more, I see that in the Rietveld response, builds that are just started but not completed have no build number information.

To avoid having to pass around the result dict information from Rietveld, it's possible to just have a list of Build objects with build_number set to None when the build is just pending or started but not completed.

So, in summary, this CL makes it so that when one runs rebaseline-cl twice in a row, it now prints out:

$ ./webkit-patch rebaseline-cl
 Triggering try jobs for:
   android_blink_rel
   linux_precise_blink_rel
   linux_trusty_blink_rel
   mac10.10_blink_rel
   mac10.11_blink_rel
   mac10.11_retina_blink_rel
   mac10.9_blink_rel
   win10_blink_rel
   win7_blink_rel
 Please re-run webkit-patch rebaseline-cl once all pending try jobs have finished.
 $ ./webkit-patch rebaseline-cl
 There are existing pending builds for:
   android_blink_rel
   linux_precise_blink_rel
   linux_trusty_blink_rel
   mac10.10_blink_rel
   mac10.11_blink_rel
   mac10.11_retina_blink_rel
   mac10.9_blink_rel
   win10_blink_rel
   win7_blink_rel
 Please re-run webkit-patch rebaseline-cl once all pending try jobs have finished.

And this CL undoes some of the changes made in http://crrev.com/2406153003 in order to simplify the code, because now I think passing full result info dicts is unnecessary.

BUG= 654919 

Review-Url: https://chromiumcodereview.appspot.com/2439693003
Cr-Commit-Position: refs/heads/master@{#426837}

[modify] https://crrev.com/9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e/third_party/WebKit/Tools/Scripts/webkitpy/common/net/rietveld.py
[modify] https://crrev.com/9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e/third_party/WebKit/Tools/Scripts/webkitpy/common/net/rietveld_unittest.py
[modify] https://crrev.com/9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
[modify] https://crrev.com/9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl_unittest.py
[modify] https://crrev.com/9b5bb47fa0a7e219fdaf093a83ba2a1f63f63b7e/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py

Status: Fixed (was: Assigned)

Sign in to add a comment