Bug 183575 - Add label on github when exporting wpt tests to w3c/web-platform-test repo
Summary: Add label on github when exporting wpt tests to w3c/web-platform-test repo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-12 12:45 PDT by Brendan McLoughlin
Modified: 2018-03-14 09:12 PDT (History)
9 users (show)

See Also:


Attachments
Patch (4.55 KB, patch)
2018-03-12 12:54 PDT, Brendan McLoughlin
no flags Details | Formatted Diff | Diff
Patch (4.54 KB, patch)
2018-03-13 11:56 PDT, Brendan McLoughlin
no flags Details | Formatted Diff | Diff
Patch (6.30 KB, patch)
2018-03-14 07:35 PDT, Brendan McLoughlin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan McLoughlin 2018-03-12 12:45:24 PDT
Add label on github when exporting wpt tests to w3c/web-platform-test repo
Comment 1 Brendan McLoughlin 2018-03-12 12:54:54 PDT
Created attachment 335627 [details]
Patch
Comment 2 youenn fablet 2018-03-12 13:37:40 PDT
Comment on attachment 335627 [details]
Patch

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

> Tools/Scripts/webkitpy/w3c/common.py:42
> +WEBKIT_EXPORT_PR_LABEL = 'webkit-export'

common.py is imported from Chromium.
Ideally, we would not want to change this file so that we can easily reimport it from Chromium.
We can probably move it to test_export.py.

> Tools/Scripts/webkitpy/w3c/test_exporter.py:181
>          pr_number = self._github.create_pr(self._wpt_fork_remote + ':' + self._branch_name, self._commit_message, self._commit_message + "\n" + description)

Would create_pr always succeed, or are there some cases where it might not be successful, in which case we should add some error handling?
Comment 3 Brendan McLoughlin 2018-03-12 13:44:49 PDT
Comment on attachment 335627 [details]
Patch

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

>> Tools/Scripts/webkitpy/w3c/test_exporter.py:181
>>          pr_number = self._github.create_pr(self._wpt_fork_remote + ':' + self._branch_name, self._commit_message, self._commit_message + "\n" + description)
> 
> Would create_pr always succeed, or are there some cases where it might not be successful, in which case we should add some error handling?

When testing this change locally I saw `create_pr` fail when my github token was missing the required permissions and when a pr had already been created for given bug_id. I will add some error handling for these cases.
Comment 4 Brendan McLoughlin 2018-03-13 11:56:17 PDT
Created attachment 335706 [details]
Patch
Comment 5 youenn fablet 2018-03-13 21:14:29 PDT
Comment on attachment 335706 [details]
Patch

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

> Tools/Scripts/webkitpy/w3c/test_exporter.py:189
> +                _log.info('Error creating a pull request on github. Please ensure that the provided github token has the "public_repo" scope.')

Might be easier to read if we put this method as a utility routine.
Comment 6 Brendan McLoughlin 2018-03-14 07:35:35 PDT
Created attachment 335767 [details]
Patch
Comment 7 WebKit Commit Bot 2018-03-14 09:11:07 PDT
Comment on attachment 335767 [details]
Patch

Clearing flags on attachment: 335767

Committed r229600: <https://trac.webkit.org/changeset/229600>
Comment 8 WebKit Commit Bot 2018-03-14 09:11:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-03-14 09:12:37 PDT
<rdar://problem/38460383>