Skip to content

Commit

Permalink
WPT: Allow setting GitHub credentials as env variables, warn when the…
Browse files Browse the repository at this point in the history
…y are not set

A fresh WPT import checkout is very likely to fail when no GitHub
credentials are used due to the number of API requests we make vs.
GitHub's rate limits for unauthenticated requests.

Make things more obvious by:
* Allowing the credentials to be specified as environment variables in
  addition to a JSON file (when both are set, the JSON file specified in
  the command-line has precedence).
* Printing a warning in test_importer.py when no credentials are set and
  pointing to the documentation in //docs/testing.
* Expanding the error message in test_exporter.py to point to the same
  location.

Bug: 816390
Change-Id: I5508e024e8a13e6f597f027cd157d7abbe9b2fe1
Reviewed-on: https://chromium-review.googlesource.com/937462
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#539164}
  • Loading branch information
Raphael Kubo da Costa authored and Commit Bot committed Feb 26, 2018
1 parent 6b79cf6 commit ac0fd25
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 7 deletions.
11 changes: 8 additions & 3 deletions docs/testing/web_platform_tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,14 @@ unauthenticated requests, so it is recommended that you let `wpt-export` and
`wpt-import` use your GitHub credentials when sending requests:

1. Generate a new [personal access token](https://github.com/settings/tokens)
1. Create a JSON file with two keys: `GH_USER`, your GitHub user name, and
`GH_TOKEN`, the access token you have just generated.
1. Pass `--credentials-json <path-to-json>` to `wpt-export` and `wpt-import`.
1. Set up your credentials by either:
* Setting the `GH_USER` environment variable to your GitHub user name
and the `GH_TOKEN` environment variable to the access token you have
just created **or**
* Creating a JSON file with two keys: `GH_USER`, your GitHub user name,
and `GH_TOKEN`, the access token you have just generated. After that,
pass `--credentials-json <path-to-json>` to `wpt-export` and
`wpt-import`.

### Manual import

Expand Down
17 changes: 15 additions & 2 deletions third_party/WebKit/Tools/Scripts/webkitpy/w3c/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,22 @@


def read_credentials(host, credentials_json):
"""Extracts credentials from a JSON file."""
"""Extracts GitHub and Gerrit credentials.
The data is read from the environment and (optionally) a JSON file. When a
JSON file is specified, any variables from the environment are discarded
and the values are all expected to be present in the file.
Args:
credentials_json: Path to a JSON file containing an object with the
keys we want to read, or None.
"""
env_credentials = {}
for key in ('GH_USER', 'GH_TOKEN', 'GERRIT_USER', 'GERRIT_TOKEN'):
if key in host.environ:
env_credentials[key] = host.environ[key]
if not credentials_json:
return {}
return env_credentials
if not host.filesystem.exists(credentials_json):
_log.warning('Credentials JSON file not found at %s.', credentials_json)
return {}
Expand Down
39 changes: 39 additions & 0 deletions third_party/WebKit/Tools/Scripts/webkitpy/w3c/common_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,24 @@ def test_get_credentials_empty(self):
def test_get_credentials_none(self):
self.assertEqual(read_credentials(MockHost(), None), {})

def test_get_credentials_gets_values_from_environment(self):
host = MockHost()
host.environ.update({
'GH_USER': 'user-github',
'GH_TOKEN': 'pass-github',
'GERRIT_USER': 'user-gerrit',
'GERRIT_TOKEN': 'pass-gerrit',
'UNUSED_VALUE': 'foo',
})
self.assertEqual(
read_credentials(host, None),
{
'GH_USER': 'user-github',
'GH_TOKEN': 'pass-github',
'GERRIT_USER': 'user-gerrit',
'GERRIT_TOKEN': 'pass-gerrit',
})

def test_get_credentials_gets_values_from_file(self):
host = MockHost()
host.filesystem.write_text_file(
Expand All @@ -44,6 +62,27 @@ def test_get_credentials_gets_values_from_file(self):
'GERRIT_TOKEN': 'pass-gerrit',
})

def test_get_credentials_choose_file_over_environment(self):
host = MockHost()
host.environ.update({
'GH_USER': 'user-github-from-env',
'GH_TOKEN': 'pass-github-from-env',
'GERRIT_USER': 'user-gerrit-from-env',
'GERRIT_TOKEN': 'pass-gerrit-from-env',
})
host.filesystem.write_text_file(
'/tmp/credentials.json',
json.dumps({
'GH_USER': 'user-github-from-json',
'GH_TOKEN': 'pass-github-from-json',
}))
self.assertEqual(
read_credentials(host, '/tmp/credentials.json'),
{
'GH_USER': 'user-github-from-json',
'GH_TOKEN': 'pass-github-from-json',
})

def test_is_testharness_baseline(self):
self.assertTrue(is_testharness_baseline('fake-test-expected.txt'))
self.assertTrue(is_testharness_baseline('external/wpt/fake-test-expected.txt'))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,13 @@ def main(self, argv=None):
self.host.executive.error_output_limit = None

credentials = read_credentials(self.host, options.credentials_json)
if not (credentials['GH_USER'] and credentials['GH_TOKEN']):
_log.error('Must provide both user and token for GitHub.')
if not (credentials.get('GH_USER') and credentials.get('GH_TOKEN')):
_log.error('You must provide your GitHub credentials for this '
'script to work.')
_log.error('See https://chromium.googlesource.com/chromium/src'
'/+/master/docs/testing/web_platform_tests.md'
'#GitHub-credentials for instructions on how to set '
'your credentials up.')
return False

self.wpt_github = self.wpt_github or WPTGitHub(self.host, credentials['GH_USER'], credentials['GH_TOKEN'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ def main(self, argv=None):
credentials = read_credentials(self.host, options.credentials_json)
gh_user = credentials.get('GH_USER')
gh_token = credentials.get('GH_TOKEN')
if not gh_user or not gh_token:
_log.warning('You have not set your GitHub credentials. This '
'script may fail with a network error when making '
'an API request to GitHub.')
_log.warning('See https://chromium.googlesource.com/chromium/src'
'/+/master/docs/testing/web_platform_tests.md'
'#GitHub-credentials for instructions on how to set '
'your credentials up.')
self.wpt_github = self.wpt_github or WPTGitHub(self.host, gh_user, gh_token)
self.git_cl = GitCL(self.host, auth_refresh_token_json=options.auth_refresh_token_json)

Expand Down

0 comments on commit ac0fd25

Please sign in to comment.