Bug 47872 - CC authors of flaky tests when the commit-queue hits a flaky test
Summary: CC authors of flaky tests when the commit-queue hits a flaky test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-18 19:25 PDT by Adam Barth
Modified: 2010-10-18 21:11 PDT (History)
2 users (show)

See Also:


Attachments
Patch (7.14 KB, patch)
2010-10-18 19:28 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (7.59 KB, patch)
2010-10-18 19:51 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (7.23 KB, patch)
2010-10-18 19:54 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-10-18 19:25:41 PDT
CC authors of flaky tests when the commit-queue hits a flaky test
Comment 1 Adam Barth 2010-10-18 19:28:23 PDT
Created attachment 71118 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-10-18 19:35:34 PDT
Comment on attachment 71118 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71118&action=review

Looks OK.  Please consider my comments below.

> WebKitTools/Scripts/webkitpy/common/checkout/api.py:118
> +    def recent_commit_infos_for_files(self, paths):

Eventually we'll want some sort of lookback_limit=5 argument here.

> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:295
> +        test_paths = map(path_for_layout_test, flaky_tests)
> +        commit_infos = self._tool.checkout().recent_commit_infos_for_files(test_paths)
> +        authors = [commit_info.author().bugzilla_email() for commit_info in commit_infos if commit_info.author()]

I would have made this a helper function.  authors = self._author_emails_for_tests(flaky_tests)

That would have cleaned up this function a little.

> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:298
> +            cc_explaination = "  The author(s) of the test(s) have been CCed on this bug."

I'm not sure this really needs to be conditional, but I guess it's nice that it is.  Is this condition tested?  Why not use a ternary to set this?
Comment 3 Adam Barth 2010-10-18 19:51:13 PDT
Created attachment 71121 [details]
Patch for landing
Comment 4 WebKit Commit Bot 2010-10-18 19:52:49 PDT
Comment on attachment 71121 [details]
Patch for landing

Rejecting patch 71121 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 71121]" exit_code: 2
Cleaning working directory
Updating working directory
Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=71121&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=47872&ctype=xml
Processing 1 patch from 1 bug.
Processing patch 71121 from bug 47872.
Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/4432075
Comment 5 Adam Barth 2010-10-18 19:54:33 PDT
Created attachment 71122 [details]
Patch
Comment 6 Adam Barth 2010-10-18 21:11:14 PDT
Comment on attachment 71122 [details]
Patch

Clearing flags on attachment: 71122

Committed r70023: <http://trac.webkit.org/changeset/70023>
Comment 7 Adam Barth 2010-10-18 21:11:20 PDT
All reviewed patches have been landed.  Closing bug.