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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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.