diff --git a/gclient_scm.py b/gclient_scm.py index e94a2f462..3b1b59f7a 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -28,7 +28,7 @@ except ImportError: # For Py3 compatibility import gclient_utils import gerrit_util import git_cache -from lib import scm +import scm import shutil import subprocess2 diff --git a/gclient_utils.py b/gclient_utils.py index d16e77b99..6a1659df3 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -38,8 +38,6 @@ else: import queue import urllib.parse as urlparse -from lib import utils - # Git wrapper retries on a transient error, and some callees do retries too, # such as GitWrapper.update (doing clone). One retry attempt should be @@ -214,14 +212,19 @@ def AskForData(message): sys.exit(1) -# TODO(sokcevic): remove the usage of this def FileRead(filename, mode='rbU'): - return utils.FileRead(filename, mode) + # mode is ignored now; we always return unicode strings. + with open(filename, mode='rb') as f: + s = f.read() + try: + return s.decode('utf-8', 'replace') + except (UnicodeDecodeError, AttributeError): + return s -# TODO(sokcevic): remove the usage of this def FileWrite(filename, content, mode='w', encoding='utf-8'): - return utils.FileWrite(filename, content, mode, encoding) + with codecs.open(filename, mode=mode, encoding=encoding) as f: + f.write(content) @contextlib.contextmanager diff --git a/git_cl.py b/git_cl.py index 3fc335412..51a57085f 100755 --- a/git_cl.py +++ b/git_cl.py @@ -45,7 +45,9 @@ import metrics_utils import owners_client import owners_finder import presubmit_canned_checks +import presubmit_support import rustfmt +import scm import setup_color import split_cl import subcommand @@ -53,7 +55,7 @@ import subprocess2 import swift_format import watchlists -from lib import change, scm, utils +from lib import utils from third_party import six from six.moves import urllib @@ -3313,8 +3315,8 @@ class ChangeDescription(object): separator = [] # No need for separator if there are no gerrit_footers. prev_line = top_lines[-1] if top_lines else '' - if (not change.Change.TAG_LINE_RE.match(prev_line) - or not change.Change.TAG_LINE_RE.match(line)): + if (not presubmit_support.Change.TAG_LINE_RE.match(prev_line) or + not presubmit_support.Change.TAG_LINE_RE.match(line)): top_lines.append('') top_lines.append(line) self._description_lines = top_lines + separator + gerrit_footers diff --git a/git_migrate_default_branch.py b/git_migrate_default_branch.py index fec10fb6e..4e00d3d72 100644 --- a/git_migrate_default_branch.py +++ b/git_migrate_default_branch.py @@ -8,7 +8,7 @@ import fix_encoding import gerrit_util import git_common import metrics -from lib import scm +import scm import sys import logging from six.moves import urllib diff --git a/lib/change.py b/lib/change.py deleted file mode 100644 index a498c1df4..000000000 --- a/lib/change.py +++ /dev/null @@ -1,481 +0,0 @@ -# Copyright 2022 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import logging -import os -import re - -from . import scm, utils - -# expectation is that depot_too -import git_footers - - -def RightHandSideLinesImpl(affected_files): - """Implements RightHandSideLines for InputApi and GclChange.""" - for af in affected_files: - lines = af.ChangedContents() - for line in lines: - yield (af, line[0], line[1]) - - -class _DiffCache(object): - """Caches diffs retrieved from a particular SCM.""" - def __init__(self, upstream=None): - """Stores the upstream revision against which all diffs will be computed.""" - self._upstream = upstream - - def GetDiff(self, path, local_root): - """Get the diff for a particular path.""" - raise NotImplementedError() - - def GetOldContents(self, path, local_root): - """Get the old version for a particular path.""" - raise NotImplementedError() - - -class _GitDiffCache(_DiffCache): - """DiffCache implementation for git; gets all file diffs at once.""" - def __init__(self, upstream): - super(_GitDiffCache, self).__init__(upstream=upstream) - self._diffs_by_file = None - - def GetDiff(self, path, local_root): - # Compare against None to distinguish between None and an initialized but - # empty dictionary. - if self._diffs_by_file == None: - # Compute a single diff for all files and parse the output; should - # with git this is much faster than computing one diff for each file. - diffs = {} - - # Don't specify any filenames below, because there are command line length - # limits on some platforms and GenerateDiff would fail. - unified_diff = scm.GIT.GenerateDiff(local_root, - files=[], - full_move=True, - branch=self._upstream) - - # This regex matches the path twice, separated by a space. Note that - # filename itself may contain spaces. - file_marker = re.compile('^diff --git (?P.*) (?P=filename)$') - current_diff = [] - keep_line_endings = True - for x in unified_diff.splitlines(keep_line_endings): - match = file_marker.match(x) - if match: - # Marks the start of a new per-file section. - diffs[match.group('filename')] = current_diff = [x] - elif x.startswith('diff --git'): - raise PresubmitFailure('Unexpected diff line: %s' % x) - else: - current_diff.append(x) - - self._diffs_by_file = dict( - (utils.normpath(path), ''.join(diff)) for path, diff in diffs.items()) - - if path not in self._diffs_by_file: - # SCM didn't have any diff on this file. It could be that the file was not - # modified at all (e.g. user used --all flag in git cl presubmit). - # Intead of failing, return empty string. - # See: https://crbug.com/808346. - return '' - - return self._diffs_by_file[path] - - def GetOldContents(self, path, local_root): - return scm.GIT.GetOldContents(local_root, path, branch=self._upstream) - - -class AffectedFile(object): - """Representation of a file in a change.""" - - DIFF_CACHE = _DiffCache - - # Method could be a function - # pylint: disable=no-self-use - def __init__(self, path, action, repository_root, diff_cache): - self._path = path - self._action = action - self._local_root = repository_root - self._is_directory = None - self._cached_changed_contents = None - self._cached_new_contents = None - self._diff_cache = diff_cache - logging.debug('%s(%s)', self.__class__.__name__, self._path) - - def LocalPath(self): - """Returns the path of this file on the local disk relative to client root. - - This should be used for error messages but not for accessing files, - because presubmit checks are run with CWD=PresubmitLocalPath() (which is - often != client root). - """ - return utils.normpath(self._path) - - def AbsoluteLocalPath(self): - """Returns the absolute path of this file on the local disk. - """ - return os.path.abspath(os.path.join(self._local_root, self.LocalPath())) - - def Action(self): - """Returns the action on this opened file, e.g. A, M, D, etc.""" - return self._action - - def IsTestableFile(self): - """Returns True if the file is a text file and not a binary file. - - Deleted files are not text file.""" - raise NotImplementedError() # Implement when needed - - def IsTextFile(self): - """An alias to IsTestableFile for backwards compatibility.""" - return self.IsTestableFile() - - def OldContents(self): - """Returns an iterator over the lines in the old version of file. - - The old version is the file before any modifications in the user's - workspace, i.e. the 'left hand side'. - - Contents will be empty if the file is a directory or does not exist. - Note: The carriage returns (LF or CR) are stripped off. - """ - return self._diff_cache.GetOldContents(self.LocalPath(), - self._local_root).splitlines() - - def NewContents(self): - """Returns an iterator over the lines in the new version of file. - - The new version is the file in the user's workspace, i.e. the 'right hand - side'. - - Contents will be empty if the file is a directory or does not exist. - Note: The carriage returns (LF or CR) are stripped off. - """ - if self._cached_new_contents is None: - self._cached_new_contents = [] - try: - self._cached_new_contents = utils.FileRead(self.AbsoluteLocalPath(), - 'rU').splitlines() - except IOError: - pass # File not found? That's fine; maybe it was deleted. - except UnicodeDecodeError as e: - # log the filename since we're probably trying to read a binary - # file, and shouldn't be. - print('Error reading %s: %s' % (self.AbsoluteLocalPath(), e)) - raise - - return self._cached_new_contents[:] - - def ChangedContents(self, keeplinebreaks=False): - """Returns a list of tuples (line number, line text) of all new lines. - - This relies on the scm diff output describing each changed code section - with a line of the form - - ^@@ , , @@$ - """ - # Don't return cached results when line breaks are requested. - if not keeplinebreaks and self._cached_changed_contents is not None: - return self._cached_changed_contents[:] - result = [] - line_num = 0 - - # The keeplinebreaks parameter to splitlines must be True or else the - # CheckForWindowsLineEndings presubmit will be a NOP. - for line in self.GenerateScmDiff().splitlines(keeplinebreaks): - m = re.match(r'^@@ [0-9\,\+\-]+ \+([0-9]+)\,[0-9]+ @@', line) - if m: - line_num = int(m.groups(1)[0]) - continue - if line.startswith('+') and not line.startswith('++'): - result.append((line_num, line[1:])) - if not line.startswith('-'): - line_num += 1 - # Don't cache results with line breaks. - if keeplinebreaks: - return result - self._cached_changed_contents = result - return self._cached_changed_contents[:] - - def __str__(self): - return self.LocalPath() - - def GenerateScmDiff(self): - return self._diff_cache.GetDiff(self.LocalPath(), self._local_root) - - -class GitAffectedFile(AffectedFile): - """Representation of a file in a change out of a git checkout.""" - # Method 'NNN' is abstract in class 'NNN' but is not overridden - # pylint: disable=abstract-method - - DIFF_CACHE = _GitDiffCache - - def __init__(self, *args, **kwargs): - AffectedFile.__init__(self, *args, **kwargs) - self._server_path = None - self._is_testable_file = None - - def IsTestableFile(self): - if self._is_testable_file is None: - if self.Action() == 'D': - # A deleted file is not testable. - self._is_testable_file = False - else: - self._is_testable_file = os.path.isfile(self.AbsoluteLocalPath()) - return self._is_testable_file - - -class Change(object): - """Describe a change. - - Used directly by the presubmit scripts to query the current change being - tested. - - Instance members: - tags: Dictionary of KEY=VALUE pairs found in the change description. - self.KEY: equivalent to tags['KEY'] - """ - - _AFFECTED_FILES = AffectedFile - - # Matches key/value (or 'tag') lines in changelist descriptions. - TAG_LINE_RE = re.compile( - '^[ \t]*(?P[A-Z][A-Z_0-9]*)[ \t]*=[ \t]*(?P.*?)[ \t]*$') - scm = '' - - def __init__(self, - name, - description, - local_root, - files, - issue, - patchset, - author, - upstream=None): - if files is None: - files = [] - self._name = name - # Convert root into an absolute path. - self._local_root = os.path.abspath(local_root) - self._upstream = upstream - self.issue = issue - self.patchset = patchset - self.author_email = author - - self._full_description = '' - self.tags = {} - self._description_without_tags = '' - self.SetDescriptionText(description) - - assert all( - (isinstance(f, (list, tuple)) and len(f) == 2) for f in files), files - - diff_cache = self._AFFECTED_FILES.DIFF_CACHE(self._upstream) - self._affected_files = [ - self._AFFECTED_FILES(path, action.strip(), self._local_root, diff_cache) - for action, path in files - ] - - def UpstreamBranch(self): - """Returns the upstream branch for the change.""" - return self._upstream - - def Name(self): - """Returns the change name.""" - return self._name - - def DescriptionText(self): - """Returns the user-entered changelist description, minus tags. - - Any line in the user-provided description starting with e.g. 'FOO=' - (whitespace permitted before and around) is considered a tag line. Such - lines are stripped out of the description this function returns. - """ - return self._description_without_tags - - def FullDescriptionText(self): - """Returns the complete changelist description including tags.""" - return self._full_description - - def SetDescriptionText(self, description): - """Sets the full description text (including tags) to |description|. - - Also updates the list of tags.""" - self._full_description = description - - # From the description text, build up a dictionary of key/value pairs - # plus the description minus all key/value or 'tag' lines. - description_without_tags = [] - self.tags = {} - for line in self._full_description.splitlines(): - m = self.TAG_LINE_RE.match(line) - if m: - self.tags[m.group('key')] = m.group('value') - else: - description_without_tags.append(line) - - # Change back to text and remove whitespace at end. - self._description_without_tags = ( - '\n'.join(description_without_tags).rstrip()) - - def AddDescriptionFooter(self, key, value): - """Adds the given footer to the change description. - - Args: - key: A string with the key for the git footer. It must conform to - the git footers format (i.e. 'List-Of-Tokens') and will be case - normalized so that each token is title-cased. - value: A string with the value for the git footer. - """ - description = git_footers.add_footer(self.FullDescriptionText(), - git_footers.normalize_name(key), value) - self.SetDescriptionText(description) - - def RepositoryRoot(self): - """Returns the repository (checkout) root directory for this change, - as an absolute path. - """ - return self._local_root - - def __getattr__(self, attr): - """Return tags directly as attributes on the object.""" - if not re.match(r'^[A-Z_]*$', attr): - raise AttributeError(self, attr) - return self.tags.get(attr) - - def GitFootersFromDescription(self): - """Return the git footers present in the description. - - Returns: - footers: A dict of {footer: [values]} containing a multimap of the footers - in the change description. - """ - return git_footers.parse_footers(self.FullDescriptionText()) - - def BugsFromDescription(self): - """Returns all bugs referenced in the commit description.""" - bug_tags = ['BUG', 'FIXED'] - - tags = [] - for tag in bug_tags: - values = self.tags.get(tag) - if values: - tags += [value.strip() for value in values.split(',')] - - footers = [] - parsed = self.GitFootersFromDescription() - unsplit_footers = parsed.get('Bug', []) + parsed.get('Fixed', []) - for unsplit_footer in unsplit_footers: - footers += [b.strip() for b in unsplit_footer.split(',')] - return sorted(set(tags + footers)) - - def ReviewersFromDescription(self): - """Returns all reviewers listed in the commit description.""" - # We don't support a 'R:' git-footer for reviewers; that is in metadata. - tags = [r.strip() for r in self.tags.get('R', '').split(',') if r.strip()] - return sorted(set(tags)) - - def TBRsFromDescription(self): - """Returns all TBR reviewers listed in the commit description.""" - tags = [r.strip() for r in self.tags.get('TBR', '').split(',') if r.strip()] - # TODO(crbug.com/839208): Remove support for 'Tbr:' when TBRs are - # programmatically determined by self-CR+1s. - footers = self.GitFootersFromDescription().get('Tbr', []) - return sorted(set(tags + footers)) - - # TODO(crbug.com/753425): Delete these once we're sure they're unused. - @property - def BUG(self): - return ','.join(self.BugsFromDescription()) - - @property - def R(self): - return ','.join(self.ReviewersFromDescription()) - - @property - def TBR(self): - return ','.join(self.TBRsFromDescription()) - - def AllFiles(self, root=None): - """List all files under source control in the repo.""" - raise NotImplementedError() - - def AffectedFiles(self, include_deletes=True, file_filter=None): - """Returns a list of AffectedFile instances for all files in the change. - - Args: - include_deletes: If false, deleted files will be filtered out. - file_filter: An additional filter to apply. - - Returns: - [AffectedFile(path, action), AffectedFile(path, action)] - """ - affected = list(filter(file_filter, self._affected_files)) - - if include_deletes: - return affected - return list(filter(lambda x: x.Action() != 'D', affected)) - - def AffectedTestableFiles(self, include_deletes=None, **kwargs): - """Return a list of the existing text files in a change.""" - if include_deletes is not None: - warn('AffectedTeestableFiles(include_deletes=%s)' - ' is deprecated and ignored' % str(include_deletes), - category=DeprecationWarning, - stacklevel=2) - return list( - filter(lambda x: x.IsTestableFile(), - self.AffectedFiles(include_deletes=False, **kwargs))) - - def AffectedTextFiles(self, include_deletes=None): - """An alias to AffectedTestableFiles for backwards compatibility.""" - return self.AffectedTestableFiles(include_deletes=include_deletes) - - def LocalPaths(self): - """Convenience function.""" - return [af.LocalPath() for af in self.AffectedFiles()] - - def AbsoluteLocalPaths(self): - """Convenience function.""" - return [af.AbsoluteLocalPath() for af in self.AffectedFiles()] - - def RightHandSideLines(self): - """An iterator over all text lines in 'new' version of changed files. - - Lists lines from new or modified text files in the change. - - This is useful for doing line-by-line regex checks, like checking for - trailing whitespace. - - Yields: - a 3 tuple: - the AffectedFile instance of the current file; - integer line number (1-based); and - the contents of the line as a string. - """ - return RightHandSideLinesImpl( - x for x in self.AffectedFiles(include_deletes=False) - if x.IsTestableFile()) - - def OriginalOwnersFiles(self): - """A map from path names of affected OWNERS files to their old content.""" - def owners_file_filter(f): - return 'OWNERS' in os.path.split(f.LocalPath())[1] - - files = self.AffectedFiles(file_filter=owners_file_filter) - return {f.LocalPath(): f.OldContents() for f in files} - - -class GitChange(Change): - _AFFECTED_FILES = GitAffectedFile - scm = 'git' - - def AllFiles(self, root=None): - """List all files under source control in the repo.""" - root = root or self.RepositoryRoot() - return subprocess.check_output( - ['git', '-c', 'core.quotePath=false', 'ls-files', '--', '.'], - cwd=root).decode('utf-8', 'ignore').splitlines() diff --git a/lib/utils.py b/lib/utils.py index ad5d746fd..b7ee4cd07 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -5,7 +5,6 @@ """ A collection of commonly used functions across depot_tools. """ -import codecs import logging import os import re @@ -94,18 +93,3 @@ def ListRelevantFilesInSourceCheckout(files, root, match_re, exclude_re): logging.debug('Presubmit files: %s', ','.join(results)) return results - - -def FileRead(filename, mode='rbU'): - # mode is ignored now; we always return unicode strings. - with open(filename, mode='rb') as f: - s = f.read() - try: - return s.decode('utf-8', 'replace') - except (UnicodeDecodeError, AttributeError): - return s - - -def FileWrite(filename, content, mode='w', encoding='utf-8'): - with codecs.open(filename, mode=mode, encoding=encoding) as f: - f.write(content) diff --git a/metrics_utils.py b/metrics_utils.py index cafe4797f..e1757bc6b 100644 --- a/metrics_utils.py +++ b/metrics_utils.py @@ -7,7 +7,7 @@ from __future__ import print_function import re import os -from lib import scm +import scm import subprocess2 import sys diff --git a/presubmit_support.py b/presubmit_support.py index d3a5b58f6..fe6e819aa 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -42,16 +42,15 @@ from warnings import warn import fix_encoding import gclient_paths # Exposed through the API import gclient_utils +import git_footers import gerrit_util import owners_client import owners_finder import presubmit_canned_checks import rdb_wrapper -from lib import scm +import scm import subprocess2 as subprocess # Exposed through the API. - from lib import utils -from lib import change as libchange if sys.version_info.major == 2: # TODO(1009814): Expose urllib2 only through urllib_request and urllib_error @@ -290,6 +289,14 @@ class ThreadPool(object): return self._messages +def _RightHandSideLinesImpl(affected_files): + """Implements RightHandSideLines for InputApi and GclChange.""" + for af in affected_files: + lines = af.ChangedContents() + for line in lines: + yield (af, line[0], line[1]) + + def prompt_should_continue(prompt_string): sys.stdout.write(prompt_string) sys.stdout.flush() @@ -815,21 +822,21 @@ class InputApi(object): Yields: a 3 tuple: - the change.AffectedFile instance of the current file; + the AffectedFile instance of the current file; integer line number (1-based); and the contents of the line as a string. Note: The carriage return (LF or CR) is stripped off. """ files = self.AffectedSourceFiles(source_file_filter) - return libchange.RightHandSideLinesImpl(files) + return _RightHandSideLinesImpl(files) def ReadFile(self, file_item, mode='r'): """Reads an arbitrary file. Deny reading anything outside the repository. """ - if isinstance(file_item, libchange.AffectedFile): + if isinstance(file_item, AffectedFile): file_item = file_item.AbsoluteLocalPath() if not file_item.startswith(self.change.RepositoryRoot()): raise IOError('Access outside the repository root is denied.') @@ -897,6 +904,456 @@ class InputApi(object): return msgs +class _DiffCache(object): + """Caches diffs retrieved from a particular SCM.""" + def __init__(self, upstream=None): + """Stores the upstream revision against which all diffs will be computed.""" + self._upstream = upstream + + def GetDiff(self, path, local_root): + """Get the diff for a particular path.""" + raise NotImplementedError() + + def GetOldContents(self, path, local_root): + """Get the old version for a particular path.""" + raise NotImplementedError() + + +class _GitDiffCache(_DiffCache): + """DiffCache implementation for git; gets all file diffs at once.""" + def __init__(self, upstream): + super(_GitDiffCache, self).__init__(upstream=upstream) + self._diffs_by_file = None + + def GetDiff(self, path, local_root): + # Compare against None to distinguish between None and an initialized but + # empty dictionary. + if self._diffs_by_file == None: + # Compute a single diff for all files and parse the output; should + # with git this is much faster than computing one diff for each file. + diffs = {} + + # Don't specify any filenames below, because there are command line length + # limits on some platforms and GenerateDiff would fail. + unified_diff = scm.GIT.GenerateDiff(local_root, files=[], full_move=True, + branch=self._upstream) + + # This regex matches the path twice, separated by a space. Note that + # filename itself may contain spaces. + file_marker = re.compile('^diff --git (?P.*) (?P=filename)$') + current_diff = [] + keep_line_endings = True + for x in unified_diff.splitlines(keep_line_endings): + match = file_marker.match(x) + if match: + # Marks the start of a new per-file section. + diffs[match.group('filename')] = current_diff = [x] + elif x.startswith('diff --git'): + raise PresubmitFailure('Unexpected diff line: %s' % x) + else: + current_diff.append(x) + + self._diffs_by_file = dict( + (utils.normpath(path), ''.join(diff)) for path, diff in diffs.items()) + + if path not in self._diffs_by_file: + # SCM didn't have any diff on this file. It could be that the file was not + # modified at all (e.g. user used --all flag in git cl presubmit). + # Intead of failing, return empty string. + # See: https://crbug.com/808346. + return '' + + return self._diffs_by_file[path] + + def GetOldContents(self, path, local_root): + return scm.GIT.GetOldContents(local_root, path, branch=self._upstream) + + +class AffectedFile(object): + """Representation of a file in a change.""" + + DIFF_CACHE = _DiffCache + + # Method could be a function + # pylint: disable=no-self-use + def __init__(self, path, action, repository_root, diff_cache): + self._path = path + self._action = action + self._local_root = repository_root + self._is_directory = None + self._cached_changed_contents = None + self._cached_new_contents = None + self._diff_cache = diff_cache + logging.debug('%s(%s)', self.__class__.__name__, self._path) + + def LocalPath(self): + """Returns the path of this file on the local disk relative to client root. + + This should be used for error messages but not for accessing files, + because presubmit checks are run with CWD=PresubmitLocalPath() (which is + often != client root). + """ + return utils.normpath(self._path) + + def AbsoluteLocalPath(self): + """Returns the absolute path of this file on the local disk. + """ + return os.path.abspath(os.path.join(self._local_root, self.LocalPath())) + + def Action(self): + """Returns the action on this opened file, e.g. A, M, D, etc.""" + return self._action + + def IsTestableFile(self): + """Returns True if the file is a text file and not a binary file. + + Deleted files are not text file.""" + raise NotImplementedError() # Implement when needed + + def IsTextFile(self): + """An alias to IsTestableFile for backwards compatibility.""" + return self.IsTestableFile() + + def OldContents(self): + """Returns an iterator over the lines in the old version of file. + + The old version is the file before any modifications in the user's + workspace, i.e. the 'left hand side'. + + Contents will be empty if the file is a directory or does not exist. + Note: The carriage returns (LF or CR) are stripped off. + """ + return self._diff_cache.GetOldContents(self.LocalPath(), + self._local_root).splitlines() + + def NewContents(self): + """Returns an iterator over the lines in the new version of file. + + The new version is the file in the user's workspace, i.e. the 'right hand + side'. + + Contents will be empty if the file is a directory or does not exist. + Note: The carriage returns (LF or CR) are stripped off. + """ + if self._cached_new_contents is None: + self._cached_new_contents = [] + try: + self._cached_new_contents = gclient_utils.FileRead( + self.AbsoluteLocalPath(), 'rU').splitlines() + except IOError: + pass # File not found? That's fine; maybe it was deleted. + except UnicodeDecodeError as e: + # log the filename since we're probably trying to read a binary + # file, and shouldn't be. + print('Error reading %s: %s' % (self.AbsoluteLocalPath(), e)) + raise + + return self._cached_new_contents[:] + + def ChangedContents(self, keeplinebreaks=False): + """Returns a list of tuples (line number, line text) of all new lines. + + This relies on the scm diff output describing each changed code section + with a line of the form + + ^@@ , , @@$ + """ + # Don't return cached results when line breaks are requested. + if not keeplinebreaks and self._cached_changed_contents is not None: + return self._cached_changed_contents[:] + result = [] + line_num = 0 + + # The keeplinebreaks parameter to splitlines must be True or else the + # CheckForWindowsLineEndings presubmit will be a NOP. + for line in self.GenerateScmDiff().splitlines(keeplinebreaks): + m = re.match(r'^@@ [0-9\,\+\-]+ \+([0-9]+)\,[0-9]+ @@', line) + if m: + line_num = int(m.groups(1)[0]) + continue + if line.startswith('+') and not line.startswith('++'): + result.append((line_num, line[1:])) + if not line.startswith('-'): + line_num += 1 + # Don't cache results with line breaks. + if keeplinebreaks: + return result; + self._cached_changed_contents = result + return self._cached_changed_contents[:] + + def __str__(self): + return self.LocalPath() + + def GenerateScmDiff(self): + return self._diff_cache.GetDiff(self.LocalPath(), self._local_root) + + +class GitAffectedFile(AffectedFile): + """Representation of a file in a change out of a git checkout.""" + # Method 'NNN' is abstract in class 'NNN' but is not overridden + # pylint: disable=abstract-method + + DIFF_CACHE = _GitDiffCache + + def __init__(self, *args, **kwargs): + AffectedFile.__init__(self, *args, **kwargs) + self._server_path = None + self._is_testable_file = None + + def IsTestableFile(self): + if self._is_testable_file is None: + if self.Action() == 'D': + # A deleted file is not testable. + self._is_testable_file = False + else: + self._is_testable_file = os.path.isfile(self.AbsoluteLocalPath()) + return self._is_testable_file + + +class Change(object): + """Describe a change. + + Used directly by the presubmit scripts to query the current change being + tested. + + Instance members: + tags: Dictionary of KEY=VALUE pairs found in the change description. + self.KEY: equivalent to tags['KEY'] + """ + + _AFFECTED_FILES = AffectedFile + + # Matches key/value (or 'tag') lines in changelist descriptions. + TAG_LINE_RE = re.compile( + '^[ \t]*(?P[A-Z][A-Z_0-9]*)[ \t]*=[ \t]*(?P.*?)[ \t]*$') + scm = '' + + def __init__( + self, name, description, local_root, files, issue, patchset, author, + upstream=None): + if files is None: + files = [] + self._name = name + # Convert root into an absolute path. + self._local_root = os.path.abspath(local_root) + self._upstream = upstream + self.issue = issue + self.patchset = patchset + self.author_email = author + + self._full_description = '' + self.tags = {} + self._description_without_tags = '' + self.SetDescriptionText(description) + + assert all( + (isinstance(f, (list, tuple)) and len(f) == 2) for f in files), files + + diff_cache = self._AFFECTED_FILES.DIFF_CACHE(self._upstream) + self._affected_files = [ + self._AFFECTED_FILES(path, action.strip(), self._local_root, diff_cache) + for action, path in files + ] + + def UpstreamBranch(self): + """Returns the upstream branch for the change.""" + return self._upstream + + def Name(self): + """Returns the change name.""" + return self._name + + def DescriptionText(self): + """Returns the user-entered changelist description, minus tags. + + Any line in the user-provided description starting with e.g. 'FOO=' + (whitespace permitted before and around) is considered a tag line. Such + lines are stripped out of the description this function returns. + """ + return self._description_without_tags + + def FullDescriptionText(self): + """Returns the complete changelist description including tags.""" + return self._full_description + + def SetDescriptionText(self, description): + """Sets the full description text (including tags) to |description|. + + Also updates the list of tags.""" + self._full_description = description + + # From the description text, build up a dictionary of key/value pairs + # plus the description minus all key/value or 'tag' lines. + description_without_tags = [] + self.tags = {} + for line in self._full_description.splitlines(): + m = self.TAG_LINE_RE.match(line) + if m: + self.tags[m.group('key')] = m.group('value') + else: + description_without_tags.append(line) + + # Change back to text and remove whitespace at end. + self._description_without_tags = ( + '\n'.join(description_without_tags).rstrip()) + + def AddDescriptionFooter(self, key, value): + """Adds the given footer to the change description. + + Args: + key: A string with the key for the git footer. It must conform to + the git footers format (i.e. 'List-Of-Tokens') and will be case + normalized so that each token is title-cased. + value: A string with the value for the git footer. + """ + description = git_footers.add_footer( + self.FullDescriptionText(), git_footers.normalize_name(key), value) + self.SetDescriptionText(description) + + def RepositoryRoot(self): + """Returns the repository (checkout) root directory for this change, + as an absolute path. + """ + return self._local_root + + def __getattr__(self, attr): + """Return tags directly as attributes on the object.""" + if not re.match(r'^[A-Z_]*$', attr): + raise AttributeError(self, attr) + return self.tags.get(attr) + + def GitFootersFromDescription(self): + """Return the git footers present in the description. + + Returns: + footers: A dict of {footer: [values]} containing a multimap of the footers + in the change description. + """ + return git_footers.parse_footers(self.FullDescriptionText()) + + def BugsFromDescription(self): + """Returns all bugs referenced in the commit description.""" + bug_tags = ['BUG', 'FIXED'] + + tags = [] + for tag in bug_tags: + values = self.tags.get(tag) + if values: + tags += [value.strip() for value in values.split(',')] + + footers = [] + parsed = self.GitFootersFromDescription() + unsplit_footers = parsed.get('Bug', []) + parsed.get('Fixed', []) + for unsplit_footer in unsplit_footers: + footers += [b.strip() for b in unsplit_footer.split(',')] + return sorted(set(tags + footers)) + + def ReviewersFromDescription(self): + """Returns all reviewers listed in the commit description.""" + # We don't support a 'R:' git-footer for reviewers; that is in metadata. + tags = [r.strip() for r in self.tags.get('R', '').split(',') if r.strip()] + return sorted(set(tags)) + + def TBRsFromDescription(self): + """Returns all TBR reviewers listed in the commit description.""" + tags = [r.strip() for r in self.tags.get('TBR', '').split(',') if r.strip()] + # TODO(crbug.com/839208): Remove support for 'Tbr:' when TBRs are + # programmatically determined by self-CR+1s. + footers = self.GitFootersFromDescription().get('Tbr', []) + return sorted(set(tags + footers)) + + # TODO(crbug.com/753425): Delete these once we're sure they're unused. + @property + def BUG(self): + return ','.join(self.BugsFromDescription()) + @property + def R(self): + return ','.join(self.ReviewersFromDescription()) + @property + def TBR(self): + return ','.join(self.TBRsFromDescription()) + + def AllFiles(self, root=None): + """List all files under source control in the repo.""" + raise NotImplementedError() + + def AffectedFiles(self, include_deletes=True, file_filter=None): + """Returns a list of AffectedFile instances for all files in the change. + + Args: + include_deletes: If false, deleted files will be filtered out. + file_filter: An additional filter to apply. + + Returns: + [AffectedFile(path, action), AffectedFile(path, action)] + """ + affected = list(filter(file_filter, self._affected_files)) + + if include_deletes: + return affected + return list(filter(lambda x: x.Action() != 'D', affected)) + + def AffectedTestableFiles(self, include_deletes=None, **kwargs): + """Return a list of the existing text files in a change.""" + if include_deletes is not None: + warn('AffectedTeestableFiles(include_deletes=%s)' + ' is deprecated and ignored' % str(include_deletes), + category=DeprecationWarning, + stacklevel=2) + return list(filter( + lambda x: x.IsTestableFile(), + self.AffectedFiles(include_deletes=False, **kwargs))) + + def AffectedTextFiles(self, include_deletes=None): + """An alias to AffectedTestableFiles for backwards compatibility.""" + return self.AffectedTestableFiles(include_deletes=include_deletes) + + def LocalPaths(self): + """Convenience function.""" + return [af.LocalPath() for af in self.AffectedFiles()] + + def AbsoluteLocalPaths(self): + """Convenience function.""" + return [af.AbsoluteLocalPath() for af in self.AffectedFiles()] + + def RightHandSideLines(self): + """An iterator over all text lines in 'new' version of changed files. + + Lists lines from new or modified text files in the change. + + This is useful for doing line-by-line regex checks, like checking for + trailing whitespace. + + Yields: + a 3 tuple: + the AffectedFile instance of the current file; + integer line number (1-based); and + the contents of the line as a string. + """ + return _RightHandSideLinesImpl( + x for x in self.AffectedFiles(include_deletes=False) + if x.IsTestableFile()) + + def OriginalOwnersFiles(self): + """A map from path names of affected OWNERS files to their old content.""" + def owners_file_filter(f): + return 'OWNERS' in os.path.split(f.LocalPath())[1] + files = self.AffectedFiles(file_filter=owners_file_filter) + return {f.LocalPath(): f.OldContents() for f in files} + + +class GitChange(Change): + _AFFECTED_FILES = GitAffectedFile + scm = 'git' + + def AllFiles(self, root=None): + """List all files under source control in the repo.""" + root = root or self.RepositoryRoot() + return subprocess.check_output( + ['git', '-c', 'core.quotePath=false', 'ls-files', '--', '.'], + cwd=root).decode('utf-8', 'ignore').splitlines() + + class GetPostUploadExecuter(object): def __init__(self, change, gerrit_obj, use_python3): """ @@ -1419,8 +1876,7 @@ def _parse_change(parser, options): parser: The parser used to parse the arguments from command line. options: The arguments parsed from command line. Returns: - A change.GitChange if the change root is a git repository, or a Change - otherwise. + A GitChange if the change root is a git repository, or a Change otherwise. """ if options.files and options.all_files: parser.error(' cannot be specified when --all-files is set.') @@ -1449,8 +1905,7 @@ def _parse_change(parser, options): logging.info('Found %d file(s).', len(change_files)) - change_class = libchange.GitChange if change_scm == 'git' \ - else libchange.Change + change_class = GitChange if change_scm == 'git' else Change return change_class( options.name, options.description, diff --git a/lib/scm.py b/scm.py similarity index 92% rename from lib/scm.py rename to scm.py index 8781aa0b8..dce9ef789 100644 --- a/lib/scm.py +++ b/scm.py @@ -1,6 +1,7 @@ # Copyright (c) 2012 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. + """SCM-specific utility classes.""" import distutils.version @@ -16,8 +17,9 @@ import subprocess2 def ValidateEmail(email): - return (re.match(r"^[a-zA-Z0-9._%\-+]+@[a-zA-Z0-9._%-]+.[a-zA-Z]{2,6}$", - email) is not None) + return ( + re.match(r"^[a-zA-Z0-9._%\-+]+@[a-zA-Z0-9._%-]+.[a-zA-Z]{2,6}$", email) + is not None) def GetCasedPath(path): @@ -30,12 +32,12 @@ def GetCasedPath(path): if i == 0: # Skip drive letter. continue - subpath = '\\'.join(paths[:i + 1]) + subpath = '\\'.join(paths[:i+1]) prev = len('\\'.join(paths[:i])) # glob.glob will return the cased path for the last item only. This is why # we are calling it in a loop. Extract the data we want and put it back # into the list. - paths[i] = glob.glob(subpath + '*')[0][prev + 1:len(subpath)] + paths[i] = glob.glob(subpath + '*')[0][prev+1:len(subpath)] path = '\\'.join(paths) return path @@ -71,10 +73,11 @@ def determine_scm(root): return 'git' try: - subprocess2.check_call(['git', 'rev-parse', '--show-cdup'], - stdout=subprocess2.DEVNULL, - stderr=subprocess2.DEVNULL, - cwd=root) + subprocess2.check_call( + ['git', 'rev-parse', '--show-cdup'], + stdout=subprocess2.DEVNULL, + stderr=subprocess2.DEVNULL, + cwd=root) return 'git' except (OSError, subprocess2.CalledProcessError): return None @@ -110,11 +113,8 @@ class GIT(object): @staticmethod def Capture(args, cwd=None, strip_out=True, **kwargs): env = GIT.ApplyEnvVars(kwargs) - output = subprocess2.check_output(['git'] + args, - cwd=cwd, - stderr=subprocess2.PIPE, - env=env, - **kwargs) + output = subprocess2.check_output( + ['git'] + args, cwd=cwd, stderr=subprocess2.PIPE, env=env, **kwargs) output = output.decode('utf-8', 'replace') return output.strip() if strip_out else output @@ -144,8 +144,8 @@ class GIT(object): # these 2 branches instead diffing to upstream. m = re.match(r'^(\w)+\t(.+)$', statusline) if not m: - raise gclient_utils.Error('status currently unsupported: %s' % - statusline) + raise gclient_utils.Error( + 'status currently unsupported: %s' % statusline) # Only grab the first letter. results.append(('%s ' % m.group(1)[0], m.group(2))) return results @@ -343,10 +343,7 @@ class GIT(object): return '' @staticmethod - def GenerateDiff(cwd, - branch=None, - branch_head='HEAD', - full_move=False, + def GenerateDiff(cwd, branch=None, branch_head='HEAD', full_move=False, files=None): """Diffs against the upstream branch or optionally another branch. @@ -354,10 +351,9 @@ class GIT(object): files, usually in the prospect to apply the patch for a try job.""" if not branch: branch = GIT.GetUpstreamBranch(cwd) - command = [ - '-c', 'core.quotePath=false', 'diff', '-p', '--no-color', '--no-prefix', - '--no-ext-diff', branch + "..." + branch_head - ] + command = ['-c', 'core.quotePath=false', 'diff', + '-p', '--no-color', '--no-prefix', '--no-ext-diff', + branch + "..." + branch_head] if full_move: command.append('--no-renames') else: @@ -371,7 +367,7 @@ class GIT(object): # In the case of added files, replace /dev/null with the path to the # file being added. if diff[i].startswith('--- /dev/null'): - diff[i] = '--- %s' % diff[i + 1][4:] + diff[i] = '--- %s' % diff[i+1][4:] return ''.join(diff) @staticmethod @@ -379,10 +375,8 @@ class GIT(object): """Returns the list of modified files between two branches.""" if not branch: branch = GIT.GetUpstreamBranch(cwd) - command = [ - '-c', 'core.quotePath=false', 'diff', '--name-only', - branch + "..." + branch_head - ] + command = ['-c', 'core.quotePath=false', 'diff', + '--name-only', branch + "..." + branch_head] return GIT.Capture(command, cwd=cwd).splitlines(False) @staticmethod diff --git a/split_cl.py b/split_cl.py index 54fd4c125..433352ffa 100644 --- a/split_cl.py +++ b/split_cl.py @@ -16,7 +16,7 @@ import tempfile import gclient_utils import git_footers -from lib import scm +import scm import git_common as git diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index 01771039e..ad83db109 100755 --- a/testing_support/fake_repos.py +++ b/testing_support/fake_repos.py @@ -26,7 +26,7 @@ import time # trial_dir must be first for non-system libraries. from testing_support import trial_dir import gclient_utils -from lib import scm +import scm import subprocess2 DEFAULT_BRANCH = 'main' diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index f9d09b096..6774f96dc 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -641,7 +641,7 @@ class ManagedGitWrapperTestCaseMock(unittest.TestCase): mock.patch('sys.stdout', StringIO()).start() self.addCleanup(mock.patch.stopall) - @mock.patch('lib.scm.GIT.IsValidRevision') + @mock.patch('scm.GIT.IsValidRevision') @mock.patch('os.path.isdir', lambda _: True) def testGetUsableRevGit(self, mockIsValidRevision): # pylint: disable=no-member diff --git a/tests/gclient_transitions_smoketest.py b/tests/gclient_transitions_smoketest.py index 0806cf4b1..54f81dfe9 100644 --- a/tests/gclient_transitions_smoketest.py +++ b/tests/gclient_transitions_smoketest.py @@ -19,7 +19,7 @@ import gclient_smoketest_base ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, ROOT_DIR) -from lib import scm +import scm from testing_support import fake_repos diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index fc60ccdc8..a2ce1054e 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -44,7 +44,7 @@ import git_common import git_footers import git_new_branch import owners_client -from lib import scm +import scm import subprocess2 NETRC_FILENAME = '_netrc' if sys.platform == 'win32' else '.netrc' @@ -629,17 +629,18 @@ class TestGitCl(unittest.TestCase): mock.patch('sys.exit', side_effect=SystemExitMock).start() mock.patch('git_cl.Settings.GetRoot', return_value='').start() self.mockGit = GitMocks() - mock.patch('lib.scm.GIT.GetBranchRef', self.mockGit.GetBranchRef).start() - mock.patch('lib.scm.GIT.GetConfig', self.mockGit.GetConfig).start() - mock.patch('lib.scm.GIT.ResolveCommit', return_value='hash').start() - mock.patch('lib.scm.GIT.IsValidRevision', return_value=True).start() - mock.patch('lib.scm.GIT.SetConfig', self.mockGit.SetConfig).start() + mock.patch('scm.GIT.GetBranchRef', self.mockGit.GetBranchRef).start() + mock.patch('scm.GIT.GetConfig', self.mockGit.GetConfig).start() + mock.patch('scm.GIT.ResolveCommit', return_value='hash').start() + mock.patch('scm.GIT.IsValidRevision', return_value=True).start() + mock.patch('scm.GIT.SetConfig', self.mockGit.SetConfig).start() mock.patch( 'git_new_branch.create_new_branch', self.mockGit.NewBranch).start() - mock.patch('lib.scm.GIT.FetchUpstreamTuple', - return_value=('origin', 'refs/heads/main')).start() - mock.patch('lib.scm.GIT.CaptureStatus', - return_value=[('M', 'foo.txt')]).start() + mock.patch( + 'scm.GIT.FetchUpstreamTuple', + return_value=('origin', 'refs/heads/main')).start() + mock.patch( + 'scm.GIT.CaptureStatus', return_value=[('M', 'foo.txt')]).start() # It's important to reset settings to not have inter-tests interference. git_cl.settings = None self.addCleanup(mock.patch.stopall) @@ -1764,9 +1765,9 @@ class TestGitCl(unittest.TestCase): @mock.patch('git_cl.Changelist._GitGetBranchConfigValue') @mock.patch('git_cl.Changelist.FetchUpstreamTuple') @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream') - @mock.patch('lib.scm.GIT.GetBranchRef') + @mock.patch('scm.GIT.GetBranchRef') @mock.patch('git_cl.Changelist.GetRemoteBranch') - @mock.patch('lib.scm.GIT.IsAncestor') + @mock.patch('scm.GIT.IsAncestor') @mock.patch('gclient_utils.AskForData') def test_upload_all_precheck_long_chain( self, mockAskForData, mockIsAncestor, mockGetRemoteBranch, @@ -1845,9 +1846,9 @@ class TestGitCl(unittest.TestCase): @mock.patch('git_cl.Changelist._GitGetBranchConfigValue') @mock.patch('git_cl.Changelist.FetchUpstreamTuple') @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream') - @mock.patch('lib.scm.GIT.GetBranchRef') + @mock.patch('scm.GIT.GetBranchRef') @mock.patch('git_cl.Changelist.GetRemoteBranch') - @mock.patch('lib.scm.GIT.IsAncestor') + @mock.patch('scm.GIT.IsAncestor') @mock.patch('gclient_utils.AskForData') def test_upload_all_precheck_options_must_upload( self, mockAskForData, mockIsAncestor, mockGetRemoteBranch, @@ -1918,8 +1919,8 @@ class TestGitCl(unittest.TestCase): @mock.patch('git_cl.Changelist._GitGetBranchConfigValue') @mock.patch('git_cl.Changelist.FetchUpstreamTuple') @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream') - @mock.patch('lib.scm.GIT.GetBranchRef') - @mock.patch('lib.scm.GIT.IsAncestor') + @mock.patch('scm.GIT.GetBranchRef') + @mock.patch('scm.GIT.IsAncestor') @mock.patch('gclient_utils.AskForData') def test_upload_all_precheck_must_rebase( self, mockAskForData, mockIsAncestor, mockGetBranchRef, @@ -1956,9 +1957,9 @@ class TestGitCl(unittest.TestCase): @mock.patch('git_cl.Changelist._GitGetBranchConfigValue') @mock.patch('git_cl.Changelist.FetchUpstreamTuple') @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream') - @mock.patch('lib.scm.GIT.GetBranchRef') + @mock.patch('scm.GIT.GetBranchRef') @mock.patch('git_cl.Changelist.GetRemoteBranch') - @mock.patch('lib.scm.GIT.IsAncestor') + @mock.patch('scm.GIT.IsAncestor') @mock.patch('gclient_utils.AskForData') def test_upload_all_precheck_hit_main(self, mockAskForData, mockIsAncestor, mockGetRemoteBranch, mockGetBranchRef, @@ -2051,9 +2052,9 @@ class TestGitCl(unittest.TestCase): @mock.patch('git_cl.Changelist._GitGetBranchConfigValue') @mock.patch('git_cl.Changelist.FetchUpstreamTuple') @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream') - @mock.patch('lib.scm.GIT.GetBranchRef') + @mock.patch('scm.GIT.GetBranchRef') @mock.patch('git_cl.Changelist.GetRemoteBranch') - @mock.patch('lib.scm.GIT.IsAncestor') + @mock.patch('scm.GIT.IsAncestor') @mock.patch('gclient_utils.AskForData') def test_upload_all_precheck_one_change( self, mockAskForData, mockIsAncestor, mockGetRemoteBranch, @@ -2097,7 +2098,7 @@ class TestGitCl(unittest.TestCase): with self.assertRaises(SystemExitMock): git_cl._UploadAllPrecheck(options, orig_args) - @mock.patch('lib.scm.GIT.GetBranchRef', return_value=None) + @mock.patch('scm.GIT.GetBranchRef', return_value=None) def test_upload_all_precheck_detached_HEAD(self, mockGetBranchRef): with self.assertRaises(SystemExitMock): @@ -2446,7 +2447,7 @@ class TestGitCl(unittest.TestCase): scm.GIT.GetBranchConfig('', branch, 'gerritserver')) def _patch_common(self, git_short_host='chromium'): - mock.patch('lib.scm.GIT.ResolveCommit', return_value='deadbeef').start() + mock.patch('scm.GIT.ResolveCommit', return_value='deadbeef').start() self.mockGit.config['remote.origin.url'] = ( 'https://%s.googlesource.com/my/repo' % git_short_host) gerrit_util.GetChangeDetail.return_value = { @@ -3642,8 +3643,8 @@ class ChangelistTest(unittest.TestCase): self.addCleanup(mock.patch.stopall) self.temp_count = 0 self.mockGit = GitMocks() - mock.patch('lib.scm.GIT.GetConfig', self.mockGit.GetConfig).start() - mock.patch('lib.scm.GIT.SetConfig', self.mockGit.SetConfig).start() + mock.patch('scm.GIT.GetConfig', self.mockGit.GetConfig).start() + mock.patch('scm.GIT.SetConfig', self.mockGit.SetConfig).start() def testRunHook(self): expected_results = { @@ -5203,7 +5204,7 @@ class CMDStatusTestCase(CMDTestCaseBase): @mock.patch('git_cl.get_cl_statuses', _mock_get_cl_statuses) @mock.patch('git_cl.Settings.GetRoot', return_value='') @mock.patch('git_cl.Settings.IsStatusCommitOrderByDate', return_value=False) - @mock.patch('lib.scm.GIT.GetBranch', return_value='a') + @mock.patch('scm.GIT.GetBranch', return_value='a') def testStatus(self, *_mocks): self.assertEqual(0, git_cl.main(['status', '--no-branch-color'])) self.maxDiff = None @@ -5227,7 +5228,7 @@ class CMDStatusTestCase(CMDTestCaseBase): @mock.patch('git_cl.get_cl_statuses', _mock_get_cl_statuses) @mock.patch('git_cl.Settings.GetRoot', return_value='') @mock.patch('git_cl.Settings.IsStatusCommitOrderByDate', return_value=False) - @mock.patch('lib.scm.GIT.GetBranch', return_value='a') + @mock.patch('scm.GIT.GetBranch', return_value='a') def testStatusByDate(self, *_mocks): self.assertEqual( 0, git_cl.main(['status', '--no-branch-color', '--date-order'])) @@ -5252,7 +5253,7 @@ class CMDStatusTestCase(CMDTestCaseBase): @mock.patch('git_cl.get_cl_statuses', _mock_get_cl_statuses) @mock.patch('git_cl.Settings.GetRoot', return_value='') @mock.patch('git_cl.Settings.IsStatusCommitOrderByDate', return_value=True) - @mock.patch('lib.scm.GIT.GetBranch', return_value='a') + @mock.patch('scm.GIT.GetBranch', return_value='a') def testStatusByDate(self, *_mocks): self.assertEqual( 0, git_cl.main(['status', '--no-branch-color'])) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 573fd4c9e..5a1ee9dfb 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -45,10 +45,9 @@ import json import owners_client import presubmit_support as presubmit import rdb_wrapper -from lib import scm +import scm import subprocess2 as subprocess from lib import utils -from lib import change as libchange # Shortcut. @@ -184,7 +183,7 @@ index fe3de7b..54ae6e1 100755 self.fake_change = FakeChange(self) self.rdb_client = mock.MagicMock() - mock.patch('lib.utils.FileRead').start() + mock.patch('gclient_utils.FileRead').start() mock.patch('gclient_utils.FileWrite').start() mock.patch('json.load').start() mock.patch('multiprocessing.cpu_count', lambda: 2) @@ -202,8 +201,8 @@ index fe3de7b..54ae6e1 100755 mock.patch('presubmit_support.time_time', return_value=0).start() mock.patch('presubmit_support.warn').start() mock.patch('random.randint').start() - mock.patch('lib.scm.GIT.GenerateDiff').start() - mock.patch('lib.scm.determine_scm').start() + mock.patch('scm.GIT.GenerateDiff').start() + mock.patch('scm.determine_scm').start() mock.patch('subprocess2.Popen').start() mock.patch('sys.stderr', StringIO()).start() mock.patch('sys.stdout', StringIO()).start() @@ -233,7 +232,7 @@ class PresubmitUnittest(PresubmitTestsBase): self.assertEqual(canned.CheckOwners, orig) def testTagLineRe(self): - m = libchange.Change.TAG_LINE_RE.match(' BUG =1223, 1445 \t') + m = presubmit.Change.TAG_LINE_RE.match(' BUG =1223, 1445 \t') self.assertIsNotNone(m) self.assertEqual(m.group('key'), 'BUG') self.assertEqual(m.group('value'), '1223, 1445') @@ -348,14 +347,15 @@ class PresubmitUnittest(PresubmitTestsBase): scm.GIT.GenerateDiff.return_value = '\n'.join(unified_diff) - change = libchange.GitChange('mychange', - '\n'.join(description_lines), - self.fake_root_dir, - files, - 0, - 0, - None, - upstream=None) + change = presubmit.GitChange( + 'mychange', + '\n'.join(description_lines), + self.fake_root_dir, + files, + 0, + 0, + None, + upstream=None) self.assertIsNotNone(change.Name() == 'mychange') self.assertIsNotNone(change.DescriptionText() == 'Hello there\nthis is a change\nand some more regular text') @@ -411,8 +411,14 @@ class PresubmitUnittest(PresubmitTestsBase): def testInvalidChange(self): with self.assertRaises(AssertionError): - libchange.GitChange('mychange', 'description', self.fake_root_dir, - ['foo/blat.cc', 'bar'], 0, 0, None) + presubmit.GitChange( + 'mychange', + 'description', + self.fake_root_dir, + ['foo/blat.cc', 'bar'], + 0, + 0, + None) def testExecPresubmitScript(self): description_lines = ('Hello there', @@ -423,8 +429,14 @@ class PresubmitUnittest(PresubmitTestsBase): ] fake_presubmit = os.path.join(self.fake_root_dir, 'PRESUBMIT.py') - change = libchange.Change('mychange', '\n'.join(description_lines), - self.fake_root_dir, files, 0, 0, None) + change = presubmit.Change( + 'mychange', + '\n'.join(description_lines), + self.fake_root_dir, + files, + 0, + 0, + None) executer = presubmit.PresubmitExecuter( change, False, None, presubmit.GerritAccessor()) self.assertFalse(executer.ExecPresubmitScript('', fake_presubmit)) @@ -472,7 +484,7 @@ class PresubmitUnittest(PresubmitTestsBase): description_lines = ('Hello there', 'this is a change', 'BUG=123') files = [['A', 'foo\\blat.cc']] fake_presubmit = os.path.join(self.fake_root_dir, 'PRESUBMIT.py') - change = libchange.Change('mychange', '\n'.join(description_lines), + change = presubmit.Change('mychange', '\n'.join(description_lines), self.fake_root_dir, files, 0, 0, None) executer = presubmit.PresubmitExecuter( change, True, None, presubmit.GerritAccessor()) @@ -572,7 +584,7 @@ class PresubmitUnittest(PresubmitTestsBase): def testDoPostUploadExecuter(self): os.path.isfile.side_effect = lambda f: 'PRESUBMIT.py' in f os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text change = self.ExampleChange() self.assertEqual( 0, @@ -591,7 +603,7 @@ class PresubmitUnittest(PresubmitTestsBase): path = os.path.join(self.fake_root_dir, 'PRESUBMIT.py') os.path.isfile.side_effect = lambda f: f == path os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text change = self.ExampleChange(extra_lines=['PROMPT_WARNING=yes']) self.assertEqual( 0, @@ -609,7 +621,7 @@ class PresubmitUnittest(PresubmitTestsBase): path = os.path.join(self.fake_root_dir, 'PRESUBMIT.py') os.path.isfile.side_effect = lambda f: f == path os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text change = self.ExampleChange(extra_lines=['ERROR=yes']) self.assertEqual( 1, @@ -637,7 +649,7 @@ class PresubmitUnittest(PresubmitTestsBase): os.path.isfile.side_effect = lambda f: f in [root_path, haspresubmit_path] os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text # Make a change which will have no warnings. change = self.ExampleChange(extra_lines=['STORY=http://tracker/123']) @@ -746,7 +758,7 @@ def CheckChangeOnCommit(input_api, output_api): os.listdir.return_value = ['PRESUBMIT.py'] random.randint.return_value = 1 - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text # Make a change with a single warning. change = self.ExampleChange(extra_lines=['PROMPT_WARNING=yes']) @@ -781,7 +793,7 @@ def CheckChangeOnCommit(input_api, output_api): os.path.isfile.side_effect = ( lambda f: f in [presubmit_path, haspresubmit_path]) os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text random.randint.return_value = 1 change = self.ExampleChange(extra_lines=['PROMPT_WARNING=yes']) @@ -806,7 +818,7 @@ def CheckChangeOnCommit(input_api, output_api): os.path.isfile.side_effect = ( lambda f: f in [presubmit_path, haspresubmit_path]) os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = self.presubmit_text + gclient_utils.FileRead.return_value = self.presubmit_text random.randint.return_value = 1 change = self.ExampleChange(extra_lines=['ERROR=yes']) @@ -864,13 +876,14 @@ def CheckChangeOnCommit(input_api, output_api): files = [ ['A', os.path.join('haspresubmit', 'blat.cc')], ] - return libchange.Change(name='mychange', - description='\n'.join(description_lines), - local_root=self.fake_root_dir, - files=files, - issue=0, - patchset=0, - author=None) + return presubmit.Change( + name='mychange', + description='\n'.join(description_lines), + local_root=self.fake_root_dir, + files=files, + issue=0, + patchset=0, + author=None) def testMergeMasters(self): merge = presubmit._MergeMasters @@ -898,7 +911,7 @@ def CheckChangeOnCommit(input_api, output_api): def testMainPostUpload(self): os.path.isfile.side_effect = lambda f: 'PRESUBMIT.py' in f os.listdir.return_value = ['PRESUBMIT.py'] - utils.FileRead.return_value = ( + gclient_utils.FileRead.return_value = ( 'USE_PYTHON3 = ' + str(sys.version_info.major == 3) + '\n' 'def PostUploadHook(gerrit, change, output_api):\n' ' return ()\n') @@ -911,7 +924,7 @@ def CheckChangeOnCommit(input_api, output_api): @mock.patch('lib.utils.ListRelevantFilesInSourceCheckout') def testMainUnversioned(self, *_mocks): - utils.FileRead.return_value = '' + gclient_utils.FileRead.return_value = '' scm.determine_scm.return_value = None utils.ListRelevantFilesInSourceCheckout.return_value = [ os.path.join(self.fake_root_dir, 'PRESUBMIT.py') @@ -923,7 +936,7 @@ def CheckChangeOnCommit(input_api, output_api): @mock.patch('lib.utils.ListRelevantFilesInSourceCheckout') def testMainUnversionedChecksFail(self, *_mocks): - utils.FileRead.return_value = ( + gclient_utils.FileRead.return_value = ( 'USE_PYTHON3 = ' + str(sys.version_info.major == 3) + '\n' 'def CheckChangeOnUpload(input_api, output_api):\n' ' return [output_api.PresubmitError("!!")]\n') @@ -949,22 +962,23 @@ def CheckChangeOnCommit(input_api, output_api): 'presubmit_unittest.py: error: is not optional for unversioned ' 'directories.\n') - @mock.patch('lib.change.Change', mock.Mock()) + @mock.patch('presubmit_support.Change', mock.Mock()) def testParseChange_Files(self): presubmit._parse_files.return_value=[('M', 'random_file.txt')] scm.determine_scm.return_value = None options = mock.Mock(all_files=False, source_controlled_only = False) change = presubmit._parse_change(None, options) - self.assertEqual(libchange.Change.return_value, change) - libchange.Change.assert_called_once_with(options.name, - options.description, - options.root, - [('M', 'random_file.txt')], - options.issue, - options.patchset, - options.author, - upstream=options.upstream) + self.assertEqual(presubmit.Change.return_value, change) + presubmit.Change.assert_called_once_with( + options.name, + options.description, + options.root, + [('M', 'random_file.txt')], + options.issue, + options.patchset, + options.author, + upstream=options.upstream) presubmit._parse_files.assert_called_once_with( options.files, options.recursive) @@ -990,63 +1004,65 @@ def CheckChangeOnCommit(input_api, output_api): parser.error.assert_called_once_with( ' cannot be specified when --all-files is set.') - @mock.patch('lib.change.GitChange', mock.Mock()) + @mock.patch('presubmit_support.GitChange', mock.Mock()) def testParseChange_FilesAndGit(self): scm.determine_scm.return_value = 'git' presubmit._parse_files.return_value = [('M', 'random_file.txt')] options = mock.Mock(all_files=False, source_controlled_only = False) change = presubmit._parse_change(None, options) - self.assertEqual(libchange.GitChange.return_value, change) - libchange.GitChange.assert_called_once_with(options.name, - options.description, - options.root, - [('M', 'random_file.txt')], - options.issue, - options.patchset, - options.author, - upstream=options.upstream) + self.assertEqual(presubmit.GitChange.return_value, change) + presubmit.GitChange.assert_called_once_with( + options.name, + options.description, + options.root, + [('M', 'random_file.txt')], + options.issue, + options.patchset, + options.author, + upstream=options.upstream) presubmit._parse_files.assert_called_once_with( options.files, options.recursive) - @mock.patch('lib.change.GitChange', mock.Mock()) - @mock.patch('lib.scm.GIT.CaptureStatus', mock.Mock()) + @mock.patch('presubmit_support.GitChange', mock.Mock()) + @mock.patch('scm.GIT.CaptureStatus', mock.Mock()) def testParseChange_NoFilesAndGit(self): scm.determine_scm.return_value = 'git' scm.GIT.CaptureStatus.return_value = [('A', 'added.txt')] options = mock.Mock(all_files=False, files=[]) change = presubmit._parse_change(None, options) - self.assertEqual(libchange.GitChange.return_value, change) - libchange.GitChange.assert_called_once_with(options.name, - options.description, - options.root, - [('A', 'added.txt')], - options.issue, - options.patchset, - options.author, - upstream=options.upstream) + self.assertEqual(presubmit.GitChange.return_value, change) + presubmit.GitChange.assert_called_once_with( + options.name, + options.description, + options.root, + [('A', 'added.txt')], + options.issue, + options.patchset, + options.author, + upstream=options.upstream) scm.GIT.CaptureStatus.assert_called_once_with( options.root, options.upstream) - @mock.patch('lib.change.GitChange', mock.Mock()) - @mock.patch('lib.scm.GIT.GetAllFiles', mock.Mock()) + @mock.patch('presubmit_support.GitChange', mock.Mock()) + @mock.patch('scm.GIT.GetAllFiles', mock.Mock()) def testParseChange_AllFilesAndGit(self): scm.determine_scm.return_value = 'git' scm.GIT.GetAllFiles.return_value = ['foo.txt', 'bar.txt'] options = mock.Mock(all_files=True, files=[]) change = presubmit._parse_change(None, options) - self.assertEqual(libchange.GitChange.return_value, change) - libchange.GitChange.assert_called_once_with(options.name, - options.description, - options.root, - [('M', 'foo.txt'), - ('M', 'bar.txt')], - options.issue, - options.patchset, - options.author, - upstream=options.upstream) + self.assertEqual(presubmit.GitChange.return_value, change) + presubmit.GitChange.assert_called_once_with( + options.name, + options.description, + options.root, + [('M', 'foo.txt'), ('M', 'bar.txt')], + options.issue, + options.patchset, + options.author, + upstream=options.upstream) scm.GIT.GetAllFiles.assert_called_once_with(options.root) def testParseGerritOptions_NoGerritUrl(self): @@ -1182,9 +1198,14 @@ class InputApiUnittest(PresubmitTestsBase): os.path.isfile.side_effect = lambda f: f in known_files presubmit.scm.GIT.GenerateDiff.return_value = '\n'.join(diffs) - change = libchange.GitChange('mychange', '\n'.join(description_lines), - self.fake_root_dir, - [[f[0], f[1]] for f in files], 0, 0, None) + change = presubmit.GitChange( + 'mychange', + '\n'.join(description_lines), + self.fake_root_dir, + [[f[0], f[1]] for f in files], + 0, + 0, + None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'), @@ -1230,9 +1251,14 @@ class InputApiUnittest(PresubmitTestsBase): for _, f in files] os.path.isfile.side_effect = lambda f: f in known_files - change = libchange.GitChange('mychange', 'description\nlines\n', - self.fake_root_dir, - [[f[0], f[1]] for f in files], 0, 0, None) + change = presubmit.GitChange( + 'mychange', + 'description\nlines\n', + self.fake_root_dir, + [[f[0], f[1]] for f in files], + 0, + 0, + None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'), @@ -1249,8 +1275,7 @@ class InputApiUnittest(PresubmitTestsBase): def testDefaultFilesToCheckFilesToSkipFilters(self): def f(x): - return libchange.AffectedFile(x, 'M', self.fake_root_dir, None) - + return presubmit.AffectedFile(x, 'M', self.fake_root_dir, None) files = [ ( [ @@ -1355,8 +1380,8 @@ class InputApiUnittest(PresubmitTestsBase): for _, item in files] os.path.isfile.side_effect = lambda f: f in known_files - change = libchange.GitChange('mychange', '', self.fake_root_dir, files, 0, - 0, None) + change = presubmit.GitChange( + 'mychange', '', self.fake_root_dir, files, 0, 0, None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, 'PRESUBMIT.py'), @@ -1375,8 +1400,8 @@ class InputApiUnittest(PresubmitTestsBase): for _, item in files] os.path.isfile.side_effect = lambda f: f in known_files - change = libchange.GitChange('mychange', '', self.fake_root_dir, files, 0, - 0, None) + change = presubmit.GitChange( + 'mychange', '', self.fake_root_dir, files, 0, 0, None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, 'PRESUBMIT.py'), False, None, False) @@ -1401,8 +1426,8 @@ class InputApiUnittest(PresubmitTestsBase): ['M', os.path.join('elsewhere', 'ouf.cc')], ] - change = libchange.Change('mychange', '', self.fake_root_dir, files, 0, 0, - None) + change = presubmit.Change( + 'mychange', '', self.fake_root_dir, files, 0, 0, None) affected_files = change.AffectedFiles() # Validate that normpath strips trailing path separators. self.assertEqual('isdir', normpath('isdir/')) @@ -1436,8 +1461,8 @@ class InputApiUnittest(PresubmitTestsBase): utils.normpath(os.path.join(self.fake_root_dir, 'isdir', 'blat.cc'))) def testDeprecated(self): - change = libchange.Change('mychange', '', self.fake_root_dir, [], 0, 0, - None) + change = presubmit.Change( + 'mychange', '', self.fake_root_dir, [], 0, 0, None) api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'), True, @@ -1446,8 +1471,8 @@ class InputApiUnittest(PresubmitTestsBase): def testReadFileStringDenied(self): - change = libchange.Change('foo', 'foo', self.fake_root_dir, [('M', 'AA')], - 0, 0, None) + change = presubmit.Change( + 'foo', 'foo', self.fake_root_dir, [('M', 'AA')], 0, 0, None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, '/p'), False, None, False) @@ -1457,35 +1482,31 @@ class InputApiUnittest(PresubmitTestsBase): path = os.path.join(self.fake_root_dir, 'AA/boo') presubmit.gclient_utils.FileRead.return_code = None - change = libchange.Change('foo', 'foo', self.fake_root_dir, [('M', 'AA')], - 0, 0, None) + change = presubmit.Change( + 'foo', 'foo', self.fake_root_dir, [('M', 'AA')], 0, 0, None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, '/p'), False, None, False) input_api.ReadFile(path, 'x') def testReadFileAffectedFileDenied(self): - fileobj = libchange.AffectedFile('boo', - 'M', - 'Unrelated', + fileobj = presubmit.AffectedFile('boo', 'M', 'Unrelated', diff_cache=mock.Mock()) - change = libchange.Change('foo', 'foo', self.fake_root_dir, [('M', 'AA')], - 0, 0, None) + change = presubmit.Change( + 'foo', 'foo', self.fake_root_dir, [('M', 'AA')], 0, 0, None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, '/p'), False, None, False) self.assertRaises(IOError, input_api.ReadFile, fileobj, 'x') def testReadFileAffectedFileAccepted(self): - fileobj = libchange.AffectedFile('AA/boo', - 'M', - self.fake_root_dir, + fileobj = presubmit.AffectedFile('AA/boo', 'M', self.fake_root_dir, diff_cache=mock.Mock()) presubmit.gclient_utils.FileRead.return_code = None - change = libchange.Change('foo', 'foo', self.fake_root_dir, [('M', 'AA')], - 0, 0, None) + change = presubmit.Change( + 'foo', 'foo', self.fake_root_dir, [('M', 'AA')], 0, 0, None) input_api = presubmit.InputApi( change, os.path.join(self.fake_root_dir, '/p'), False, None, False) @@ -1560,25 +1581,25 @@ class OutputApiUnittest(PresubmitTestsBase): class AffectedFileUnittest(PresubmitTestsBase): def testAffectedFile(self): - utils.FileRead.return_value = 'whatever\ncookie' - af = libchange.GitAffectedFile('foo/blat.cc', 'M', self.fake_root_dir, None) + gclient_utils.FileRead.return_value = 'whatever\ncookie' + af = presubmit.GitAffectedFile('foo/blat.cc', 'M', self.fake_root_dir, None) self.assertEqual(utils.normpath('foo/blat.cc'), af.LocalPath()) self.assertEqual('M', af.Action()) self.assertEqual(['whatever', 'cookie'], af.NewContents()) def testAffectedFileNotExists(self): notfound = 'notfound.cc' - utils.FileRead.side_effect = IOError - af = libchange.AffectedFile(notfound, 'A', self.fake_root_dir, None) + gclient_utils.FileRead.side_effect = IOError + af = presubmit.AffectedFile(notfound, 'A', self.fake_root_dir, None) self.assertEqual([], af.NewContents()) def testIsTestableFile(self): files = [ - libchange.GitAffectedFile('foo/blat.txt', 'M', self.fake_root_dir, + presubmit.GitAffectedFile('foo/blat.txt', 'M', self.fake_root_dir, None), - libchange.GitAffectedFile('foo/binary.blob', 'M', self.fake_root_dir, + presubmit.GitAffectedFile('foo/binary.blob', 'M', self.fake_root_dir, None), - libchange.GitAffectedFile('blat/flop.txt', 'D', self.fake_root_dir, + presubmit.GitAffectedFile('blat/flop.txt', 'D', self.fake_root_dir, None) ] blat = os.path.join('foo', 'blat.txt') @@ -1594,14 +1615,14 @@ class AffectedFileUnittest(PresubmitTestsBase): class ChangeUnittest(PresubmitTestsBase): def testAffectedFiles(self): - change = libchange.Change('', '', self.fake_root_dir, [('Y', 'AA')], 3, 5, - '') + change = presubmit.Change( + '', '', self.fake_root_dir, [('Y', 'AA')], 3, 5, '') self.assertEqual(1, len(change.AffectedFiles())) self.assertEqual('Y', change.AffectedFiles()[0].Action()) def testSetDescriptionText(self): - change = libchange.Change('', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, - '') + change = presubmit.Change( + '', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, '') self.assertEqual('foo', change.DescriptionText()) self.assertEqual('foo\nDRU=ro', change.FullDescriptionText()) self.assertEqual({'DRU': 'ro'}, change.tags) @@ -1612,28 +1633,28 @@ class ChangeUnittest(PresubmitTestsBase): self.assertEqual({'WHIZ': 'bang', 'FOO': 'baz'}, change.tags) def testAddDescriptionFooter(self): - change = libchange.Change('', 'foo\nDRU=ro\n\nChange-Id: asdf', - self.fake_root_dir, [], 3, 5, '') + change = presubmit.Change( + '', 'foo\nDRU=ro\n\nChange-Id: asdf', self.fake_root_dir, [], 3, 5, '') change.AddDescriptionFooter('my-footer', 'my-value') self.assertEqual( 'foo\nDRU=ro\n\nChange-Id: asdf\nMy-Footer: my-value', change.FullDescriptionText()) def testAddDescriptionFooter_NoPreviousFooters(self): - change = libchange.Change('', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, - '') + change = presubmit.Change( + '', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, '') change.AddDescriptionFooter('my-footer', 'my-value') self.assertEqual( 'foo\nDRU=ro\n\nMy-Footer: my-value', change.FullDescriptionText()) def testAddDescriptionFooter_InvalidFooter(self): - change = libchange.Change('', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, - '') + change = presubmit.Change( + '', 'foo\nDRU=ro', self.fake_root_dir, [], 3, 5, '') with self.assertRaises(ValueError): change.AddDescriptionFooter('invalid.characters in:the', 'footer key') def testGitFootersFromDescription(self): - change = libchange.Change( + change = presubmit.Change( '', 'foo\n\nChange-Id: asdf\nBug: 1\nBug: 2\nNo-Try: True', self.fake_root_dir, [], 0, 0, '') self.assertEqual({ @@ -1643,45 +1664,48 @@ class ChangeUnittest(PresubmitTestsBase): }, change.GitFootersFromDescription()) def testGitFootersFromDescription_NoFooters(self): - change = libchange.Change('', 'foo', self.fake_root_dir, [], 0, 0, '') + change = presubmit.Change('', 'foo', self.fake_root_dir, [], 0, 0, '') self.assertEqual({}, change.GitFootersFromDescription()) def testBugFromDescription_FixedAndBugGetDeduped(self): - change = libchange.Change('', - 'foo\n\nChange-Id: asdf\nBug: 1, 2\nFixed:2, 1 ', - self.fake_root_dir, [], 0, 0, '') + change = presubmit.Change( + '', 'foo\n\nChange-Id: asdf\nBug: 1, 2\nFixed:2, 1 ', + self.fake_root_dir, [], 0, 0, '') self.assertEqual(['1', '2'], change.BugsFromDescription()) self.assertEqual('1,2', change.BUG) def testBugsFromDescription_MixedTagsAndFooters(self): - change = libchange.Change('', 'foo\nBUG=2,1\n\nChange-Id: asdf\nBug: 3, 6', - self.fake_root_dir, [], 0, 0, '') + change = presubmit.Change( + '', 'foo\nBUG=2,1\n\nChange-Id: asdf\nBug: 3, 6', + self.fake_root_dir, [], 0, 0, '') self.assertEqual(['1', '2', '3', '6'], change.BugsFromDescription()) self.assertEqual('1,2,3,6', change.BUG) def testBugsFromDescription_MultipleFooters(self): - change = libchange.Change( + change = presubmit.Change( '', 'foo\n\nChange-Id: asdf\nBug: 1\nBug:4, 6\nFixed: 7', self.fake_root_dir, [], 0, 0, '') self.assertEqual(['1', '4', '6', '7'], change.BugsFromDescription()) self.assertEqual('1,4,6,7', change.BUG) def testBugFromDescription_OnlyFixed(self): - change = libchange.Change('', 'foo\n\nChange-Id: asdf\nFixed:1, 2', - self.fake_root_dir, [], 0, 0, '') + change = presubmit.Change( + '', 'foo\n\nChange-Id: asdf\nFixed:1, 2', + self.fake_root_dir, [], 0, 0, '') self.assertEqual(['1', '2'], change.BugsFromDescription()) self.assertEqual('1,2', change.BUG) def testReviewersFromDescription(self): - change = libchange.Change('', 'foo\nR=foo,bar\n\nChange-Id: asdf\nR: baz', - self.fake_root_dir, [], 0, 0, '') + change = presubmit.Change( + '', 'foo\nR=foo,bar\n\nChange-Id: asdf\nR: baz', + self.fake_root_dir, [], 0, 0, '') self.assertEqual(['bar', 'foo'], change.ReviewersFromDescription()) self.assertEqual('bar,foo', change.R) def testTBRsFromDescription(self): - change = libchange.Change('', - 'foo\nTBR=foo,bar\n\nChange-Id: asdf\nTBR: baz', - self.fake_root_dir, [], 0, 0, '') + change = presubmit.Change( + '', 'foo\nTBR=foo,bar\n\nChange-Id: asdf\nTBR: baz', + self.fake_root_dir, [], 0, 0, '') self.assertEqual(['bar', 'baz', 'foo'], change.TBRsFromDescription()) self.assertEqual('bar,baz,foo', change.TBR) @@ -1732,11 +1756,11 @@ class CannedChecksUnittest(PresubmitTestsBase): def DescriptionTest(self, check, description1, description2, error_type, committing): - change1 = libchange.Change('foo1', description1, self.fake_root_dir, None, - 0, 0, None) + change1 = presubmit.Change( + 'foo1', description1, self.fake_root_dir, None, 0, 0, None) input_api1 = self.MockInputApi(change1, committing) - change2 = libchange.Change('foo2', description2, self.fake_root_dir, None, - 0, 0, None) + change2 = presubmit.Change( + 'foo2', description2, self.fake_root_dir, None, 0, 0, None) input_api2 = self.MockInputApi(change2, committing) results1 = check(input_api1, presubmit.OutputApi) @@ -1757,10 +1781,10 @@ class CannedChecksUnittest(PresubmitTestsBase): content2_path: file path for content2. error_type: the type of the error expected for content2. """ - change1 = libchange.Change('foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, - None) + change1 = presubmit.Change( + 'foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, None) input_api1 = self.MockInputApi(change1, False) - affected_file1 = mock.MagicMock(libchange.GitAffectedFile) + affected_file1 = mock.MagicMock(presubmit.GitAffectedFile) input_api1.AffectedFiles.return_value = [affected_file1] affected_file1.LocalPath.return_value = content1_path affected_file1.NewContents.return_value = [ @@ -1777,11 +1801,11 @@ class CannedChecksUnittest(PresubmitTestsBase): (43, 'hfoo'), (23, 'ifoo')] - change2 = libchange.Change('foo2', 'foo2\n', self.fake_root_dir, None, 0, 0, - None) + change2 = presubmit.Change( + 'foo2', 'foo2\n', self.fake_root_dir, None, 0, 0, None) input_api2 = self.MockInputApi(change2, False) - affected_file2 = mock.MagicMock(libchange.GitAffectedFile) + affected_file2 = mock.MagicMock(presubmit.GitAffectedFile) input_api2.AffectedFiles.return_value = [affected_file2] affected_file2.LocalPath.return_value = content2_path affected_file2.NewContents.return_value = [ @@ -1813,10 +1837,10 @@ class CannedChecksUnittest(PresubmitTestsBase): content: Python source which is expected to pass or fail the test. should_pass: True iff the test should pass, False otherwise. """ - change = libchange.Change('foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, + change = presubmit.Change('foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) - affected_file = mock.MagicMock(libchange.GitAffectedFile) + affected_file = mock.MagicMock(presubmit.GitAffectedFile) input_api.AffectedFiles.return_value = [affected_file] affected_file.LocalPath.return_value = 'foo.py' affected_file.NewContents.return_value = content.splitlines() @@ -1831,16 +1855,16 @@ class CannedChecksUnittest(PresubmitTestsBase): presubmit.OutputApi.PresubmitPromptWarning) def ReadFileTest(self, check, content1, content2, error_type): - change1 = libchange.Change('foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, - None) + change1 = presubmit.Change( + 'foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, None) input_api1 = self.MockInputApi(change1, False) - affected_file1 = mock.MagicMock(libchange.GitAffectedFile) + affected_file1 = mock.MagicMock(presubmit.GitAffectedFile) input_api1.AffectedSourceFiles.return_value = [affected_file1] input_api1.ReadFile.return_value = content1 - change2 = libchange.Change('foo2', 'foo2\n', self.fake_root_dir, None, 0, 0, - None) + change2 = presubmit.Change( + 'foo2', 'foo2\n', self.fake_root_dir, None, 0, 0, None) input_api2 = self.MockInputApi(change2, False) - affected_file2 = mock.MagicMock(libchange.GitAffectedFile) + affected_file2 = mock.MagicMock(presubmit.GitAffectedFile) input_api2.AffectedSourceFiles.return_value = [affected_file2] input_api2.ReadFile.return_value = content2 affected_file2.LocalPath.return_value = 'bar.cc' @@ -1935,10 +1959,10 @@ class CannedChecksUnittest(PresubmitTestsBase): @mock.patch('git_cl.Changelist') @mock.patch('auth.Authenticator') def testCannedCheckChangedLUCIConfigs(self, mockGetAuth, mockChangelist): - affected_file1 = mock.MagicMock(libchange.GitAffectedFile) + affected_file1 = mock.MagicMock(presubmit.GitAffectedFile) affected_file1.LocalPath.return_value = 'foo.cfg' affected_file1.NewContents.return_value = ['test', 'foo'] - affected_file2 = mock.MagicMock(libchange.GitAffectedFile) + affected_file2 = mock.MagicMock(presubmit.GitAffectedFile) affected_file2.LocalPath.return_value = 'bar.cfg' affected_file2.NewContents.return_value = ['test', 'bar'] @@ -1957,8 +1981,8 @@ class CannedChecksUnittest(PresubmitTestsBase): mockChangelist().GetRemoteBranch.return_value = ('remote', branch) mockChangelist().GetRemoteUrl.return_value = host - change1 = libchange.Change('foo', 'foo1', self.fake_root_dir, None, 0, 0, - None) + change1 = presubmit.Change( + 'foo', 'foo1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change1, False) affected_files = (affected_file1, affected_file2) @@ -1976,17 +2000,17 @@ class CannedChecksUnittest(PresubmitTestsBase): presubmit.OutputApi.PresubmitPromptWarning) # Make sure makefiles are ignored. - change1 = libchange.Change('foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, - None) + change1 = presubmit.Change( + 'foo1', 'foo1\n', self.fake_root_dir, None, 0, 0, None) input_api1 = self.MockInputApi(change1, False) - affected_file1 = mock.MagicMock(libchange.GitAffectedFile) + affected_file1 = mock.MagicMock(presubmit.GitAffectedFile) affected_file1.LocalPath.return_value = 'foo.cc' - affected_file2 = mock.MagicMock(libchange.GitAffectedFile) + affected_file2 = mock.MagicMock(presubmit.GitAffectedFile) affected_file2.LocalPath.return_value = 'foo/Makefile' - affected_file3 = mock.MagicMock(libchange.GitAffectedFile) + affected_file3 = mock.MagicMock(presubmit.GitAffectedFile) affected_file3.LocalPath.return_value = 'makefile' # Only this one will trigger. - affected_file4 = mock.MagicMock(libchange.GitAffectedFile) + affected_file4 = mock.MagicMock(presubmit.GitAffectedFile) affected_file1.LocalPath.return_value = 'foo.cc' affected_file1.NewContents.return_value = ['yo, '] affected_file4.LocalPath.return_value = 'makefile.foo' @@ -2208,10 +2232,10 @@ the current line as well! expected_result, new_file=False, **kwargs): - change = mock.MagicMock(libchange.GitChange) + change = mock.MagicMock(presubmit.GitChange) change.scm = 'svn' input_api = self.MockInputApi(change, committing) - affected_file = mock.MagicMock(libchange.GitAffectedFile) + affected_file = mock.MagicMock(presubmit.GitAffectedFile) if new_file: affected_file.Action.return_value = 'A' input_api.AffectedSourceFiles.return_value = [affected_file] @@ -2494,12 +2518,12 @@ the current line as well! self.checkstdout('') def GetInputApiWithFiles(self, files): - change = mock.MagicMock(libchange.Change) - change.AffectedFiles = lambda *a, **kw: (libchange.Change.AffectedFiles( - change, *a, **kw)) + change = mock.MagicMock(presubmit.Change) + change.AffectedFiles = lambda *a, **kw: ( + presubmit.Change.AffectedFiles(change, *a, **kw)) change._affected_files = [] for path, (action, contents) in files.items(): - affected_file = mock.MagicMock(libchange.GitAffectedFile) + affected_file = mock.MagicMock(presubmit.GitAffectedFile) affected_file.AbsoluteLocalPath.return_value = path affected_file.LocalPath.return_value = path affected_file.Action.return_value = action @@ -2634,7 +2658,7 @@ the current line as well! "reviewers": {"REVIEWER": [{u'email': a}] for a in approvers}, } - change = mock.MagicMock(libchange.Change) + change = mock.MagicMock(presubmit.Change) change.OriginalOwnersFiles.return_value = {} change.RepositoryRoot.return_value = None change.ReviewersFromDescription.return_value = manually_specified_reviewers @@ -2644,7 +2668,7 @@ the current line as well! affected_files = [] for f in modified_files: - affected_file = mock.MagicMock(libchange.GitAffectedFile) + affected_file = mock.MagicMock(presubmit.GitAffectedFile) affected_file.LocalPath.return_value = f affected_files.append(affected_file) change.AffectedFiles.return_value = affected_files @@ -2683,8 +2707,8 @@ the current line as well! @mock.patch('io.open', mock.mock_open()) def testCannedRunUnitTests(self): io.open().readline.return_value = '' - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.PresubmitLocalPath.return_value = self.fake_root_dir @@ -2729,8 +2753,8 @@ the current line as well! @mock.patch('io.open', mock.mock_open()) def testCannedRunUnitTestsWithTimer(self): io.open().readline.return_value = '' - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.thread_pool.timeout = 100 @@ -2755,8 +2779,8 @@ the current line as well! @mock.patch('io.open', mock.mock_open()) def testCannedRunUnitTestsWithTimerTimesOut(self): io.open().readline.return_value = '' - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.thread_pool.timeout = 100 @@ -2789,8 +2813,8 @@ the current line as well! @mock.patch('io.open', mock.mock_open()) def testCannedRunUnitTestsPython3(self): io.open().readline.return_value = '#!/usr/bin/env python3' - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.PresubmitLocalPath.return_value = self.fake_root_dir @@ -2845,8 +2869,8 @@ the current line as well! @mock.patch('io.open', mock.mock_open()) def testCannedRunUnitTestsDontRunOnPython2(self): io.open().readline.return_value = '#!/usr/bin/env python3' - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.PresubmitLocalPath.return_value = self.fake_root_dir @@ -2889,8 +2913,8 @@ the current line as well! @mock.patch('io.open', mock.mock_open()) def testCannedRunUnitTestsDontRunOnPython3(self): io.open().readline.return_value = '#!/usr/bin/env python3' - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.PresubmitLocalPath.return_value = self.fake_root_dir @@ -2931,8 +2955,8 @@ the current line as well! self.checkstdout('') def testCannedRunUnitTestsInDirectory(self): - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) input_api.verbose = True input_api.logging = mock.MagicMock(logging) @@ -2964,10 +2988,10 @@ the current line as well! def testPanProjectChecks(self): # Make sure it accepts both list and tuples. - change = libchange.Change('foo1', 'description1', self.fake_root_dir, None, - 0, 0, None) + change = presubmit.Change( + 'foo1', 'description1', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) - affected_file = mock.MagicMock(libchange.GitAffectedFile) + affected_file = mock.MagicMock(presubmit.GitAffectedFile) input_api.AffectedFiles.return_value = [affected_file] affected_file.NewContents.return_value = 'Hey!\nHo!\nHey!\nHo!\n\n' # CheckChangeHasNoTabs() calls _FindNewViolationsOfRule() which calls @@ -3064,13 +3088,13 @@ the current line as well! ]) def testCannedCheckVPythonSpec(self): - change = libchange.Change('a', 'b', self.fake_root_dir, None, 0, 0, None) + change = presubmit.Change('a', 'b', self.fake_root_dir, None, 0, 0, None) input_api = self.MockInputApi(change, False) affected_filenames = ['/path1/to/.vpython', '/path1/to/.vpython3'] affected_files = [] for filename in affected_filenames: - affected_file = mock.MagicMock(libchange.GitAffectedFile) + affected_file = mock.MagicMock(presubmit.GitAffectedFile) affected_file.AbsoluteLocalPath.return_value = filename affected_files.append(affected_file) input_api.AffectedTestableFiles.return_value = affected_files diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 675a431e4..52d560d3c 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -19,7 +19,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from testing_support import fake_repos -from lib import scm +import scm import subprocess2 @@ -32,13 +32,13 @@ class GitWrapperTestCase(unittest.TestCase): super(GitWrapperTestCase, self).setUp() self.root_dir = '/foo/bar' - @mock.patch('lib.scm.GIT.Capture') + @mock.patch('scm.GIT.Capture') def testGetEmail(self, mockCapture): mockCapture.return_value = 'mini@me.com' self.assertEqual(scm.GIT.GetEmail(self.root_dir), 'mini@me.com') mockCapture.assert_called_with(['config', 'user.email'], cwd=self.root_dir) - @mock.patch('lib.scm.GIT.Capture') + @mock.patch('scm.GIT.Capture') def testAssertVersion(self, mockCapture): cases = [ ('1.7', True), @@ -99,7 +99,7 @@ class GitWrapperTestCase(unittest.TestCase): r = scm.GIT.RemoteRefToRef(k, remote) self.assertEqual(r, v, msg='%s -> %s, expected %s' % (k, r, v)) - @mock.patch('lib.scm.GIT.Capture') + @mock.patch('scm.GIT.Capture') @mock.patch('os.path.exists', lambda _:True) def testGetRemoteHeadRefLocal(self, mockCapture): mockCapture.side_effect = ['refs/remotes/origin/main'] @@ -107,7 +107,7 @@ class GitWrapperTestCase(unittest.TestCase): scm.GIT.GetRemoteHeadRef('foo', 'proto://url', 'origin')) self.assertEqual(mockCapture.call_count, 1) - @mock.patch('lib.scm.GIT.Capture') + @mock.patch('scm.GIT.Capture') @mock.patch('os.path.exists', lambda _: True) def testGetRemoteHeadRefLocalUpdateHead(self, mockCapture): mockCapture.side_effect = [ @@ -119,7 +119,7 @@ class GitWrapperTestCase(unittest.TestCase): scm.GIT.GetRemoteHeadRef('foo', 'proto://url', 'origin')) self.assertEqual(mockCapture.call_count, 3) - @mock.patch('lib.scm.GIT.Capture') + @mock.patch('scm.GIT.Capture') @mock.patch('os.path.exists', lambda _:True) def testGetRemoteHeadRefRemote(self, mockCapture): mockCapture.side_effect = [ @@ -215,12 +215,12 @@ class RealGitTest(fake_repos.FakeReposTestBase): self.assertEqual( (None, None), scm.GIT.FetchUpstreamTuple(self.cwd)) - @mock.patch('lib.scm.GIT.GetRemoteBranches', return_value=['origin/main']) + @mock.patch('scm.GIT.GetRemoteBranches', return_value=['origin/main']) def testFetchUpstreamTuple_GuessOriginMaster(self, _mockGetRemoteBranches): self.assertEqual(('origin', 'refs/heads/main'), scm.GIT.FetchUpstreamTuple(self.cwd)) - @mock.patch('lib.scm.GIT.GetRemoteBranches', + @mock.patch('scm.GIT.GetRemoteBranches', return_value=['origin/master', 'origin/main']) def testFetchUpstreamTuple_GuessOriginMain(self, _mockGetRemoteBranches): self.assertEqual(('origin', 'refs/heads/main'), @@ -235,7 +235,7 @@ class RealGitTest(fake_repos.FakeReposTestBase): scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-branch') scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-remote') - @mock.patch('lib.scm.GIT.GetBranch', side_effect=callError()) + @mock.patch('scm.GIT.GetBranch', side_effect=callError()) def testFetchUpstreamTuple_NotOnBranch(self, _mockGetBranch): scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-branch', 'rietveld-upstream') scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-remote', 'rietveld-remote')