diff --git a/gerrit_client.py b/gerrit_client.py index aecb921fd4..ccf78b56a1 100755 --- a/gerrit_client.py +++ b/gerrit_client.py @@ -19,14 +19,11 @@ import sys import urllib import urlparse -from third_party import colorama import fix_encoding import gerrit_util import setup_color __version__ = '0.1' -# Shortcut since it quickly becomes redundant. -Fore = colorama.Fore def write_result(result, opt): @@ -47,6 +44,7 @@ def CMDbranchinfo(parser, args): logging.info(result) write_result(result, opt) + @subcommand.usage('[args ...]') def CMDbranch(parser, args): parser.add_option('--branch', dest='branch', help='branch name') @@ -124,7 +122,7 @@ class OptionParser(optparse.OptionParser): def main(argv): if sys.hexversion < 0x02060000: print('\nYour python version %s is unsupported, please upgrade.\n' - %(sys.version.split(' ', 1)[0],), + % (sys.version.split(' ', 1)[0],), file=sys.stderr) return 2 dispatcher = subcommand.CommandDispatcher(__name__) diff --git a/gerrit_util.py b/gerrit_util.py index 24cc357ea5..9e694cf37f 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -3,7 +3,7 @@ # found in the LICENSE file. """ -Utilities for requesting information for a gerrit server via https. +Utilities for requesting information for a Gerrit server via HTTPS. https://gerrit-review.googlesource.com/Documentation/rest-api.html """ @@ -44,7 +44,7 @@ LOGGER = logging.getLogger() TRY_LIMIT = 7 -# Controls the transport protocol used to communicate with gerrit. +# Controls the transport protocol used to communicate with Gerrit. # This is parameterized primarily to enable GerritTestCase. GERRIT_PROTOCOL = 'https' @@ -143,7 +143,7 @@ class CookiesAuthenticator(Authenticator): @classmethod def get_new_password_message(cls, host): if host is None: - return ('Git host for gerrit upload is unknown. Check your remote ' + return ('Git host for Gerrit upload is unknown. Check your remote ' 'and the branch your branch is tracking. This tool assumes ' 'that you are using a git server at *.googlesource.com.') assert not host.startswith('http') @@ -321,7 +321,6 @@ class GceAuthenticator(Authenticator): time.sleep(next_delay_sec) next_delay_sec *= 2 - @classmethod def _get_token_dict(cls): if cls._token_cache: @@ -366,7 +365,7 @@ class LuciContextAuthenticator(Authenticator): def CreateHttpConn(host, path, reqtype='GET', headers=None, body=None): - """Opens an https connection to a gerrit service, and sends a request.""" + """Opens an HTTPS connection to a Gerrit service, and sends a request.""" headers = headers or {} bare_host = host.partition(':')[0] @@ -407,7 +406,7 @@ def CreateHttpConn(host, path, reqtype='GET', headers=None, body=None): def ReadHttpResponse(conn, accept_statuses=frozenset([200])): - """Reads an http response from a connection into a string buffer. + """Reads an HTTP response from a connection into a string buffer. Args: conn: An Http object created by CreateHttpConn above. @@ -445,8 +444,8 @@ def ReadHttpResponse(conn, accept_statuses=frozenset([200])): LOGGER.debug('got response %d for %s %s', response.status, conn.req_params['method'], conn.req_params['uri']) # If 404 was in accept_statuses, then it's expected that the file might - # not exist, so don't return the gitiles error page because that's not the - # "content" that was actually requested. + # not exist, so don't return the gitiles error page because that's not + # the "content" that was actually requested. if response.status == 404: contents = '' break @@ -511,6 +510,7 @@ def QueryChanges(host, params, first_param=None, limit=None, o_params=None, start: how many changes to skip (starting with the most recent) o_params: A list of additional output specifiers, as documented here: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes + Returns: A list of json-decoded query results. """ @@ -529,11 +529,11 @@ def QueryChanges(host, params, first_param=None, limit=None, o_params=None, def GenerateAllChanges(host, params, first_param=None, limit=500, o_params=None, start=None): - """ - Queries a gerrit-on-borg server for all the changes matching the query terms. + """Queries a gerrit-on-borg server for all the changes matching the query + terms. WARNING: this is unreliable if a change matching the query is modified while - this function is being called. + this function is being called. A single query to gerrit-on-borg is limited on the number of results by the limit parameter on the request (see QueryChanges) and the server maximum @@ -549,6 +549,7 @@ def GenerateAllChanges(host, params, first_param=None, limit=500, A generator object to the list of returned changes. """ already_returned = set() + def at_most_once(cls): for cl in cls: if cl['_number'] not in already_returned: @@ -615,12 +616,12 @@ def MultiQueryChanges(host, params, change_list, limit=None, o_params=None, def GetGerritFetchUrl(host): - """Given a gerrit host name returns URL of a gerrit instance to fetch from.""" + """Given a Gerrit host name returns URL of a Gerrit instance to fetch from.""" return '%s://%s/' % (GERRIT_PROTOCOL, host) def GetCodeReviewTbrScore(host, project): - """Given a gerrit host name and project, return the Code-Review score for TBR. + """Given a Gerrit host name and project, return the Code-Review score for TBR. """ conn = CreateHttpConn(host, '/projects/%s' % urllib.quote(project, safe='')) project = ReadHttpJsonResponse(conn) @@ -632,23 +633,23 @@ def GetCodeReviewTbrScore(host, project): def GetChangePageUrl(host, change_number): - """Given a gerrit host name and change number, return change page url.""" + """Given a Gerrit host name and change number, returns change page URL.""" return '%s://%s/#/c/%d/' % (GERRIT_PROTOCOL, host, change_number) def GetChangeUrl(host, change): - """Given a gerrit host name and change id, return an url for the change.""" + """Given a Gerrit host name and change ID, returns a URL for the change.""" return '%s://%s/a/changes/%s' % (GERRIT_PROTOCOL, host, change) def GetChange(host, change): - """Query a gerrit server for information about a single change.""" + """Queries a Gerrit server for information about a single change.""" path = 'changes/%s' % change return ReadHttpJsonResponse(CreateHttpConn(host, path)) def GetChangeDetail(host, change, o_params=None): - """Query a gerrit server for extended information about a single change.""" + """Queries a Gerrit server for extended information about a single change.""" path = 'changes/%s/detail' % change if o_params: path += '?%s' % '&'.join(['o=%s' % p for p in o_params]) @@ -656,7 +657,7 @@ def GetChangeDetail(host, change, o_params=None): def GetChangeCommit(host, change, revision='current'): - """Query a gerrit server for a revision associated with a change.""" + """Query a Gerrit server for a revision associated with a change.""" path = 'changes/%s/revisions/%s/commit?links' % (change, revision) return ReadHttpJsonResponse(CreateHttpConn(host, path)) @@ -667,12 +668,12 @@ def GetChangeCurrentRevision(host, change): def GetChangeRevisions(host, change): - """Get information about all revisions associated with a change.""" + """Gets information about all revisions associated with a change.""" return QueryChanges(host, [], change, o_params=('ALL_REVISIONS',)) def GetChangeReview(host, change, revision=None): - """Get the current review information for a change.""" + """Gets the current review information for a change.""" if not revision: jmsg = GetChangeRevisions(host, change) if not jmsg: @@ -691,13 +692,13 @@ def GetChangeComments(host, change): def GetChangeRobotComments(host, change): - """Get the line- and file-level robot comments on a change.""" + """Gets the line- and file-level robot comments on a change.""" path = 'changes/%s/robotcomments' % change return ReadHttpJsonResponse(CreateHttpConn(host, path)) def AbandonChange(host, change, msg=''): - """Abandon a gerrit change.""" + """Abandons a Gerrit change.""" path = 'changes/%s/abandon' % change body = {'message': msg} if msg else {} conn = CreateHttpConn(host, path, reqtype='POST', body=body) @@ -705,7 +706,7 @@ def AbandonChange(host, change, msg=''): def RestoreChange(host, change, msg=''): - """Restore a previously abandoned change.""" + """Restores a previously abandoned change.""" path = 'changes/%s/restore' % change body = {'message': msg} if msg else {} conn = CreateHttpConn(host, path, reqtype='POST', body=body) @@ -713,7 +714,7 @@ def RestoreChange(host, change, msg=''): def SubmitChange(host, change, wait_for_merge=True): - """Submits a gerrit change via Gerrit.""" + """Submits a Gerrit change via Gerrit.""" path = 'changes/%s/submit' % change body = {'wait_for_merge': wait_for_merge} conn = CreateHttpConn(host, path, reqtype='POST', body=body) @@ -734,7 +735,7 @@ def HasPendingChangeEdit(host, change): def DeletePendingChangeEdit(host, change): conn = CreateHttpConn(host, 'changes/%s/edit' % change, reqtype='DELETE') - # On success, gerrit returns status 204; if the edit was already deleted it + # On success, Gerrit returns status 204; if the edit was already deleted it # returns 404. Anything else is an error. ReadHttpResponse(conn, accept_statuses=[204, 404]) @@ -755,13 +756,13 @@ def SetCommitMessage(host, change, description, notify='ALL'): def GetReviewers(host, change): - """Get information about all reviewers attached to a change.""" + """Gets information about all reviewers attached to a change.""" path = 'changes/%s/reviewers' % change return ReadHttpJsonResponse(CreateHttpConn(host, path)) def GetReview(host, change, revision): - """Get review information about a specific revision of a change.""" + """Gets review information about a specific revision of a change.""" path = 'changes/%s/revisions/%s/review' % (change, revision) return ReadHttpJsonResponse(CreateHttpConn(host, path)) @@ -810,7 +811,7 @@ def AddReviewers(host, change, reviewers=None, ccs=None, notify=True, def RemoveReviewers(host, change, remove=None): - """Remove reveiewers from a change.""" + """Removes reviewers from a change.""" if not remove: return if isinstance(remove, basestring): @@ -828,7 +829,7 @@ def RemoveReviewers(host, change, remove=None): def SetReview(host, change, msg=None, labels=None, notify=None, ready=None): - """Set labels and/or add a message to a code review.""" + """Sets labels and/or adds a message to a code review.""" if not msg and not labels: return path = 'changes/%s/revisions/current/review' % change @@ -853,7 +854,7 @@ def SetReview(host, change, msg=None, labels=None, notify=None, ready=None): def ResetReviewLabels(host, change, label, value='0', message=None, notify=None): - """Reset the value of a given label for all reviewers on a change.""" + """Resets the value of a given label for all reviewers on a change.""" # This is tricky, because we want to work on the "current revision", but # there's always the risk that "current revision" will change in between # API calls. So, we check "current revision" at the beginning and end; if @@ -898,12 +899,12 @@ def ResetReviewLabels(host, change, label, value='0', message=None, def CreateGerritBranch(host, project, branch, commit): - """ - Create a new branch from given project and commit + """Creates a new branch from given project and commit + https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#create-branch Returns: - A JSON with 'ref' key + A JSON object with 'ref' key. """ path = 'projects/%s/branches/%s' % (project, branch) body = {'revision': commit} @@ -915,12 +916,13 @@ def CreateGerritBranch(host, project, branch, commit): def GetGerritBranch(host, project, branch): - """ - Get a branch from given project and commit + """Gets a branch from given project and commit. + + See: https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-branch Returns: - A JSON object with 'revision' key + A JSON object with 'revision' key. """ path = 'projects/%s/branches/%s' % (project, branch) conn = CreateHttpConn(host, path, reqtype='GET') @@ -937,7 +939,7 @@ def GetAccountDetails(host, account_id='self'): whichever account user is authenticating as. Documentation: - https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#get-account + https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#get-account Returns None if account is not found (i.e., Gerrit returned 404). """ @@ -955,11 +957,13 @@ def ValidAccounts(host, accounts, max_threads=10): accounts = list(set(accounts)) if not accounts: return {} + def get_one(account): try: return account, GetAccountDetails(host, account) except GerritError: return None, None + valid = {} with contextlib.closing(ThreadPool(min(max_threads, len(accounts)))) as pool: for account, details in pool.map(get_one, accounts): @@ -969,12 +973,11 @@ def ValidAccounts(host, accounts, max_threads=10): def PercentEncodeForGitRef(original): - """Apply percent-encoding for strings sent to gerrit via git ref metadata. + """Applies percent-encoding for strings sent to Gerrit via git ref metadata. - The encoding used is based on but stricter than URL encoding (Section 2.1 - of RFC 3986). The only non-escaped characters are alphanumerics, and - 'SPACE' (U+0020) can be represented as 'LOW LINE' (U+005F) or - 'PLUS SIGN' (U+002B). + The encoding used is based on but stricter than URL encoding (Section 2.1 of + RFC 3986). The only non-escaped characters are alphanumerics, and 'SPACE' + (U+0020) can be represented as 'LOW LINE' (U+005F) or 'PLUS SIGN' (U+002B). For more information, see the Gerrit docs here: @@ -983,7 +986,7 @@ def PercentEncodeForGitRef(original): safe = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 ' encoded = ''.join(c if c in safe else '%%%02X' % ord(c) for c in original) - # spaces are not allowed in git refs; gerrit will interpret either '_' or + # Spaces are not allowed in git refs; gerrit will interpret either '_' or # '+' (or '%20') as space. Use '_' since that has been supported the longest. return encoded.replace(' ', '_') @@ -1016,12 +1019,12 @@ assert all(3 == len(p) for p in _GERRIT_MIRROR_PREFIXES) def _UseGerritMirror(url, host): - """Returns new url which uses randomly selected mirror for a gerrit host. + """Returns a new URL which uses randomly selected mirror for a Gerrit host. - url's host should be for a given host or a result of prior call to this + The URL's host should be for a given host or a result of prior call to this function. - Assumes url has a single occurence of the host substring. + Assumes that the URL has a single occurence of the host substring. """ assert host in url suffix = '-mirror-' + host @@ -1032,7 +1035,7 @@ def _UseGerritMirror(url, host): actual_host = host else: # Already uses some mirror. - assert st >= prefix_len, (uri, host, st, prefix_len) + assert st >= prefix_len, (url, host, st, prefix_len) prefixes.remove(url[st-prefix_len:st]) actual_host = url[st-prefix_len:st+len(suffix)] return url.replace(actual_host, random.choice(list(prefixes)) + suffix) diff --git a/git_cl.py b/git_cl.py index f8b1596008..ba5c54f5ff 100755 --- a/git_cl.py +++ b/git_cl.py @@ -15,7 +15,6 @@ import base64 import collections import contextlib import datetime -import fnmatch import httplib import itertools import json @@ -42,11 +41,9 @@ from third_party import httplib2 import auth import clang_format import dart_format -import setup_color import fix_encoding import gclient_utils import gerrit_util -import git_cache import git_common import git_footers import metrics @@ -55,6 +52,7 @@ import owners import owners_finder import presubmit_support import scm +import setup_color import split_cl import subcommand import subprocess2 @@ -112,10 +110,10 @@ DEFAULT_LINT_IGNORE_REGEX = r"$^" # File name for yapf style config files. YAPF_CONFIG_FILENAME = '.style.yapf' -# Buildbucket master name prefix. +# Buildbucket master name prefix for Buildbot masters. MASTER_PREFIX = 'master.' -# Shortcut since it quickly becomes redundant. +# Shortcut since it quickly becomes repetitive. Fore = colorama.Fore # Initialized in main() @@ -194,7 +192,7 @@ def IsGitVersionAtLeast(min_version): prefix = 'git version ' version = RunGit(['--version']).strip() return (version.startswith(prefix) and - LooseVersion(version[len(prefix):]) >= LooseVersion(min_version)) + LooseVersion(version[len(prefix):]) >= LooseVersion(min_version)) def BranchExists(branch): @@ -276,7 +274,7 @@ def _git_get_branch_config_value(key, default=None, value_type=str, args = ['config'] if value_type == bool: args.append('--bool') - # git config also has --int, but apparently git config suffers from integer + # `git config` also has --int, but apparently git config suffers from integer # overflows (http://crbug.com/640115), so don't use it. args.append(_git_branch_config_key(branch, key)) code, out = RunGitWithCode(args) @@ -307,8 +305,8 @@ def _git_set_branch_config_value(key, value, branch=None, **kwargs): args.append('--bool') value = str(value).lower() else: - # git config also has --int, but apparently git config suffers from integer - # overflows (http://crbug.com/640115), so don't use it. + # `git config` also has --int, but apparently git config suffers from + # integer overflows (http://crbug.com/640115), so don't use it. value = str(value) args.append(_git_branch_config_key(branch, key)) if value is not None: @@ -321,7 +319,7 @@ def _get_committer_timestamp(commit): Commit can be whatever git show would recognize, such as HEAD, sha1 or ref. """ - # Git also stores timezone offset, but it only affects visual display, + # Git also stores timezone offset, but it only affects visual display; # actual point in time is defined by this timestamp only. return int(RunGit(['show', '-s', '--format=%ct', commit]).strip()) @@ -395,9 +393,9 @@ def _buildbucket_retry(operation_name, http, *args, **kwargs): if response.status == 200: if content_json is None: raise BuildbucketResponseException( - 'Buildbucket returns invalid json content: %s.\n' + 'Buildbucket returned invalid JSON content: %s.\n' 'Please file bugs at http://crbug.com, ' - 'component "Infra>Platform>BuildBucket".' % + 'component "Infra>Platform>Buildbucket".' % content) return content_json if response.status < 500 or try_count >= 2: @@ -405,14 +403,14 @@ def _buildbucket_retry(operation_name, http, *args, **kwargs): # status >= 500 means transient failures. logging.debug('Transient errors when %s. Will retry.', operation_name) - time_sleep(0.5 + 1.5*try_count) + time_sleep(0.5 + (1.5 * try_count)) try_count += 1 assert False, 'unreachable' def _get_bucket_map(changelist, options, option_parser): """Returns a dict mapping bucket names to builders and tests, - for triggering try jobs. + for triggering tryjobs. """ # If no bots are listed, we try to get a set of builders and tests based # on GetPreferredTryMasters functions in PRESUBMIT.py files. @@ -443,11 +441,11 @@ def _get_bucket_map(changelist, options, option_parser): def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset): - """Sends a request to Buildbucket to trigger try jobs for a changelist. + """Sends a request to Buildbucket to trigger tryjobs for a changelist. Args: auth_config: AuthConfig for Buildbucket. - changelist: Changelist that the try jobs are associated with. + changelist: Changelist that the tryjobs are associated with. buckets: A nested dict mapping bucket names to builders to tests. options: Command-line options. """ @@ -522,7 +520,7 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset): ) _buildbucket_retry( - 'triggering try jobs', + 'triggering tryjobs', http, buildbucket_put_url, 'PUT', @@ -536,7 +534,7 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset): def fetch_try_jobs(auth_config, changelist, buildbucket_host, patchset=None): - """Fetches try jobs from buildbucket. + """Fetches tryjobs from buildbucket. Returns a map from build id to build info as a dictionary. """ @@ -570,7 +568,7 @@ def fetch_try_jobs(auth_config, changelist, buildbucket_host, url = 'https://{hostname}/_ah/api/buildbucket/v1/search?{params}'.format( hostname=buildbucket_host, params=urllib.urlencode(params)) - content = _buildbucket_retry('fetching try jobs', http, url, 'GET') + content = _buildbucket_retry('fetching tryjobs', http, url, 'GET') for build in content.get('builds', []): builds[build['id']] = build if 'next_cursor' in content: @@ -583,7 +581,7 @@ def fetch_try_jobs(auth_config, changelist, buildbucket_host, def print_try_jobs(options, builds): """Prints nicely result of fetch_try_jobs.""" if not builds: - print('No try jobs scheduled.') + print('No tryjobs scheduled.') return # Make a copy, because we'll be modifying builds dictionary. @@ -676,7 +674,7 @@ def print_try_jobs(options, builds): pop(title='Other:', f=lambda b: (get_name(b), 'id=%s' % b['id'])) assert len(builds) == 0 - print('Total: %d try jobs' % total) + print('Total: %d tryjobs' % total) def _ComputeDiffLineRanges(files, upstream_commit): @@ -734,8 +732,8 @@ def _FindYapfConfigFile(fpath, yapf_config_cache, top_dir=None): """Checks if a yapf file is in any parent directory of fpath until top_dir. Recursively checks parent directories to find yapf file and if no yapf file - is found returns None. Uses yapf_config_cache as a cache for - previously found configs. + is found returns None. Uses yapf_config_cache as a cache for previously found + configs. """ fpath = os.path.abspath(fpath) # Return result if we've already computed it. @@ -873,14 +871,14 @@ class Settings(object): return self._GetConfig('rietveld.cc', error_ok=True) def GetIsGerrit(self): - """Return true if this repo is associated with gerrit code review system.""" + """Returns True if this repo is associated with Gerrit.""" if self.is_gerrit is None: self.is_gerrit = ( self._GetConfig('gerrit.host', error_ok=True).lower() == 'true') return self.is_gerrit def GetSquashGerritUploads(self): - """Return true if uploads to Gerrit should be squashed by default.""" + """Returns True if uploads to Gerrit should be squashed by default.""" if self.squash_gerrit_uploads is None: self.squash_gerrit_uploads = self.GetSquashGerritUploadsOverride() if self.squash_gerrit_uploads is None: @@ -914,7 +912,7 @@ class Settings(object): return self.gerrit_skip_ensure_authenticated def GetGitEditor(self): - """Return the editor specified in the git config, or None if none is.""" + """Returns the editor specified in the git config, or None if none is.""" if self.git_editor is None: # Git requires single quotes for paths with spaces. We need to replace # them with double quotes for Windows to treat such paths as a single @@ -1357,8 +1355,8 @@ class Changelist(object): self._cached_remote_url = (True, url) return url - # If it cannot be parsed as an url, assume it is a local directory, probably - # a git cache. + # If it cannot be parsed as an url, assume it is a local directory, + # probably a git cache. logging.warning('"%s" doesn\'t appear to point to a git host. ' 'Interpreting it as a local directory.', url) if not os.path.isdir(url): @@ -1428,7 +1426,7 @@ class Changelist(object): raw_description = self.GetDescription() msg_lines, _, footers = git_footers.split_footers(raw_description) if footers: - msg_lines = msg_lines[:len(msg_lines)-1] + msg_lines = msg_lines[:len(msg_lines) - 1] return msg_lines, footers def GetPatchset(self): @@ -1601,10 +1599,9 @@ class Changelist(object): base_branch = self.GetCommonAncestorWithUpstream() git_diff_args = [base_branch, 'HEAD'] - - # Fast best-effort checks to abort before running potentially - # expensive hooks if uploading is likely to fail anyway. Passing these - # checks does not guarantee that uploading will not fail. + # Fast best-effort checks to abort before running potentially expensive + # hooks if uploading is likely to fail anyway. Passing these checks does + # not guarantee that uploading will not fail. self._codereview_impl.EnsureAuthenticated(force=options.force) self._codereview_impl.EnsureCanUploadPatchset(force=options.force) @@ -1718,11 +1715,11 @@ class Changelist(object): return self._codereview_impl.GetMostRecentPatchset() def CannotTriggerTryJobReason(self): - """Returns reason (str) if unable trigger try jobs on this CL or None.""" + """Returns reason (str) if unable trigger tryjobs on this CL or None.""" return self._codereview_impl.CannotTriggerTryJobReason() def GetTryJobProperties(self, patchset=None): - """Returns dictionary of properties to launch try job.""" + """Returns dictionary of properties to launch tryjob.""" return self._codereview_impl.GetTryJobProperties(patchset=patchset) def __getattr__(self, attr): @@ -1861,7 +1858,7 @@ class _ChangelistCodereviewBase(object): raise NotImplementedError() def CannotTriggerTryJobReason(self): - """Returns reason (str) if unable trigger try jobs on this CL or None.""" + """Returns reason (str) if unable trigger tryjobs on this CL or None.""" raise NotImplementedError() def GetIssueOwner(self): @@ -1899,7 +1896,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): parsed = urlparse.urlparse(self.GetRemoteUrl()) if parsed.scheme == 'sso': print('WARNING: using non-https URLs for remote is likely broken\n' - ' Your current remote is: %s' % self.GetRemoteUrl()) + ' Your current remote is: %s' % self.GetRemoteUrl()) self._gerrit_host = '%s.googlesource.com' % self._gerrit_host self._gerrit_server = 'https://%s' % self._gerrit_host return self._gerrit_host @@ -2075,7 +2072,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): return presubmit_support.GerritAccessor(self._GetGerritHost()) def GetStatus(self): - """Apply a rough heuristic to give a simple summary of an issue's review + """Applies a rough heuristic to give a simple summary of an issue's review or CQ status, assuming adherence to a common workflow. Returns None if no issue for this branch, or one of the following keywords: @@ -2327,7 +2324,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): def _IsCqConfigured(self): detail = self._GetChangeDetail(['LABELS']) - if not u'Commit-Queue' in detail.get('labels', {}): + if u'Commit-Queue' not in detail.get('labels', {}): return False # TODO(crbug/753213): Remove temporary hack if ('https://chromium.googlesource.com/chromium/src' == @@ -2482,8 +2479,8 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): hook = os.path.join(settings.GetRoot(), '.git', 'hooks', 'commit-msg') if not os.path.exists(hook): return - # Crude attempt to distinguish Gerrit Codereview hook from potentially - # custom developer made one. + # Crude attempt to distinguish Gerrit Codereview hook from a potentially + # custom developer-made one. data = gclient_utils.FileRead(hook) if not('From Gerrit Code Review' in data and 'add_ChangeId()' in data): return @@ -2978,9 +2975,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): def SetCQState(self, new_state): """Sets the Commit-Queue label assuming canonical CQ config for Gerrit.""" vote_map = { - _CQState.NONE: 0, - _CQState.DRY_RUN: 1, - _CQState.COMMIT: 2, + _CQState.NONE: 0, + _CQState.DRY_RUN: 1, + _CQState.COMMIT: 2, } labels = {'Commit-Queue': vote_map[new_state]} notify = False if new_state == _CQState.DRY_RUN else None @@ -2998,7 +2995,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): return 'CL %s is closed' % self.GetIssue() def GetTryJobProperties(self, patchset=None): - """Returns dictionary of properties to launch try job.""" + """Returns dictionary of properties to launch a tryjob.""" data = self._GetChangeDetail(['ALL_REVISIONS']) patchset = int(patchset or self.GetPatchset()) assert patchset @@ -3420,7 +3417,7 @@ def FindCodereviewSettingsFile(filename='codereview.settings'): def LoadCodereviewSettingsFromFile(fileobj): - """Parse a codereview.settings file and updates hooks.""" + """Parses a codereview.settings file and updates hooks.""" keyvals = gclient_utils.ParseCodereviewSettingsContent(fileobj.read()) def SetProperty(name, setting, unset_error_ok=False): @@ -3463,8 +3460,10 @@ def LoadCodereviewSettingsFromFile(fileobj): def urlretrieve(source, destination): - """urllib is broken for SSL connections via a proxy therefore we - can't use urllib.urlretrieve().""" + """Downloads a network object to a local file, like urllib.urlretrieve. + + This is necessary because urllib is broken for SSL connections via a proxy. + """ with open(destination, 'w') as f: f.write(urllib2.urlopen(source).read()) @@ -3481,7 +3480,7 @@ def DownloadHooks(*args, **kwargs): def DownloadGerritHook(force): - """Download and install Gerrit commit-msg hook. + """Downloads and installs a Gerrit commit-msg hook. Args: force: True to update hooks. False to install hooks if not present. @@ -3769,7 +3768,7 @@ class _GitCookiesChecker(object): found = True print('\n\n.gitcookies problem report:\n') bad_hosts.update(hosts or []) - print(' %s%s' % (title , (':' if sublines else ''))) + print(' %s%s' % (title, (':' if sublines else ''))) if sublines: print() print(' %s' % '\n '.join(sublines)) @@ -3833,6 +3832,7 @@ def CMDbaseurl(parser, args): return RunGit(['config', 'branch.%s.base-url' % branch, args[0]], error_ok=False).strip() + def color_for_status(status): """Maps a Changelist status to color, for CMDstatus and other tools.""" return { @@ -3910,18 +3910,19 @@ def upload_branch_deps(cl, args): """Uploads CLs of local branches that are dependents of the current branch. If the local branch dependency tree looks like: - test1 -> test2.1 -> test3.1 - -> test3.2 - -> test2.2 -> test3.3 + + test1 -> test2.1 -> test3.1 + -> test3.2 + -> test2.2 -> test3.3 and you run "git cl upload --dependencies" from test1 then "git cl upload" is run on the dependent branches in this order: test2.1, test3.1, test3.2, test2.2, test3.3 - Note: This function does not rebase your local dependent branches. Use it when - you make a change to the parent branch that will not conflict with its - dependent branches, and you would like their dependencies updated in - Rietveld. + Note: This function does not rebase your local dependent branches. Use it + when you make a change to the parent branch that will not conflict + with its dependent branches, and you would like their dependencies + updated in Rietveld. """ if git_common.is_dirty_git_tree('upload-branch-deps'): return 1 @@ -3941,8 +3942,8 @@ def upload_branch_deps(cl, args): print('No local branches found.') return 0 - # Create a dictionary of all local branches to the branches that are dependent - # on it. + # Create a dictionary of all local branches to the branches that are + # dependent on it. tracked_to_dependents = collections.defaultdict(list) for b in branches.splitlines(): tokens = b.split() @@ -3953,6 +3954,7 @@ def upload_branch_deps(cl, args): print() print('The dependent local branches of %s are:' % root_branch) dependents = [] + def traverse_dependents_preorder(branch, padding=''): dependents_to_process = tracked_to_dependents.get(branch, []) padding += ' ' @@ -3960,6 +3962,7 @@ def upload_branch_deps(cl, args): print('%s%s' % (padding, dependent)) dependents.append(dependent) traverse_dependents_preorder(dependent, padding) + traverse_dependents_preorder(root_branch) print() @@ -4557,14 +4560,14 @@ def CMDpresubmit(parser, args): def GenerateGerritChangeId(message): - """Returns Ixxxxxx...xxx change id. + """Returns the Change ID footer value (Ixxxxxx...xxx). Works the same way as https://gerrit-review.googlesource.com/tools/hooks/commit-msg but can be called on demand on all platforms. - The basic idea is to generate git hash of a state of the tree, original commit - message, author/committer info and timestamps. + The basic idea is to generate git hash of a state of the tree, original + commit message, author/committer info and timestamps. """ lines = [] tree_hash = RunGitSilent(['write-tree']) @@ -4656,16 +4659,16 @@ def CMDupload(parser, args): Can skip dependency patchset uploads for a branch by running: git config branch.branch_name.skip-deps-uploads True - To unset run: + To unset, run: git config --unset branch.branch_name.skip-deps-uploads Can also set the above globally by using the --global flag. - If the name of the checked out branch starts with "bug-" or "fix-" followed by - a bug number, this bug number is automatically populated in the CL + If the name of the checked out branch starts with "bug-" or "fix-" followed + by a bug number, this bug number is automatically populated in the CL description. If subject contains text in square brackets or has ": " prefix, such - text(s) is treated as Gerrit hashtags. For example, CLs with subjects + text(s) is treated as Gerrit hashtags. For example, CLs with subjects: [git-cl] add support for hashtags Foo bar: implement foo will be hashtagged with "git-cl" and "foo-bar" respectively. @@ -4797,22 +4800,22 @@ def CMDsplit(parser, args): comment, the string '$directory', is replaced with the directory containing the shared OWNERS file. """ - parser.add_option("-d", "--description", dest="description_file", - help="A text file containing a CL description in which " - "$directory will be replaced by each CL's directory.") - parser.add_option("-c", "--comment", dest="comment_file", - help="A text file containing a CL comment.") - parser.add_option("-n", "--dry-run", dest="dry_run", action='store_true', + parser.add_option('-d', '--description', dest='description_file', + help='A text file containing a CL description in which ' + '$directory will be replaced by each CL\'s directory.') + parser.add_option('-c', '--comment', dest='comment_file', + help='A text file containing a CL comment.') + parser.add_option('-n', '--dry-run', dest='dry_run', action='store_true', default=False, - help="List the files and reviewers for each CL that would " - "be created, but don't create branches or CLs.") - parser.add_option("--cq-dry-run", action='store_true', - help="If set, will do a cq dry run for each uploaded CL. " - "Please be careful when doing this; more than ~10 CLs " - "has the potential to overload our build " - "infrastructure. Try to upload these not during high " - "load times (usually 11-3 Mountain View time). Email " - "infra-dev@chromium.org with any questions.") + help='List the files and reviewers for each CL that would ' + 'be created, but don\'t create branches or CLs.') + parser.add_option('--cq-dry-run', action='store_true', + help='If set, will do a cq dry run for each uploaded CL. ' + 'Please be careful when doing this; more than ~10 CLs ' + 'has the potential to overload our build ' + 'infrastructure. Try to upload these not during high ' + 'load times (usually 11-3 Mountain View time). Email ' + 'infra-dev@chromium.org with any questions.') parser.add_option('-a', '--enable-auto-submit', action='store_true', default=True, help='Sends your change to the CQ after an approval. Only ' @@ -4895,7 +4898,6 @@ def CMDpatch(parser, args): parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit', help='don\'t commit after patch applies. Rietveld only.') - group = optparse.OptionGroup( parser, 'Options for continuing work on the current issue uploaded from a ' @@ -5029,8 +5031,8 @@ def CMDtree(parser, args): @metrics.collector.collect_metrics('git cl try') def CMDtry(parser, args): - """Triggers tryjobs using either BuildBucket or CQ dry run.""" - group = optparse.OptionGroup(parser, 'Try job options') + """Triggers tryjobs using either Buildbucket or CQ dry run.""" + group = optparse.OptionGroup(parser, 'Tryjob options') group.add_option( '-b', '--bot', action='append', help=('IMPORTANT: specify ONE builder per --bot flag. Use it multiple ' @@ -5046,7 +5048,7 @@ def CMDtry(parser, args): help=('DEPRECATED, use -B. The try master where to run the builds.')) group.add_option( '-r', '--revision', - help='Revision to use for the try job; default: the revision will ' + help='Revision to use for the tryjob; default: the revision will ' 'be determined by the try recipe that builder runs, which usually ' 'defaults to HEAD of origin/master') group.add_option( @@ -5065,8 +5067,8 @@ def CMDtry(parser, args): help='Specify generic properties in the form -p key1=value1 -p ' 'key2=value2 etc. The value will be treated as ' 'json if decodable, or as string otherwise. ' - 'NOTE: using this may make your try job not usable for CQ, ' - 'which will then schedule another try job with default properties') + 'NOTE: using this may make your tryjob not usable for CQ, ' + 'which will then schedule another tryjob with default properties') group.add_option( '--buildbucket-host', default='cr-buildbucket.appspot.com', help='Host of buildbucket. The default host is %default.') @@ -5099,7 +5101,7 @@ def CMDtry(parser, args): error_message = cl.CannotTriggerTryJobReason() if error_message: - parser.error('Can\'t trigger try jobs: %s' % error_message) + parser.error('Can\'t trigger tryjobs: %s' % error_message) if options.bucket and options.master: parser.error('Only one of --bucket and --master may be used.') @@ -5134,7 +5136,7 @@ def CMDtry(parser, args): @metrics.collector.collect_metrics('git cl try-results') def CMDtry_results(parser, args): """Prints info about results for tryjobs associated with the current CL.""" - group = optparse.OptionGroup(parser, 'Try job results options') + group = optparse.OptionGroup(parser, 'Tryjob results options') group.add_option( '-p', '--patchset', type=int, help='patchset number if not current.') group.add_option( @@ -5146,7 +5148,7 @@ def CMDtry_results(parser, args): '--buildbucket-host', default='cr-buildbucket.appspot.com', help='Host of buildbucket. The default host is %default.') group.add_option( - '--json', help=('Path of JSON output file to write try job results to,' + '--json', help=('Path of JSON output file to write tryjob results to,' 'or "-" for stdout.')) parser.add_option_group(group) auth.add_auth_options(parser) @@ -5220,8 +5222,8 @@ def CMDweb(parser, args): return 1 # Redirect I/O before invoking browser to hide its output. For example, this - # allows to hide "Created new window in existing browser session." message - # from Chrome. Based on https://stackoverflow.com/a/2323563. + # allows us to hide the "Created new window in existing browser session." + # message from Chrome. Based on https://stackoverflow.com/a/2323563. saved_stdout = os.dup(1) saved_stderr = os.dup(2) os.close(1) @@ -5638,13 +5640,13 @@ def CMDformat(parser, args): return_value = 2 # Not formatted. elif opts.diff and gn_ret == 2: # TODO this should compute and print the actual diff. - print("This change has GN build file diff for " + gn_diff_file) + print('This change has GN build file diff for ' + gn_diff_file) elif gn_ret != 0: # For non-dry run cases (and non-2 return values for dry-run), a # nonzero error code indicates a failure, probably because the file # doesn't parse. - DieWithError("gn format failed on " + gn_diff_file + - "\nTry running 'gn format' on this file manually.") + DieWithError('gn format failed on ' + gn_diff_file + + '\nTry running `gn format` on this file manually.') # Skip the metrics formatting from the global presubmit hook. These files have # a separate presubmit hook that issues an error if the files need formatting, @@ -5664,13 +5666,15 @@ def CMDformat(parser, args): return return_value + def GetDirtyMetricsDirs(diff_files): xml_diff_files = [x for x in diff_files if MatchingFileType(x, ['.xml'])] metrics_xml_dirs = [ os.path.join('tools', 'metrics', 'actions'), os.path.join('tools', 'metrics', 'histograms'), os.path.join('tools', 'metrics', 'rappor'), - os.path.join('tools', 'metrics', 'ukm')] + os.path.join('tools', 'metrics', 'ukm'), + ] for xml_dir in metrics_xml_dirs: if any(file.startswith(xml_dir) for file in xml_diff_files): yield xml_dir @@ -5782,7 +5786,7 @@ class OptionParser(optparse.OptionParser): def main(argv): if sys.hexversion < 0x02060000: - print('\nYour python version %s is unsupported, please upgrade.\n' % + print('\nYour Python version %s is unsupported, please upgrade.\n' % (sys.version.split(' ', 1)[0],), file=sys.stderr) return 2 @@ -5796,14 +5800,14 @@ def main(argv): if e.code != 500: raise DieWithError( - ('AppEngine is misbehaving and returned HTTP %d, again. Keep faith ' + ('App Engine is misbehaving and returned HTTP %d, again. Keep faith ' 'and retry or visit go/isgaeup.\n%s') % (e.code, str(e))) return 0 if __name__ == '__main__': - # These affect sys.stdout so do it outside of main() to simplify mocks in - # unit testing. + # These affect sys.stdout, so do it outside of main() to simplify mocks in + # the unit tests. fix_encoding.fix_encoding() setup_color.init() with metrics.collector.print_notice_and_exit(): diff --git a/gsutil.py b/gsutil.py index 55e4cb035d..64e4b5cbc9 100755 --- a/gsutil.py +++ b/gsutil.py @@ -86,6 +86,7 @@ def temporary_directory(base): if os.path.isdir(tmpdir): shutil.rmtree(tmpdir) + def ensure_gsutil(version, target, clean): bin_dir = os.path.join(target, 'gsutil_%s' % version) gsutil_bin = os.path.join(bin_dir, 'gsutil', 'gsutil') @@ -183,5 +184,6 @@ def main(): return run_gsutil(args.force_version, args.fallback, args.target, args.args, clean=args.clean) + if __name__ == '__main__': sys.exit(main()) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 7e8895ae20..0ddbad94a3 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -5,7 +5,6 @@ """Unit tests for git_cl.py.""" -import contextlib import datetime import json import logging @@ -30,6 +29,7 @@ import git_common import git_footers import subprocess2 + def callError(code=1, cmd='', cwd='', stdout='', stderr=''): return subprocess2.CalledProcessError(code, cmd, cwd, stdout, stderr) @@ -62,7 +62,7 @@ def MakeNamedTemporaryFileMock(expected_content): class ChangelistMock(object): # A class variable so we can access it when we don't have access to the # instance that's being set. - desc = "" + desc = '' def __init__(self, **kwargs): pass def GetIssue(self): @@ -96,8 +96,8 @@ class CodereviewSettingsFileMock(object): pass # pylint: disable=no-self-use def read(self): - return ("CODE_REVIEW_SERVER: gerrit.chromium.org\n" + - "GERRIT_HOST: True\n") + return ('CODE_REVIEW_SERVER: gerrit.chromium.org\n' + + 'GERRIT_HOST: True\n') class AuthenticatorMock(object): @@ -224,7 +224,6 @@ class TestGitClBasic(unittest.TestCase): 'Cq-Do-Not-Cancel-Tryjobs: true', ]) - def test_get_bug_line_values(self): f = lambda p, bugs: list(git_cl._get_bug_line_values(p, bugs)) self.assertEqual(f('', ''), []) @@ -471,7 +470,6 @@ class TestGitClBasic(unittest.TestCase): }) - class TestParseIssueURL(unittest.TestCase): def _validate(self, parsed, issue=None, patchset=None, hostname=None, codereview=None, fail=False): @@ -493,10 +491,6 @@ class TestParseIssueURL(unittest.TestCase): self._validate(result, *args, fail=False, **kwargs) def test_gerrit(self): - def test(url, issue=None, patchset=None, hostname=None, fail=None): - self._test_ParseIssueUrl( - git_cl._GerritChangelistImpl.ParseIssueURL, - url, issue, patchset, hostname, fail) def test(url, *args, **kwargs): self._run_and_validate(git_cl._GerritChangelistImpl.ParseIssueURL, url, *args, codereview='gerrit', **kwargs) @@ -650,7 +644,7 @@ class TestGitCl(TestCase): self.mock(git_common, 'is_dirty_git_tree', lambda x: False) self.mock(git_common, 'get_or_create_merge_base', lambda *a: ( - self._mocked_call(['get_or_create_merge_base']+list(a)))) + self._mocked_call(['get_or_create_merge_base'] + list(a)))) self.mock(git_cl, 'BranchExists', lambda _: True) self.mock(git_cl, 'FindCodereviewSettingsFile', lambda: '') self.mock(git_cl, 'SaveDescriptionBackup', lambda _: @@ -722,7 +716,7 @@ class TestGitCl(TestCase): # diagnose. if expected_args != args: N = 5 - prior_calls = '\n '.join( + prior_calls = '\n '.join( '@%d: %r' % (len(self._calls_done) - N + i, c[0]) for i, c in enumerate(self._calls_done[-N:])) following_calls = '\n '.join( @@ -868,7 +862,7 @@ class TestGitCl(TestCase): 'owner': {'email': (other_cl_owner or 'owner@example.com')}, 'change_id': '123456789', 'current_revision': 'sha1_of_current_revision', - 'revisions': { 'sha1_of_current_revision': { + 'revisions': {'sha1_of_current_revision': { 'commit': {'message': fetched_description}, }}, 'status': fetched_status or 'NEW', @@ -1383,8 +1377,8 @@ class TestGitCl(TestCase): squash_mode='override_nosquash', notify=True, change_id='I123456789', - final_description= - 'desc\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789') + final_description=( + 'desc\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789')) def test_gerrit_reviewer_multiple(self): self.mock(git_cl.gerrit_util, 'GetCodeReviewTbrScore', @@ -1670,7 +1664,6 @@ class TestGitCl(TestCase): expected, 'GetHashTags(%r) == %r, expected %r' % (desc, actual, expected)) - self.assertEqual(None, git_cl.GetTargetRef('origin', None, 'master')) self.assertEqual(None, git_cl.GetTargetRef(None, 'refs/remotes/origin/master', @@ -1719,7 +1712,7 @@ class TestGitCl(TestCase): branch)) def test_patch_when_dirty(self): - # Patch when local tree is dirty + # Patch when local tree is dirty. self.mock(git_common, 'is_dirty_git_tree', lambda x: True) self.assertNotEqual(git_cl.main(['patch', '123456']), 0) @@ -1740,12 +1733,12 @@ class TestGitCl(TestCase): CERR1 if value is None else value)) if value is None: calls += [ - ((['git', 'config', 'branch.' + branch + '.merge'],), - 'refs/heads' + branch), - ((['git', 'config', 'branch.' + branch + '.remote'],), - 'origin'), - ((['git', 'config', 'remote.origin.url'],), - 'https://%s.googlesource.com/my/repo' % git_short_host), + ((['git', 'config', 'branch.' + branch + '.merge'],), + 'refs/heads' + branch), + ((['git', 'config', 'branch.' + branch + '.remote'],), + 'origin'), + ((['git', 'config', 'remote.origin.url'],), + 'https://%s.googlesource.com/my/repo' % git_short_host), ] return calls @@ -1758,7 +1751,7 @@ class TestGitCl(TestCase): self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True) if new_branch: - self.calls = [((['git', 'new-branch', 'master'],), ''),] + self.calls = [((['git', 'new-branch', 'master'],), '')] if codereview_in_url and actual_codereview == 'rietveld': self.calls += [ @@ -1914,6 +1907,7 @@ class TestGitCl(TestCase): git_cl.main(['patch', '123456']) def test_patch_gerrit_not_exists(self): + def notExists(_issue, *_, **kwargs): raise git_cl.gerrit_util.GerritError(404, '') self.mock(git_cl.gerrit_util, 'GetChangeDetail', notExists) @@ -2118,6 +2112,7 @@ class TestGitCl(TestCase): self.assertEqual(out.getvalue(), 'foobar\n') def test_SetCloseOverrideIssue(self): + def assertIssue(cl_self, *_args): self.assertEquals(cl_self.issue, 1) return 'foobar' @@ -2208,15 +2203,16 @@ class TestGitCl(TestCase): def test_archive(self): self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) - self.calls = \ - [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), - 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.gerritissue'],), '456'), - ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), - ((['git', 'config', 'branch.bar.gerritissue'],), '789'), - ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''), - ((['git', 'branch', '-D', 'foo'],), '')] + self.calls = [ + ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), + 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), + ((['git', 'config', 'branch.master.gerritissue'],), '456'), + ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), + ((['git', 'config', 'branch.bar.gerritissue'],), '789'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''), + ((['git', 'branch', '-D', 'foo'],), ''), + ] self.mock(git_cl, 'get_cl_statuses', lambda branches, fine_grained, max_processes: @@ -2228,11 +2224,12 @@ class TestGitCl(TestCase): def test_archive_current_branch_fails(self): self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) - self.calls = \ - [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), - 'refs/heads/master'), - ((['git', 'config', 'branch.master.gerritissue'],), '1'), - ((['git', 'symbolic-ref', 'HEAD'],), 'master')] + self.calls = [ + ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), + 'refs/heads/master'), + ((['git', 'config', 'branch.master.gerritissue'],), '1'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ] self.mock(git_cl, 'get_cl_statuses', lambda branches, fine_grained, max_processes: @@ -2243,13 +2240,14 @@ class TestGitCl(TestCase): def test_archive_dry_run(self): self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) - self.calls = \ - [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), - 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.gerritissue'],), '456'), - ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), - ((['git', 'config', 'branch.bar.gerritissue'],), '789'), - ((['git', 'symbolic-ref', 'HEAD'],), 'master'),] + self.calls = [ + ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), + 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), + ((['git', 'config', 'branch.master.gerritissue'],), '456'), + ((['git', 'config', 'branch.foo.gerritissue'],), CERR1), + ((['git', 'config', 'branch.bar.gerritissue'],), '789'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master') + ] self.mock(git_cl, 'get_cl_statuses', lambda branches, fine_grained, max_processes: @@ -2262,14 +2260,15 @@ class TestGitCl(TestCase): def test_archive_no_tags(self): self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) - self.calls = \ - [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), - 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), - ((['git', 'config', 'branch.master.gerritissue'],), '1'), - ((['git', 'config', 'branch.foo.gerritissue'],), '456'), - ((['git', 'config', 'branch.bar.gerritissue'],), CERR1), - ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'branch', '-D', 'foo'],), '')] + self.calls = [ + ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), + 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), + ((['git', 'config', 'branch.master.gerritissue'],), '1'), + ((['git', 'config', 'branch.foo.gerritissue'],), '456'), + ((['git', 'config', 'branch.bar.gerritissue'],), CERR1), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'branch', '-D', 'foo'],), '') + ] self.mock(git_cl, 'get_cl_statuses', lambda branches, fine_grained, max_processes: @@ -2337,7 +2336,7 @@ class TestGitCl(TestCase): def test_git_cl_try_default_cq_dry_run_gerrit(self): self.mock(git_cl.Changelist, 'GetChange', lambda _, *a: ( - self._mocked_call(['GetChange']+list(a)))) + self._mocked_call(['GetChange'] + list(a)))) self.mock(git_cl.presubmit_support, 'DoGetTryMasters', lambda *_, **__: ( self._mocked_call(['DoGetTryMasters']))) @@ -2360,7 +2359,7 @@ class TestGitCl(TestCase): 'status': 'OPEN', 'owner': {'email': 'owner@e.mail'}, 'revisions': { - 'deadbeaf': { + 'deadbeaf': { '_number': 6, }, 'beeeeeef': { @@ -2411,7 +2410,7 @@ class TestGitCl(TestCase): 'status': 'OPEN', 'owner': {'email': 'owner@e.mail'}, 'revisions': { - 'deadbeaf': { + 'deadbeaf': { '_number': 6, }, 'beeeeeef': { @@ -2663,7 +2662,7 @@ class TestGitCl(TestCase): # self.assertRegexpMatches( # sys.stdout.getvalue(), # r'Warning: Codereview server has newer patchsets \(13\)') - self.assertRegexpMatches(sys.stdout.getvalue(), 'No try jobs') + self.assertRegexpMatches(sys.stdout.getvalue(), 'No tryjobs') def test_fetch_try_jobs_some_gerrit(self): self._setup_fetch_try_jobs_gerrit({ @@ -2679,7 +2678,7 @@ class TestGitCl(TestCase): self.assertNotRegexpMatches(sys.stdout.getvalue(), 'Warning') self.assertRegexpMatches(sys.stdout.getvalue(), '^Failures:') self.assertRegexpMatches(sys.stdout.getvalue(), 'Started:') - self.assertRegexpMatches(sys.stdout.getvalue(), '2 try jobs') + self.assertRegexpMatches(sys.stdout.getvalue(), '2 tryjobs') def _mock_gerrit_changes_for_detail_cache(self): self.mock(git_cl._GerritChangelistImpl, '_GetGerritHost', lambda _: 'host') @@ -2878,10 +2877,10 @@ class TestGitCl(TestCase): 'owner': {'email': 'owner@example.com'}, 'current_revision': 'ba5eba11', 'revisions': { - 'deadbeaf': { + 'deadbeaf': { '_number': 1, }, - 'ba5eba11': { + 'ba5eba11': { '_number': 2, }, }, @@ -2912,7 +2911,7 @@ class TestGitCl(TestCase): { u'_revision_number': 2, u'author': { - u'_account_id': 148512 , + u'_account_id': 148512, u'email': u'reviewer@example.com', u'name': u'reviewer' }, @@ -2976,7 +2975,7 @@ class TestGitCl(TestCase): u'disapproval': False, u'sender': u'reviewer@example.com' } - ]),'') + ]), '') ] expected_comments_summary = [ git_cl._CommentSummary( @@ -3029,10 +3028,10 @@ class TestGitCl(TestCase): 'owner': {'email': 'owner@example.com'}, 'current_revision': 'ba5eba11', 'revisions': { - 'deadbeaf': { + 'deadbeaf': { '_number': 1, }, - 'ba5eba11': { + 'ba5eba11': { '_number': 2, }, }, @@ -3076,7 +3075,7 @@ class TestGitCl(TestCase): { u'_revision_number': 2, u'author': { - u'_account_id': 123 , + u'_account_id': 123, u'email': u'tricium@serviceaccount.com', u'name': u'reviewer' }, @@ -3123,10 +3122,12 @@ class TestGitCl(TestCase): def test_get_remote_url_with_mirror(self): original_os_path_isdir = os.path.isdir + def selective_os_path_isdir_mock(path): if path == '/cache/this-dir-exists': return self._mocked_call('os.path.isdir', path) return original_os_path_isdir(path) + self.mock(os.path, 'isdir', selective_os_path_isdir_mock) url = 'https://chromium.googlesource.com/my/repo' @@ -3148,10 +3149,12 @@ class TestGitCl(TestCase): def test_get_remote_url_non_existing_mirror(self): original_os_path_isdir = os.path.isdir + def selective_os_path_isdir_mock(path): if path == '/cache/this-dir-doesnt-exist': return self._mocked_call('os.path.isdir', path) return original_os_path_isdir(path) + self.mock(os.path, 'isdir', selective_os_path_isdir_mock) self.mock(logging, 'error', lambda fmt, *a: self._mocked_call('logging.error', fmt % a)) @@ -3173,10 +3176,12 @@ class TestGitCl(TestCase): def test_get_remote_url_misconfigured_mirror(self): original_os_path_isdir = os.path.isdir + def selective_os_path_isdir_mock(path): if path == '/cache/this-dir-exists': return self._mocked_call('os.path.isdir', path) return original_os_path_isdir(path) + self.mock(os.path, 'isdir', selective_os_path_isdir_mock) self.mock(logging, 'error', lambda *a: self._mocked_call('logging.error', *a)) @@ -3203,7 +3208,6 @@ class TestGitCl(TestCase): cl = git_cl.Changelist(codereview='gerrit', issue=1) self.assertIsNone(cl.GetRemoteUrl()) - def test_gerrit_change_identifier_with_project(self): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'),