diff --git a/scm.py b/scm.py index c348abcca..7dbca0df8 100644 --- a/scm.py +++ b/scm.py @@ -3,12 +3,16 @@ # found in the LICENSE file. """SCM-specific utility classes.""" -from collections import defaultdict +import abc import os import pathlib import platform import re -from typing import Mapping, List +import threading + +from collections import defaultdict +from typing import Iterable, Literal, Dict, List, Optional, Sequence +from typing import Tuple, Mapping import gclient_utils import git_common @@ -41,6 +45,349 @@ def determine_scm(root): return 'diff' +GitConfigScope = Literal['system', 'local', 'worktree'] +GitScopeOrder: List[GitConfigScope] = ['system', 'local', 'worktree'] +GitFlatConfigData = Mapping[str, Sequence[str]] + + +class GitConfigStateBase(metaclass=abc.ABCMeta): + """GitConfigStateBase is the abstract base class for implementations of + CachedGitConfigState. + + This is meant to model the manipulation of some underlying config data. + + In GitConfigStateReal, this is modeled using `git config` commands in + a specific git repo. + + In GitConfigStateTest, this is modeled using a set of GitConfigScope-indexed + dictionaries. + """ + + @abc.abstractmethod + def load_config(self) -> GitFlatConfigData: + """When invoked, this should return the full state of the configuration + observable. + + The caller must not mutate the returned value. + """ + + @abc.abstractmethod + def set_config(self, key: str, value: str, *, append: bool, + scope: GitConfigScope): + """When invoked, this should set `key` to a singluar `value` in the git + scope `scope` in this state's underlying data. + + If `append` is True, this should add an additional value to the existing + `key`, if any. + """ + + @abc.abstractmethod + def set_config_multi(self, key: str, value: str, *, append: bool, + value_pattern: Optional[str], scope: GitConfigScope): + """When invoked, this should replace all existing values of `key` with + `value` in the git scope `scope` in this state's underlying data. + + If `value_pattern` is supplied, only existing values matching this + pattern will be replaced. + + TODO: Make value_pattern an re.Pattern. This wasn't done at the time of + this refactor to keep the refactor small. + """ + + @abc.abstractmethod + def unset_config(self, key: str, *, scope: GitConfigScope): + """When invoked, remove a singlar value from `key` in this state's underlying data. + """ + + @abc.abstractmethod + def unset_config_multi(self, key: str, *, value_pattern: Optional[str], + scope: GitConfigScope): + """When invoked, remove all values from `key` in this state's underlying data. + + If `value_pattern` is supplied, only values matching this pattern will + be removed. + + TODO: Make value_pattern an re.Pattern. This wasn't done at the time of + this refactor to keep the refactor small. + """ + + +class CachedGitConfigState(object): + """This represents the observable git configuration state for a given + repository (whose top-level path is `root`). + + This maintains an in-memory cache of the entire, flattened, observable + configuration state according to the GitConfigStateBase implementation. + + All SetConfig operations which actually change the underlying data will + clear the internal cache. All read operations will either use the internal + cache, or repopulate it from the GitConfigStateBase implementation + on-demand. + + This design assumes no other processes are mutating git config state, which + is typically true for git_cl and other short-lived programs in depot_tools + which use scm.py. + """ + + def __init__(self, impl: GitConfigStateBase): + """Initializes a git config cache against the given underlying + GitConfigStateBase (either GitConfigStateReal or GitConfigStateTest). + """ + self._impl: GitConfigStateBase = impl + + # Actual cached configuration from the point of view of this root. + self._config: Optional[GitFlatConfigData] = None + + def _maybe_load_config(self) -> GitFlatConfigData: + if self._config is None: + self._config = self._impl.load_config() + return self._config + + def clear_cache(self): + self._config = None + + def GetConfig(self, + key: str, + default: Optional[str] = None) -> Optional[str]: + """Lazily loads all configration observable for this CachedGitConfigState, + then returns the last value for `key` as a string. + + If `key` is missing, returns default. + """ + values = self._maybe_load_config().get(key, None) + if not values: + return default + + return values[-1] + + def GetConfigBool(self, key: str) -> bool: + """Returns the booleanized value of `key`. + + This follows `git config` semantics (i.e. it normalizes the string value + of the config value to "true" - all other string values return False). + """ + return self.GetConfig(key) == 'true' + + def GetConfigList(self, key: str) -> List[str]: + """Returns all values of `key` as a list of strings.""" + return self._maybe_load_config().get(key, []) + + def YieldConfigRegexp(self, pattern: str) -> Iterable[Tuple[str, str]]: + """Yields (key, value) pairs for any config keys matching `pattern`. + + This use re.match, so `pattern` needs to be for the entire config key. + """ + p = re.compile(pattern) + for name, values in sorted(self._maybe_load_config().items()): + if p.match(name): + for value in values: + yield name, value + + def SetConfig(self, + key, + value=None, + *, + append: bool = False, + missing_ok: bool = True, + modify_all: bool = False, + scope: GitConfigScope = 'local', + value_pattern: Optional[str] = None): + """Sets or unsets one or more config values. + + Args: + cwd: path to set `git config` for. + key: The specific config key to affect. + value: The value to set. If this is None, `key` will be unset. + append: If True and `value` is not None, this will append + the value instead of replacing an existing one. + missing_ok: If `value` is None (i.e. this is an unset operation), + ignore retcode=5 from `git config` (meaning that the value is + not present). If `value` is not None, then this option has no + effect. + modify_all: If True, this will change a set operation to + `--replace-all`, and will change an unset operation to + `--unset-all`. + scope: By default this is the local scope, but could be `system`, + `global`, or `worktree`, depending on which config scope you + want to affect. + value_pattern: For use with `modify_all=True`, allows + further filtering of the set or unset operation based on + the currently configured value. Ignored for + `modify_all=False`. + """ + if value is None: + if key not in self._maybe_load_config(): + if missing_ok: + # Implements early return - underlying state doesn't need to + # be told to remove already-missing keys. + # + # For GitConfigStateReal this avoids lots of unnecessary + # subprocess invocations to unset keys which don't exist. + return + + raise ValueError( + f'CachedGitConfigState: Cannot unset missing key {key!r}' + ' with missing_ok=False.') + + if modify_all: + self._impl.unset_config_multi(key, + value_pattern=value_pattern, + scope=scope) + else: + self._impl.unset_config(key, scope=scope) + else: + if modify_all: + self._impl.set_config_multi(key, + value, + append=append, + value_pattern=value_pattern, + scope=scope) + else: + self._impl.set_config(key, value, append=append, scope=scope) + + # Once the underlying storage has set the value, we clear our cache so + # the next getter will reload it. + self.clear_cache() + + +class GitConfigStateReal(GitConfigStateBase): + """GitConfigStateReal implements CachedGitConfigState by actually interacting with + the git configuration files on disk via GIT.Capture. + """ + + def __init__(self, root: pathlib.Path): + super().__init__() + self.root = root + + def load_config(self) -> GitFlatConfigData: + try: + rawConfig = GIT.Capture(['config', '--list', '-z'], + cwd=self.root, + strip_out=False) + except subprocess2.CalledProcessError: + return {} + + cfg: Dict[str, List[str]] = defaultdict(list) + + # Splitting by '\x00' gets an additional empty string at the end. + for line in rawConfig.split('\x00')[:-1]: + key, value = map(str.strip, line.split('\n', 1)) + cfg[key].append(value) + + return cfg + + def set_config(self, key: str, value: str, *, append: bool, + scope: GitConfigScope): + args = ['config', f'--{scope}', key, value] + if append: + args.append('--add') + GIT.Capture(args, cwd=self.root) + + def set_config_multi(self, key: str, value: str, *, append: bool, + value_pattern: Optional[str], scope: GitConfigScope): + args = ['config', f'--{scope}', '--replace-all', key, value] + if value_pattern is not None: + args.append(value_pattern) + if append: + args.append('--add') + GIT.Capture(args, cwd=self.root) + + def unset_config(self, key: str, *, scope: GitConfigScope): + GIT.Capture(['config', f'--{scope}', '--unset', key], cwd=self.root) + + def unset_config_multi(self, key: str, *, value_pattern: Optional[str], + scope: GitConfigScope): + args = ['config', f'--{scope}', '--unset-all', key] + if value_pattern is not None: + args.append(value_pattern) + GIT.Capture(args, cwd=self.root) + + +class GitConfigStateTest(GitConfigStateBase): + """A fake implementation of GitConfigStateBase for testing.""" + + def __init__(self, + initial_state: Optional[Dict[GitConfigScope, + GitFlatConfigData]] = None): + self.state: Dict[GitConfigScope, Dict[str, List[str]]] = {} + if initial_state is not None: + # We want to copy initial_state to make it mutable inside our class. + for scope, data in initial_state.items(): + self.state[scope] = {k: list(v) for k, v in data.items()} + super().__init__() + + def _get_scope(self, scope: GitConfigScope) -> Dict[str, List[str]]: + ret = self.state.get(scope, None) + if ret is None: + ret = {} + self.state[scope] = ret + return ret + + def load_config(self) -> GitFlatConfigData: + ret = {} + for scope in GitScopeOrder: + for key, value in self._get_scope(scope).items(): + curvals = ret.get(key, None) + if curvals is None: + curvals = [] + ret[key] = curvals + curvals.extend(value) + return ret + + def set_config(self, key: str, value: str, *, append: bool, + scope: GitConfigScope): + cfg = self._get_scope(scope) + cur = cfg.get(key) + if cur is None or len(cur) == 1: + if append: + cfg[key] = (cur or []) + [value] + else: + cfg[key] = [value] + return + raise ValueError(f'GitConfigStateTest: Cannot set key {key} ' + f'- current value {cur!r} is multiple.') + + def set_config_multi(self, key: str, value: str, *, append: bool, + value_pattern: Optional[str], scope: GitConfigScope): + cfg = self._get_scope(scope) + cur = cfg.get(key) + if value_pattern is None or cur is None: + if append: + cfg[key] = (cur or []) + [value] + else: + cfg[key] = [value] + return + + pat = re.compile(value_pattern) + newval = [v for v in cur if pat.match(v)] + newval.append(value) + cfg[key] = newval + + def unset_config(self, key: str, *, scope: GitConfigScope): + cfg = self._get_scope(scope) + cur = cfg.get(key) + if cur is None or len(cur) == 1: + del cfg[key] + return + raise ValueError(f'GitConfigStateTest: Cannot unset key {key} ' + f'- current value {cur!r} is multiple.') + + def unset_config_multi(self, key: str, *, value_pattern: Optional[str], + scope: GitConfigScope): + cfg = self._get_scope(scope) + if value_pattern is None: + del cfg[key] + return + + cur = cfg.get(key) + if cur is None: + del cfg[key] + return + + pat = re.compile(value_pattern) + cfg[key] = [v for v in cur if not pat.match(v)] + + class GIT(object): current_version = None rev_parse_cache = {} @@ -49,44 +396,35 @@ class GIT(object): # This cache speeds up all `git config ...` operations by only running a # single subcommand, which can greatly accelerate things like # git-map-branches. - _CONFIG_CACHE: Mapping[str, Mapping[str, List[str]]] = {} - - @staticmethod - def _load_config(cwd: str) -> Mapping[str, List[str]]: - """Loads git config for the given cwd. - - The calls to this method are cached in-memory for performance. The - config is only reloaded on cache misses. + _CONFIG_CACHE: Dict[pathlib.Path, Optional[CachedGitConfigState]] = {} + _CONFIG_CACHE_LOCK = threading.Lock() - Args: - cwd: path to fetch `git config` for. + @classmethod + def drop_config_cache(cls): + """Completely purges all cached git config data. - Returns: - A dict mapping git config keys to a list of its values. + This should always be safe to call (it will be lazily repopulated), but + really is only meant to be called from tests. """ - if cwd not in GIT._CONFIG_CACHE: - try: - rawConfig = GIT.Capture(['config', '--list', '-z'], - cwd=cwd, - strip_out=False) - except subprocess2.CalledProcessError: - return {} - - cfg = defaultdict(list) - - # Splitting by '\x00' gets an additional empty string at the end. - for line in rawConfig.split('\x00')[:-1]: - key, value = map(str.strip, line.split('\n', 1)) - cfg[key].append(value) - - GIT._CONFIG_CACHE[cwd] = cfg - - return GIT._CONFIG_CACHE[cwd] + with cls._CONFIG_CACHE_LOCK: + cls._CONFIG_CACHE = {} @staticmethod - def _clear_config(cwd: str) -> None: - GIT._CONFIG_CACHE.pop(cwd, None) + def _new_config_state(root: pathlib.Path) -> GitConfigStateBase: + """_new_config_state is mocked in tests/scm_mock to return + a GitConfigStateTest.""" + return GitConfigStateReal(root) + @classmethod + def _get_config_state(cls, cwd: str) -> CachedGitConfigState: + key = pathlib.Path(cwd).absolute() + with cls._CONFIG_CACHE_LOCK: + cur = GIT._CONFIG_CACHE.get(key, None) + if cur is not None: + return cur + ret = CachedGitConfigState(cls._new_config_state(key)) + cls._CONFIG_CACHE[key] = ret + return ret @staticmethod def ApplyEnvVars(kwargs): @@ -152,29 +490,34 @@ class GIT(object): return results @staticmethod - def GetConfig(cwd, key, default=None): - values = GIT._load_config(cwd).get(key, None) - if not values: - return default + def GetConfig(cwd: str, + key: str, + default: Optional[str] = None) -> Optional[str]: + """Lazily loads all configration observable for this CachedGitConfigState, + then returns the last value for `key` as a string. - return values[-1] + If `key` is missing, returns default. + """ + return GIT._get_config_state(cwd).GetConfig(key, default) @staticmethod - def GetConfigBool(cwd, key) -> bool: - return GIT.GetConfig(cwd, key) == 'true' + def GetConfigBool(cwd: str, key: str) -> bool: + """Returns the booleanized value of `key`. + + This follows `git config` semantics (i.e. it normalizes the string value + of the config value to "true" - all other string values return False). + """ + return GIT._get_config_state(cwd).GetConfigBool(key) @staticmethod - def GetConfigList(cwd, key): - return GIT._load_config(cwd).get(key, []) + def GetConfigList(cwd: str, key: str) -> List[str]: + """Returns all values of `key` as a list of strings.""" + return GIT._get_config_state(cwd).GetConfigList(key) @staticmethod - def YieldConfigRegexp(cwd, pattern): + def YieldConfigRegexp(cwd: str, pattern: str) -> Iterable[Tuple[str, str]]: """Yields (key, value) pairs for any config keys matching `pattern`.""" - p = re.compile(pattern) - for name, values in GIT._load_config(cwd).items(): - if p.match(name): - for value in values: - yield name, value + yield from GIT._get_config_state(cwd).YieldConfigRegexp(pattern) @staticmethod def GetBranchConfig(cwd, branch, key, default=None): @@ -183,17 +526,15 @@ class GIT(object): return GIT.GetConfig(cwd, key, default) @staticmethod - def SetConfig( - cwd, - key, - value=None, - *, - append=False, - missing_ok=True, - modify_all=False, - scope='local', - value_pattern=None, - ): + def SetConfig(cwd: str, + key: str, + value: Optional[str] = None, + *, + append: bool = False, + missing_ok: bool = True, + modify_all: bool = False, + scope: GitConfigScope = 'local', + value_pattern: Optional[str] = None): """Sets or unsets one or more config values. Args: @@ -217,26 +558,13 @@ class GIT(object): the currently configured value. Ignored for `modify_all=False`. """ - GIT._clear_config(cwd) - - args = ['config', f'--{scope}'] - if value == None: - args.extend(['--unset' + ('-all' if modify_all else ''), key]) - else: - if modify_all: - args.append('--replace-all') - if append: - args.append('--add') - args.extend([key, value]) - - if modify_all and value_pattern: - args.append(value_pattern) - - accepted_retcodes = [0] - if value is None and missing_ok: - accepted_retcodes = [0, 5] - - GIT.Capture(args, cwd=cwd, accepted_retcodes=accepted_retcodes) + GIT._get_config_state(cwd).SetConfig(key, + value, + append=append, + missing_ok=missing_ok, + modify_all=modify_all, + scope=scope, + value_pattern=value_pattern) @staticmethod def SetBranchConfig(cwd, branch, key, value=None): diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 207a8fb9c..631de5c67 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -17,8 +17,12 @@ import tempfile import unittest from unittest import mock +import scm_mock + sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import scm + import gclient_scm import gclient_utils import git_cache @@ -51,29 +55,25 @@ def strip_timestamps(value): class BasicTests(unittest.TestCase): - @mock.patch('gclient_scm.scm.GIT.Capture') - def testGetFirstRemoteUrl(self, mockCapture): - REMOTE_STRINGS = [ - ('remote.origin.url\nE:\\foo\\bar\x00', 'E:\\foo\\bar'), - ('remote.origin.url\n/b/foo/bar\x00', '/b/foo/bar'), - ('remote.origin.url\nhttps://foo/bar\x00', 'https://foo/bar'), - ('remote.origin.url\nE:\\Fo Bar\\bax\x00', 'E:\\Fo Bar\\bax'), - ('remote.origin.url\ngit://what/"do\x00', 'git://what/"do') - ] - FAKE_PATH = '/fake/path' - mockCapture.side_effect = [question for question, _ in REMOTE_STRINGS] - for _, answer in REMOTE_STRINGS: - self.assertEqual( - gclient_scm.SCMWrapper._get_first_remote_url(FAKE_PATH), answer) - gclient_scm.scm.GIT._clear_config(FAKE_PATH) + def setUp(self) -> None: + scm_mock.GIT(self) + return super().setUp() - expected_calls = [ - mock.call(['config', '--list', '-z'], - cwd=FAKE_PATH, - strip_out=False) for _ in REMOTE_STRINGS - ] - self.assertEqual(mockCapture.mock_calls, expected_calls) + def testGetFirstRemoteUrl(self): + FAKE_PATH = '/fake/path' + scm.GIT.SetConfig(FAKE_PATH, + 'remote.origin.url', + 'first-value', + append=True) + scm.GIT.SetConfig(FAKE_PATH, + 'remote.origin.url', + 'second-value', + append=True) + + self.assertEqual( + gclient_scm.SCMWrapper._get_first_remote_url(FAKE_PATH), + 'first-value') class BaseGitWrapperTestCase(unittest.TestCase, test_case_utils.TestCaseUtils): @@ -246,15 +246,16 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): return options = self.Options() file_path = join(self.base_path, 'a') - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, None, file_list) + git_wrapper.update(options, None, file_list) gclient_scm.os.remove(file_path) file_list = [] - scm.revert(options, self.args, file_list) + git_wrapper.revert(options, self.args, file_list) self.assertEqual(file_list, [file_path]) file_list = [] - scm.diff(options, self.args, file_list) + git_wrapper.diff(options, self.args, file_list) self.assertEqual(file_list, []) sys.stdout.close() @@ -262,13 +263,14 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, None, file_list) + git_wrapper.update(options, None, file_list) file_list = [] - scm.revert(options, self.args, file_list) + git_wrapper.revert(options, self.args, file_list) self.assertEqual(file_list, []) - self.assertEqual(scm.revinfo(options, self.args, None), + self.assertEqual(git_wrapper.revinfo(options, self.args, None), '4091c7d010ca99d0f2dd416d4b70b758ae432187') sys.stdout.close() @@ -276,19 +278,20 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, None, file_list) + git_wrapper.update(options, None, file_list) file_path = join(self.base_path, 'a') with open(file_path, 'a') as f: f.writelines('touched\n') file_list = [] - scm.revert(options, self.args, file_list) + git_wrapper.revert(options, self.args, file_list) self.assertEqual(file_list, [file_path]) file_list = [] - scm.diff(options, self.args, file_list) + git_wrapper.diff(options, self.args, file_list) self.assertEqual(file_list, []) - self.assertEqual(scm.revinfo(options, self.args, None), + self.assertEqual(git_wrapper.revinfo(options, self.args, None), '4091c7d010ca99d0f2dd416d4b70b758ae432187') sys.stdout.close() @@ -296,21 +299,22 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, None, file_list) + git_wrapper.update(options, None, file_list) file_path = join(self.base_path, 'c') with open(file_path, 'w') as f: f.writelines('new\n') Popen([GIT, 'add', 'c'], stdout=PIPE, stderr=STDOUT, cwd=self.base_path).communicate() file_list = [] - scm.revert(options, self.args, file_list) + git_wrapper.revert(options, self.args, file_list) self.assertEqual(file_list, [file_path]) file_list = [] - scm.diff(options, self.args, file_list) + git_wrapper.diff(options, self.args, file_list) self.assertEqual(file_list, []) - self.assertEqual(scm.revinfo(options, self.args, None), + self.assertEqual(git_wrapper.revinfo(options, self.args, None), '4091c7d010ca99d0f2dd416d4b70b758ae432187') sys.stdout.close() @@ -321,11 +325,11 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): file_paths = [join(self.base_path, 'a')] with open(file_paths[0], 'a') as f: f.writelines('touched\n') - scm = gclient_scm.GitWrapper(self.url + '@refs/heads/feature', - self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url + '@refs/heads/feature', + self.root_dir, self.relpath) file_paths.append(join(self.base_path, 'c')) # feature branch touches c file_list = [] - scm.status(options, self.args, file_list) + git_wrapper.status(options, self.args, file_list) self.assertEqual(file_list, file_paths) self.checkstdout(( '\n________ running \'git -c core.quotePath=false diff --name-status ' @@ -339,11 +343,11 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): file_path = join(self.base_path, 'a') with open(file_path, 'a') as f: f.writelines('touched\n') - scm = gclient_scm.GitWrapper( + git_wrapper = gclient_scm.GitWrapper( self.url + '@069c602044c5388d2d15c3f875b057c852003458', self.root_dir, self.relpath) file_list = [] - scm.status(options, self.args, file_list) + git_wrapper.status(options, self.args, file_list) self.assertEqual(file_list, [file_path]) self.checkstdout(( '\n________ running \'git -c core.quotePath=false diff --name-status ' @@ -357,9 +361,10 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): file_path = join(self.base_path, 'a') with open(file_path, 'a') as f: f.writelines('touched\n') - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.status(options, self.args, file_list) + git_wrapper.status(options, self.args, file_list) self.assertEqual(file_list, [file_path]) self.checkstdout(( '\n________ running \'git -c core.quotePath=false diff --name-status' @@ -375,11 +380,11 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): with open(file_path, 'a') as f: f.writelines('touched\n') expected_file_list.extend([file_path]) - scm = gclient_scm.GitWrapper( + git_wrapper = gclient_scm.GitWrapper( self.url + '@069c602044c5388d2d15c3f875b057c852003458', self.root_dir, self.relpath) file_list = [] - scm.status(options, self.args, file_list) + git_wrapper.status(options, self.args, file_list) expected_file_list = [join(self.base_path, x) for x in ['a', 'b']] self.assertEqual(sorted(file_list), expected_file_list) self.checkstdout(( @@ -394,19 +399,23 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): expected_file_list = [ join(self.base_path, x) for x in ['a', 'b', 'submodule'] ] - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '4091c7d010ca99d0f2dd416d4b70b758ae432187') self.assertEqual( - scm._Capture(['config', '--get', 'diff.ignoreSubmodules']), 'dirty') + git_wrapper._Capture(['config', '--get', 'diff.ignoreSubmodules']), + 'dirty') self.assertEqual( - scm._Capture(['config', '--get', 'fetch.recurseSubmodules']), 'off') + git_wrapper._Capture(['config', '--get', + 'fetch.recurseSubmodules']), 'off') self.assertEqual( - scm._Capture(['config', '--get', 'push.recurseSubmodules']), 'off') + git_wrapper._Capture(['config', '--get', 'push.recurseSubmodules']), + 'off') sys.stdout.close() def testUpdateMerge(self): @@ -414,50 +423,52 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): return options = self.Options() options.merge = True - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) # This sets correct remote HEAD - scm.update(options, (), []) + git_wrapper.update(options, (), []) - scm._Run(['checkout', '-q', 'feature'], options) - rev = scm.revinfo(options, (), None) + git_wrapper._Run(['checkout', '-q', 'feature'], options) + rev = git_wrapper.revinfo(options, (), None) file_list = [] - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual( file_list, [join(self.base_path, x) for x in ['a', 'b', 'c', 'submodule']]) # The actual commit that is created is unstable, so we verify its tree # and parents instead. - self.assertEqual(scm._Capture(['rev-parse', 'HEAD:']), + self.assertEqual(git_wrapper._Capture(['rev-parse', 'HEAD:']), '3a3ba72731fa208d37b06598a129ba93970325df') - self.assertEqual(scm._Capture(['rev-parse', 'HEAD^1']), rev) - self.assertEqual(scm._Capture(['rev-parse', 'HEAD^2']), - scm._Capture(['rev-parse', 'origin/main'])) + self.assertEqual(git_wrapper._Capture(['rev-parse', 'HEAD^1']), rev) + self.assertEqual(git_wrapper._Capture(['rev-parse', 'HEAD^2']), + git_wrapper._Capture(['rev-parse', 'origin/main'])) sys.stdout.close() def testUpdateRebase(self): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) # This sets correct remote HEAD - scm.update(options, (), []) + git_wrapper.update(options, (), []) - scm._Run(['checkout', '-q', 'feature'], options) + git_wrapper._Run(['checkout', '-q', 'feature'], options) # Fake a 'y' key press. - scm._AskForData = self._GetAskForDataCallback( + git_wrapper._AskForData = self._GetAskForDataCallback( 'Cannot fast-forward merge, attempt to rebase? ' '(y)es / (q)uit / (s)kip : ', 'y') file_list = [] - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual( file_list, [join(self.base_path, x) for x in ['a', 'b', 'c', 'submodule']]) # The actual commit that is created is unstable, so we verify its tree # and parent instead. - self.assertEqual(scm._Capture(['rev-parse', 'HEAD:']), + self.assertEqual(git_wrapper._Capture(['rev-parse', 'HEAD:']), '3a3ba72731fa208d37b06598a129ba93970325df') - self.assertEqual(scm._Capture(['rev-parse', 'HEAD^1']), - scm._Capture(['rev-parse', 'origin/main'])) + self.assertEqual(git_wrapper._Capture(['rev-parse', 'HEAD^1']), + git_wrapper._Capture(['rev-parse', 'origin/main'])) sys.stdout.close() def testUpdateReset(self): @@ -475,9 +486,10 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): with open(file_path, 'w') as f: f.writelines('new\n') - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assert_(gclient_scm.os.path.isdir(dir_path)) self.assert_(gclient_scm.os.path.isfile(file_path)) sys.stdout.close() @@ -488,15 +500,16 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): options = self.Options() options.reset = True - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) - scm._Run([ + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) + git_wrapper._Run([ 'config', 'remote.origin.fetch', '+refs/heads/bad/ref:refs/remotes/origin/bad/ref' ], options) file_list = [] - scm.update(options, (), file_list) - self.assertEqual(scm.revinfo(options, (), None), + git_wrapper.update(options, (), file_list) + self.assertEqual(git_wrapper.revinfo(options, (), None), '069c602044c5388d2d15c3f875b057c852003458') sys.stdout.close() @@ -516,9 +529,10 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): with open(file_path, 'w') as f: f.writelines('new\n') - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assert_(not gclient_scm.os.path.isdir(dir_path)) self.assert_(gclient_scm.os.path.isfile(file_path)) sys.stdout.close() @@ -527,12 +541,13 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_path = join(self.base_path, 'b') with open(file_path, 'w') as f: f.writelines('conflict\n') try: - scm.update(options, (), []) + git_wrapper.update(options, (), []) self.fail() except (gclient_scm.gclient_utils.Error, subprocess2.CalledProcessError): @@ -549,12 +564,13 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_path = join(self.base_path, '.git', 'index.lock') with open(file_path, 'w'): pass with self.assertRaises(subprocess2.CalledProcessError): - scm.update(options, (), []) + git_wrapper.update(options, (), []) sys.stdout.close() def testUpdateLockedBreak(self): @@ -562,11 +578,12 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): return options = self.Options() options.break_repo_locks = True - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_path = join(self.base_path, '.git', 'index.lock') with open(file_path, 'w'): pass - scm.update(options, (), []) + git_wrapper.update(options, (), []) self.assertRegexpMatches(sys.stdout.getvalue(), r'breaking lock.*\.git[/|\\]index\.lock') self.assertFalse(os.path.exists(file_path)) @@ -576,24 +593,25 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_path = join(self.base_path, 'b') with open(file_path, 'w') as f: f.writelines('conflict\n') - scm._Run(['commit', '-am', 'test'], options) - scm._AskForData = self._GetAskForDataCallback( + git_wrapper._Run(['commit', '-am', 'test'], options) + git_wrapper._AskForData = self._GetAskForDataCallback( 'Cannot fast-forward merge, attempt to rebase? ' '(y)es / (q)uit / (s)kip : ', 'y') with self.assertRaises(gclient_scm.gclient_utils.Error) as e: - scm.update(options, (), []) + git_wrapper.update(options, (), []) self.assertEqual( e.exception.args[0], 'Conflict while rebasing this branch.\n' 'Fix the conflict and run gclient again.\n' 'See \'man git-rebase\' for details.\n') with self.assertRaises(gclient_scm.gclient_utils.Error) as e: - scm.update(options, (), []) + git_wrapper.update(options, (), []) self.assertEqual( e.exception.args[0], '\n____ . at refs/remotes/origin/main\n' '\tYou have uncommitted changes.\n' @@ -606,8 +624,9 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): if not self.enabled: return options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) - rev_info = scm.revinfo(options, (), None) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) + rev_info = git_wrapper.revinfo(options, (), None) self.assertEqual(rev_info, '069c602044c5388d2d15c3f875b057c852003458') @@ -679,8 +698,9 @@ class ManagedGitWrapperTestCaseMock(unittest.TestCase): mockRun.side_effect = ['refs/remotes/origin/main', '', ''] options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) - scm.update(options, None, []) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) + git_wrapper.update(options, None, []) env = gclient_scm.scm.GIT.ApplyEnvVars({}) self.assertEqual(mockRun.mock_calls, [ @@ -721,8 +741,9 @@ class ManagedGitWrapperTestCaseMock(unittest.TestCase): ] options = self.Options() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) - scm.update(options, None, []) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) + git_wrapper.update(options, None, []) env = gclient_scm.scm.GIT.ApplyEnvVars({}) self.assertEqual(mockRun.mock_calls, [ @@ -782,8 +803,8 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): self.relpath = '.' self.base_path = join(self.root_dir, self.relpath) - scm = gclient_scm.GitWrapper(origin_root_dir, self.root_dir, - self.relpath) + git_wrapper = gclient_scm.GitWrapper(origin_root_dir, self.root_dir, + self.relpath) expected_file_list = [ join(self.base_path, "a"), @@ -791,10 +812,10 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): ] file_list = [] options.revision = 'unmanaged' - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '069c602044c5388d2d15c3f875b057c852003458') # indicates detached HEAD self.assertEqual(self.getCurrentBranch(), None) @@ -815,8 +836,8 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): url_with_commit_ref = origin_root_dir +\ '@4091c7d010ca99d0f2dd416d4b70b758ae432187' - scm = gclient_scm.GitWrapper(url_with_commit_ref, self.root_dir, - self.relpath) + git_wrapper = gclient_scm.GitWrapper(url_with_commit_ref, self.root_dir, + self.relpath) expected_file_list = [ join(self.base_path, "a"), @@ -825,10 +846,10 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): ] file_list = [] options.revision = 'unmanaged' - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '4091c7d010ca99d0f2dd416d4b70b758ae432187') # indicates detached HEAD self.assertEqual(self.getCurrentBranch(), None) @@ -849,8 +870,8 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): self.base_path = join(self.root_dir, self.relpath) url_with_branch_ref = origin_root_dir + '@feature' - scm = gclient_scm.GitWrapper(url_with_branch_ref, self.root_dir, - self.relpath) + git_wrapper = gclient_scm.GitWrapper(url_with_branch_ref, self.root_dir, + self.relpath) expected_file_list = [ join(self.base_path, "a"), @@ -859,10 +880,10 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): ] file_list = [] options.revision = 'unmanaged' - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '9a51244740b25fa2ded5252ca00a3178d3f665a9') # indicates detached HEAD self.assertEqual(self.getCurrentBranch(), None) @@ -883,8 +904,8 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): self.base_path = join(self.root_dir, self.relpath) url_with_branch_ref = origin_root_dir + '@refs/remotes/origin/feature' - scm = gclient_scm.GitWrapper(url_with_branch_ref, self.root_dir, - self.relpath) + git_wrapper = gclient_scm.GitWrapper(url_with_branch_ref, self.root_dir, + self.relpath) expected_file_list = [ join(self.base_path, "a"), @@ -893,10 +914,10 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): ] file_list = [] options.revision = 'unmanaged' - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '9a51244740b25fa2ded5252ca00a3178d3f665a9') # indicates detached HEAD self.assertEqual(self.getCurrentBranch(), None) @@ -916,8 +937,8 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): self.base_path = join(self.root_dir, self.relpath) url_with_branch_ref = origin_root_dir + '@refs/heads/feature' - scm = gclient_scm.GitWrapper(url_with_branch_ref, self.root_dir, - self.relpath) + git_wrapper = gclient_scm.GitWrapper(url_with_branch_ref, self.root_dir, + self.relpath) expected_file_list = [ join(self.base_path, "a"), @@ -926,10 +947,10 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): ] file_list = [] options.revision = 'unmanaged' - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '9a51244740b25fa2ded5252ca00a3178d3f665a9') # @refs/heads/feature is AKA @refs/remotes/origin/feature in the clone, # so should be treated as such by gclient. TODO(mmoss): Though really, @@ -948,12 +969,13 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase): return options = self.Options() expected_file_list = [] - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) file_list = [] options.revision = 'unmanaged' - scm.update(options, (), file_list) + git_wrapper.update(options, (), file_list) self.assertEqual(file_list, expected_file_list) - self.assertEqual(scm.revinfo(options, (), None), + self.assertEqual(git_wrapper.revinfo(options, (), None), '069c602044c5388d2d15c3f875b057c852003458') self.checkstdout('________ unmanaged solution; skipping .\n') @@ -998,14 +1020,14 @@ class CipdWrapperTestCase(unittest.TestCase): def testRevert(self): """Checks that revert does nothing.""" - scm = self.createScmWithPackageThatSatisfies(lambda _: True) - scm.revert(None, (), []) + git_wrapper = self.createScmWithPackageThatSatisfies(lambda _: True) + git_wrapper.revert(None, (), []) @mock.patch('gclient_scm.gclient_utils.CheckCallAndFilter') @mock.patch('gclient_scm.gclient_utils.rmtree') def testRevinfo(self, mockRmtree, mockCheckCallAndFilter): """Checks that revinfo uses the JSON from cipd describe.""" - scm = self.createScmWithPackageThatSatisfies(lambda _: True) + git_wrapper = self.createScmWithPackageThatSatisfies(lambda _: True) expected_revinfo = '0123456789abcdef0123456789abcdef01234567' json_contents = { @@ -1019,7 +1041,7 @@ class CipdWrapperTestCase(unittest.TestCase): with open(describe_json_path, 'w') as describe_json: json.dump(json_contents, describe_json) - revinfo = scm.revinfo(None, (), []) + revinfo = git_wrapper.revinfo(None, (), []) self.assertEqual(revinfo, expected_revinfo) mockRmtree.assert_called_with(self._workdir) @@ -1037,8 +1059,8 @@ class CipdWrapperTestCase(unittest.TestCase): def testUpdate(self): """Checks that update does nothing.""" - scm = self.createScmWithPackageThatSatisfies(lambda _: True) - scm.update(None, (), []) + git_wrapper = self.createScmWithPackageThatSatisfies(lambda _: True) + git_wrapper.update(None, (), []) class GcsWrapperTestCase(unittest.TestCase): @@ -1054,18 +1076,18 @@ class GcsWrapperTestCase(unittest.TestCase): def testRevert(self): """Checks that revert does nothing.""" - scm = self.createScm() - scm.revert(None, (), []) + git_wrapper = self.createScm() + git_wrapper.revert(None, (), []) def testRevinfo(self): """Checks that revinfo does nothing.""" - scm = self.createScm() - scm.revinfo(None, (), []) + git_wrapper = self.createScm() + git_wrapper.revinfo(None, (), []) def testUpdate(self): """Checks that update does nothing.""" - scm = self.createScm() - scm.update(None, (), []) + git_wrapper = self.createScm() + git_wrapper.update(None, (), []) class BranchHeadsFakeRepo(fake_repos.FakeReposBase): @@ -1106,11 +1128,11 @@ class BranchHeadsTest(fake_repos.FakeReposTestBase): self.addCleanup(git_cache.Mirror.SetCachePath, None) def testCheckoutBranchHeads(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = 'refs/branch-heads/5' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir)) @@ -1123,9 +1145,9 @@ class BranchHeadsTest(fake_repos.FakeReposTestBase): cwd=self.url) # Sync to refs/branch-heads/5 - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') self.options.revision = 'refs/branch-heads/5' - scm.update(self.options, None, []) + git_wrapper.update(self.options, None, []) # Set refs/branch-heads/5 back to its original value. subprocess2.check_call([ @@ -1241,25 +1263,25 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): 'Unexpected commit: %s' % name) def testCanCloneGerritChange(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = 'refs/changes/35/1235/1' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 6), self.gitrevparse(self.root_dir)) def testCanSyncToGerritChange(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = self.githash('repo_1', 1) - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) self.options.revision = 'refs/changes/35/1235/1' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 6), self.gitrevparse(self.root_dir)) @@ -1274,13 +1296,13 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): def testMirrorPushUrl(self): self.setUpMirror() - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] - self.assertIsNotNone(scm._GetMirror(self.url, self.options)) + self.assertIsNotNone(git_wrapper._GetMirror(self.url, self.options)) - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) - fetch_url = scm._Capture(['remote', 'get-url', 'origin']) + fetch_url = git_wrapper._Capture(['remote', 'get-url', 'origin']) self.assertTrue( fetch_url.startswith(self.mirror), msg='\n'.join([ @@ -1288,22 +1310,23 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): ' fetch_url: %s' % fetch_url, ' mirror: %s' % self.mirror ])) - push_url = scm._Capture(['remote', 'get-url', '--push', 'origin']) + push_url = git_wrapper._Capture( + ['remote', 'get-url', '--push', 'origin']) self.assertEqual(push_url, self.url) def testAppliesPatchOnTopOfMasterByDefault(self): """Test the default case, where we apply a patch on top of main.""" - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Make sure we don't specify a revision. self.options.revision = None - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 2, 3, 4, 5, 6]) self.assertEqual(self.githash('repo_1', 4), @@ -1316,18 +1339,18 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): contains commits 5 and 6, and is based on top of commit 3. The final result should contain commits 1, 5 and 6, but not commits 2 or 3. """ - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Sync to commit 1 self.options.revision = self.githash('repo_1', 1) - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) # Apply the change on top of that. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 5, 6]) self.assertEqual(self.githash('repo_1', 1), @@ -1335,18 +1358,19 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): def testCheckoutOriginFeature(self): """Tests that we can apply a patch on a branch other than main.""" - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Sync to remote's refs/heads/feature self.options.revision = 'refs/heads/feature' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 9), self.gitrevparse(self.root_dir)) # Apply the change on top of that. - scm.apply_patch_ref(self.url, 'refs/changes/36/1236/1', - 'refs/heads/feature', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/36/1236/1', + 'refs/heads/feature', self.options, + file_list) self.assertCommits([1, 2, 7, 8, 9, 10]) self.assertEqual(self.githash('repo_1', 9), @@ -1355,18 +1379,19 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): def testCheckoutOriginFeatureOnOldRevision(self): """Tests that we can apply a patch on an old checkout, on a branch other than main.""" - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Sync to remote's refs/heads/feature on an old revision self.options.revision = self.githash('repo_1', 7) - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 7), self.gitrevparse(self.root_dir)) # Apply the change on top of that. - scm.apply_patch_ref(self.url, 'refs/changes/36/1236/1', - 'refs/heads/feature', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/36/1236/1', + 'refs/heads/feature', self.options, + file_list) # We shouldn't have rebased on top of 2 (which is the merge base between # remote's main branch and the change) but on top of 7 (which is the @@ -1376,19 +1401,19 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): self.gitrevparse(self.root_dir)) def testCheckoutOriginFeaturePatchBranch(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Sync to the hash instead of remote's refs/heads/feature. self.options.revision = self.githash('repo_1', 9) - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 9), self.gitrevparse(self.root_dir)) # Apply refs/changes/34/1234/1, created for remote's main branch on top # of remote's feature branch. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) # Commits 5 and 6 are part of the patch, and commits 1, 2, 7, 8 and 9 # are part of remote's feature branch. @@ -1398,17 +1423,17 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): def testDoesntRebasePatchMaster(self): """Tests that we can apply a patch without rebasing it.""" - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.rebase_patch_ref = False - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) # Apply the change on top of that. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 2, 3, 5, 6]) self.assertEqual(self.githash('repo_1', 5), @@ -1417,19 +1442,19 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): def testDoesntRebasePatchOldCheckout(self): """Tests that we can apply a patch without rebasing it on an old checkout. """ - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Sync to commit 1 self.options.revision = self.githash('repo_1', 1) self.options.rebase_patch_ref = False - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) # Apply the change on top of that. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 2, 3, 5, 6]) self.assertEqual(self.githash('repo_1', 5), @@ -1437,16 +1462,16 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): def testDoesntSoftResetIfNotAskedTo(self): """Test that we can apply a patch without doing a soft reset.""" - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.reset_patch_ref = False - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 2, 3, 4, 5, 6]) # The commit hash after cherry-picking is not known, but it must be @@ -1468,19 +1493,19 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): } }]) def testDownloadTopics(self, query_changes_mock, get_change_mock): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = 'refs/changes/34/1234/1' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir)) # pylint: disable=attribute-defined-outside-init self.options.download_topics = True - scm.url = 'https://test-repo.googlesource.com/repo_1.git' - scm.apply_patch_ref(self.url, 'refs/changes/34/1234/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.url = 'https://test-repo.googlesource.com/repo_1.git' + git_wrapper.apply_patch_ref(self.url, 'refs/changes/34/1234/1', + 'refs/heads/main', self.options, file_list) get_change_mock.assert_called_once_with(mock.ANY, '1234') query_changes_mock.assert_called_once_with(mock.ANY, @@ -1496,36 +1521,37 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): self.gitrevparse(self.root_dir)) def testRecoversAfterPatchFailure(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = 'refs/changes/34/1234/1' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir)) # Checkout 'refs/changes/34/1234/1' modifies the 'change' file, so # trying to patch 'refs/changes/36/1236/1' creates a patch failure. with self.assertRaises(subprocess2.CalledProcessError) as cm: - scm.apply_patch_ref(self.url, 'refs/changes/36/1236/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/36/1236/1', + 'refs/heads/main', self.options, + file_list) self.assertEqual(cm.exception.cmd[3], 'cherry-pick') self.assertIn(b'error: could not apply', cm.exception.stderr) # Try to apply 'refs/changes/35/1235/1', which doesn't have a merge # conflict. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 2, 3, 5, 6]) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir)) def testIgnoresAlreadyMergedCommits(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = 'refs/heads/main-with-5' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 12), self.gitrevparse(self.root_dir)) @@ -1533,32 +1559,34 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): # 'refs/changes/34/1234/1' will be an empty commit, since the changes # were already present in the tree as commit 11. Make sure we deal with # this gracefully. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/feature', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/feature', self.options, + file_list) self.assertCommits([1, 2, 3, 5, 6, 12]) self.assertEqual(self.githash('repo_1', 12), self.gitrevparse(self.root_dir)) def testRecoversFromExistingCherryPick(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] self.options.revision = 'refs/changes/34/1234/1' - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir)) # Checkout 'refs/changes/34/1234/1' modifies the 'change' file, so # trying to cherry-pick 'refs/changes/36/1236/1' raises an error. - scm._Run(['fetch', 'origin', 'refs/changes/36/1236/1'], self.options) + git_wrapper._Run(['fetch', 'origin', 'refs/changes/36/1236/1'], + self.options) with self.assertRaises(subprocess2.CalledProcessError) as cm: - scm._Run(['cherry-pick', 'FETCH_HEAD'], self.options) + git_wrapper._Run(['cherry-pick', 'FETCH_HEAD'], self.options) self.assertEqual(cm.exception.cmd[:2], ['git', 'cherry-pick']) # Try to apply 'refs/changes/35/1235/1', which doesn't have a merge # conflict. - scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', - 'refs/heads/main', self.options, file_list) + git_wrapper.apply_patch_ref(self.url, 'refs/changes/35/1235/1', + 'refs/heads/main', self.options, file_list) self.assertCommits([1, 2, 3, 5, 6]) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir)) @@ -1594,42 +1622,44 @@ class CheckDiffTest(fake_repos.FakeReposTestBase): def testCheckDiff(self): """Correctly check for diffs.""" - scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, '.') file_list = [] # Make sure we don't specify a revision. self.options.revision = None - scm.update(self.options, None, file_list) + git_wrapper.update(self.options, None, file_list) self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) self.assertFalse( - scm.check_diff(self.githash('repo_1', 1), files=['DEPS'])) - self.assertTrue(scm.check_diff(self.githash('repo_1', 1))) + git_wrapper.check_diff(self.githash('repo_1', 1), files=['DEPS'])) + self.assertTrue(git_wrapper.check_diff(self.githash('repo_1', 1))) self.assertTrue( - scm.check_diff(self.githash('repo_1', 3), files=['DEPS'])) + git_wrapper.check_diff(self.githash('repo_1', 3), files=['DEPS'])) self.assertFalse( - scm.check_diff(self.githash('repo_1', 2), - files=['DEPS', 'doesnotmatter'])) - self.assertFalse(scm.check_diff(self.githash('repo_1', 2))) + git_wrapper.check_diff(self.githash('repo_1', 2), + files=['DEPS', 'doesnotmatter'])) + self.assertFalse(git_wrapper.check_diff(self.githash('repo_1', 2))) class Submodules(BaseGitWrapperTestCase): submodule_hash = '1111111111111111111111111111111111111111' def testGetSubmoduleClean(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) options = self.Options() - scm.update(options, None, []) - self.assertEqual(scm.GetSubmoduleStateFromIndex(), + git_wrapper.update(options, None, []) + self.assertEqual(git_wrapper.GetSubmoduleStateFromIndex(), {'submodule': self.submodule_hash}) - self.assertEqual(scm.GetSubmoduleDiff(), {}) + self.assertEqual(git_wrapper.GetSubmoduleDiff(), {}) def testGetSubmoduleModified(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) options = self.Options() - scm.update(options, None, []) + git_wrapper.update(options, None, []) # Create submodule diff submodule_dir = os.path.join(self.root_dir, 'submodule') @@ -1645,25 +1675,26 @@ class Submodules(BaseGitWrapperTestCase): with open(os.path.join(self.root_dir, 'a'), 'w') as f: f.write('foo') - self.assertEqual(scm.GetSubmoduleStateFromIndex(), + self.assertEqual(git_wrapper.GetSubmoduleStateFromIndex(), {'submodule': self.submodule_hash}) - self.assertEqual(scm.GetSubmoduleDiff(), + self.assertEqual(git_wrapper.GetSubmoduleDiff(), {'submodule': (self.submodule_hash, new_rev)}) def testGetSubmoduleDeleted(self): - scm = gclient_scm.GitWrapper(self.url, self.root_dir, self.relpath) + git_wrapper = gclient_scm.GitWrapper(self.url, self.root_dir, + self.relpath) options = self.Options() - scm.update(options, None, []) + git_wrapper.update(options, None, []) subprocess2.check_output( ['git', '-C', self.root_dir, 'rm', 'submodule']) # When git removes submodule, it's autmatically staged and content is # unavailable. Therefore, the index shouldn't have any entries and diff # should be empty. - self.assertEqual(scm.GetSubmoduleStateFromIndex(), {}) + self.assertEqual(git_wrapper.GetSubmoduleStateFromIndex(), {}) - self.assertEqual(scm.GetSubmoduleDiff(), {}) + self.assertEqual(git_wrapper.GetSubmoduleDiff(), {}) if 'unittest.util' in __import__('sys').modules: diff --git a/tests/gerrit_util_test.py b/tests/gerrit_util_test.py index 3b94406f4..ddd0fee54 100755 --- a/tests/gerrit_util_test.py +++ b/tests/gerrit_util_test.py @@ -21,9 +21,11 @@ import httplib2 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import scm_mock + import gerrit_util -import git_common import metrics +import scm import subprocess2 RUN_SUBPROC_TESTS = 'RUN_SUBPROC_TESTS' in os.environ @@ -104,6 +106,7 @@ class CookiesAuthenticatorTest(unittest.TestCase): return_value=self._GITCOOKIES).start() mock.patch('os.getenv', return_value={}).start() mock.patch('os.environ', {'HOME': '$HOME'}).start() + mock.patch('os.getcwd', return_value='/fame/cwd').start() mock.patch('os.path.exists', return_value=True).start() mock.patch( 'git_common.run', @@ -111,6 +114,8 @@ class CookiesAuthenticatorTest(unittest.TestCase): subprocess2.CalledProcessError(1, ['cmd'], 'cwd', 'out', 'err') ], ).start() + scm_mock.GIT(self) + self.addCleanup(mock.patch.stopall) self.maxDiff = None @@ -144,16 +149,10 @@ class CookiesAuthenticatorTest(unittest.TestCase): os.path.expanduser(os.path.join('~', '.gitcookies')), gerrit_util.CookiesAuthenticator().get_gitcookies_path()) - git_common.run.side_effect = ['http.cookiefile\nhttp.cookiefile\x00'] + scm.GIT.SetConfig(os.getcwd(), 'http.cookiefile', '/some/path') self.assertEqual( - 'http.cookiefile', + '/some/path', gerrit_util.CookiesAuthenticator().get_gitcookies_path()) - git_common.run.assert_called_with('config', - '--list', - '-z', - autostrip=False, - cwd=os.getcwd(), - env=mock.ANY) os.getenv.return_value = 'git-cookies-path' self.assertEqual( @@ -717,9 +716,17 @@ class ShouldUseSSOTest(unittest.TestCase): def setUp(self) -> None: self.newauth = mock.patch('newauth.Enabled', return_value=True) self.newauth.start() + + self.cwd = mock.patch('os.getcwd', return_value='/fake/cwd') + self.cwd.start() + self.sso = mock.patch('gerrit_util.ssoHelper.find_cmd', return_value='/fake/git-remote-sso') self.sso.start() + + scm_mock.GIT(self) + + self.addCleanup(mock.patch.stopall) return super().setUp() def tearDown(self) -> None: diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index c61e2a1b7..c49305f79 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -17,10 +17,13 @@ import shutil import sys import tempfile import unittest + from unittest import mock sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import scm_mock + import metrics import metrics_utils # We have to disable monitoring before importing git_cl. @@ -84,33 +87,6 @@ class ChangelistMock(object): return ('origin', 'refs/remotes/origin/main') -class GitMocks(object): - def __init__(self, config=None, branchref=None): - self.branchref = branchref or 'refs/heads/main' - self.config = config or {} - - def GetBranchRef(self, _root): - return self.branchref - - def NewBranch(self, branchref): - self.branchref = branchref - - def GetConfig(self, root, key, default=None): - if root != '': - key = '%s:%s' % (root, key) - return self.config.get(key, default) - - def SetConfig(self, root, key, value=None): - if root != '': - key = '%s:%s' % (root, key) - if value: - self.config[key] = value - return - if key not in self.config: - raise CERR1 - del self.config[key] - - class WatchlistsMock(object): def __init__(self, _): pass @@ -654,14 +630,9 @@ class TestGitCl(unittest.TestCase): lambda *a: self._mocked_call('ValidAccounts', *a)).start() mock.patch('sys.exit', side_effect=SystemExitMock).start() mock.patch('git_cl.Settings.GetRoot', return_value='').start() - self.mockGit = GitMocks() - mock.patch('scm.GIT.GetBranchRef', self.mockGit.GetBranchRef).start() - mock.patch('scm.GIT.GetConfig', self.mockGit.GetConfig).start() + scm_mock.GIT(self) 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('scm.GIT.FetchUpstreamTuple', return_value=('origin', 'refs/heads/main')).start() mock.patch('scm.GIT.CaptureStatus', @@ -864,7 +835,8 @@ class TestGitCl(unittest.TestCase): calls = [] if squash_mode in ('override_squash', 'override_nosquash'): - self.mockGit.config['gerrit.override-squash-uploads'] = ( + scm.GIT.SetConfig( + '', 'gerrit.override-squash-uploads', 'true' if squash_mode == 'override_squash' else 'false') if not git_footers.get_footer_change_id(description) and not squash: @@ -1244,12 +1216,12 @@ class TestGitCl(unittest.TestCase): 'gclient_utils.AskForData', lambda prompt: self._mocked_call('ask_for_data', prompt)).start() - self.mockGit.config['gerrit.host'] = 'true' - self.mockGit.config['branch.main.gerritissue'] = (str(issue) - if issue else None) - self.mockGit.config['remote.origin.url'] = ( - 'https://%s.googlesource.com/my/repo' % short_hostname) - self.mockGit.config['user.email'] = 'owner@example.com' + scm.GIT.SetConfig('', 'gerrit.host', 'true') + scm.GIT.SetConfig('', 'branch.main.gerritissue', + (str(issue) if issue else None)) + scm.GIT.SetConfig('', 'remote.origin.url', + f'https://{short_hostname}.googlesource.com/my/repo') + scm.GIT.SetConfig('', 'user.email', 'owner@example.com') self.calls = [] if squash_mode == "override_nosquash": @@ -1421,8 +1393,8 @@ class TestGitCl(unittest.TestCase): mockUploadAllPrecheck.return_value = (cls, True) upstream_gerrit_commit = 'upstream-commit' - self.mockGit.config[ - 'branch.upstream-branch.gerritsquashhash'] = upstream_gerrit_commit + scm.GIT.SetConfig('', 'branch.upstream-branch.gerritsquashhash', + upstream_gerrit_commit) reviewers = [] ccs = [] @@ -1730,13 +1702,13 @@ class TestGitCl(unittest.TestCase): # (so no LAST_UPLOAD_HASH_CONIFG_KEY) # Case 4: upstream2's last_upload is behind upstream3's base_commit - self.mockGit.config['branch.upstream2.%s' % - git_cl.LAST_UPLOAD_HASH_CONFIG_KEY] = 'commit2.3' + key = f'branch.upstream2.{git_cl.LAST_UPLOAD_HASH_CONFIG_KEY}' + scm.GIT.SetConfig('', key, 'commit2.3') mockIsAncestor.side_effect = [True] # Case 3: upstream1's last_upload matches upstream2's base_commit - self.mockGit.config['branch.upstream1.%s' % - git_cl.LAST_UPLOAD_HASH_CONFIG_KEY] = 'commit1.5' + key = f'branch.upstream1.{git_cl.LAST_UPLOAD_HASH_CONFIG_KEY}' + scm.GIT.SetConfig('', key, 'commit1.5') cls, cherry_pick = git_cl._UploadAllPrecheck(options, orig_args) self.assertFalse(cherry_pick) @@ -1858,8 +1830,8 @@ class TestGitCl(unittest.TestCase): mockGitGetBranchConfigValue.return_value = None # Case 5: current's base_commit is behind upstream3's last_upload. - self.mockGit.config['branch.upstream3.%s' % - git_cl.LAST_UPLOAD_HASH_CONFIG_KEY] = 'commit3.7' + key = f'branch.upstream3.{git_cl.LAST_UPLOAD_HASH_CONFIG_KEY}' + scm.GIT.SetConfig('', key, 'commit3.7') mockIsAncestor.side_effect = [False, True] with self.assertRaises(SystemExitMock): options = optparse.Values() @@ -1904,8 +1876,8 @@ class TestGitCl(unittest.TestCase): mockIsAncestor.return_value = True # Give upstream3 a last upload hash - self.mockGit.config['branch.upstream3.%s' % - git_cl.LAST_UPLOAD_HASH_CONFIG_KEY] = 'commit3.4' + key = f'branch.upstream3.{git_cl.LAST_UPLOAD_HASH_CONFIG_KEY}' + scm.GIT.SetConfig('', key, 'commit3.4') # end commits mockRunGit.return_value = 'commit4' @@ -2391,8 +2363,8 @@ class TestGitCl(unittest.TestCase): def _patch_common(self, git_short_host='chromium'): mock.patch('scm.GIT.ResolveCommit', return_value='deadbeef').start() - self.mockGit.config['remote.origin.url'] = ( - 'https://%s.googlesource.com/my/repo' % git_short_host) + scm.GIT.SetConfig('', 'remote.origin.url', + f'https://{git_short_host}.googlesource.com/my/repo') gerrit_util.GetChangeDetail.return_value = { 'current_revision': '7777777777', 'revisions': { @@ -2505,8 +2477,8 @@ class TestGitCl(unittest.TestCase): side_effect=gerrit_util.GerritError(404, '')) @mock.patch('sys.stderr', io.StringIO()) def test_patch_gerrit_not_exists(self, *_mocks): - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/my/repo') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/my/repo') with self.assertRaises(SystemExitMock): self.assertEqual(1, git_cl.main(['patch', '123456'])) self.assertEqual( @@ -2550,8 +2522,8 @@ class TestGitCl(unittest.TestCase): mock.patch( 'git_cl.gerrit_util.CookiesAuthenticator', CookiesAuthenticatorMockFactory(hosts_with_creds=auth)).start() - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/my/repo') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/my/repo') cl = git_cl.Changelist() cl.branch = 'main' cl.branchref = 'refs/heads/main' @@ -2594,12 +2566,12 @@ class TestGitCl(unittest.TestCase): self.assertIsNone(cl.EnsureAuthenticated(force=False)) def test_gerrit_ensure_authenticated_skipped(self): - self.mockGit.config['gerrit.skip-ensure-authenticated'] = 'true' + scm.GIT.SetConfig('', 'gerrit.skip-ensure-authenticated', 'true') cl = self._test_gerrit_ensure_authenticated_common(auth={}) self.assertIsNone(cl.EnsureAuthenticated(force=False)) def test_gerrit_ensure_authenticated_sso(self): - self.mockGit.config['remote.origin.url'] = 'sso://repo' + scm.GIT.SetConfig('', 'remote.origin.url', 'sso://repo') mock.patch( 'git_cl.gerrit_util.CookiesAuthenticator', @@ -2629,7 +2601,7 @@ class TestGitCl(unittest.TestCase): self.assertTrue('Bearer' in conn.req_headers['Authorization']) def test_gerrit_ensure_authenticated_non_https_sso(self): - self.mockGit.config['remote.origin.url'] = 'custom-scheme://repo' + scm.GIT.SetConfig('', 'remote.origin.url', 'custom-scheme://repo') self.calls = [ (('logging.warning', 'Ignoring branch %(branch)s with non-https remote ' @@ -2650,8 +2622,8 @@ class TestGitCl(unittest.TestCase): self.assertIsNone(cl.EnsureAuthenticated(force=False)) def test_gerrit_ensure_authenticated_non_url(self): - self.mockGit.config['remote.origin.url'] = ( - 'git@somehost.example:foo/bar.git') + scm.GIT.SetConfig('', 'remote.origin.url', + 'git@somehost.example:foo/bar.git') self.calls = [ (('logging.error', 'Remote "%(remote)s" for branch "%(branch)s" points to "%(url)s", ' @@ -2673,11 +2645,11 @@ class TestGitCl(unittest.TestCase): self.assertIsNone(cl.EnsureAuthenticated(force=False)) def _cmd_set_commit_gerrit_common(self, vote, notify=None): - self.mockGit.config['branch.main.gerritissue'] = '123' - self.mockGit.config['branch.main.gerritserver'] = ( - 'https://chromium-review.googlesource.com') - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/infra/infra') + scm.GIT.SetConfig('', 'branch.main.gerritissue', '123') + scm.GIT.SetConfig('', 'branch.main.gerritserver', + 'https://chromium-review.googlesource.com') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/infra/infra') self.calls = [ (('SetReview', 'chromium-review.googlesource.com', 'infra%2Finfra~123', None, { @@ -2732,8 +2704,8 @@ class TestGitCl(unittest.TestCase): self.assertEqual(git_cl.main(['set-close', '--issue', '1']), 0) def test_description(self): - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/my/repo') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/my/repo') gerrit_util.GetChangeDetail.return_value = { 'current_revision': 'sha1', 'revisions': { @@ -2783,7 +2755,7 @@ class TestGitCl(unittest.TestCase): UpdateDescription).start() mock.patch('git_cl.gclient_utils.RunEditor', RunEditor).start() - self.mockGit.config['branch.main.gerritissue'] = '123' + scm.GIT.SetConfig('', 'branch.main.gerritissue', '123') self.assertEqual(0, git_cl.main(['description'])) def test_description_does_not_append_bug_line_if_fixed_is_present(self): @@ -2803,7 +2775,7 @@ class TestGitCl(unittest.TestCase): lambda *args: current_desc).start() mock.patch('git_cl.gclient_utils.RunEditor', RunEditor).start() - self.mockGit.config['branch.main.gerritissue'] = '123' + scm.GIT.SetConfig('', 'branch.main.gerritissue', '123') self.assertEqual(0, git_cl.main(['description'])) def test_description_set_stdin(self): @@ -2950,20 +2922,20 @@ class TestGitCl(unittest.TestCase): 'archived/{issue}-{branch}'])) def test_cmd_issue_erase_existing(self): - self.mockGit.config['branch.main.gerritissue'] = '123' - self.mockGit.config['branch.main.gerritserver'] = ( - 'https://chromium-review.googlesource.com') + scm.GIT.SetConfig('', 'branch.main.gerritissue', '123') + scm.GIT.SetConfig('', 'branch.main.gerritserver', + 'https://chromium-review.googlesource.com') self.calls = [ ((['git', 'log', '-1', '--format=%B'], ), 'This is a description'), ] self.assertEqual(0, git_cl.main(['issue', '0'])) - self.assertNotIn('branch.main.gerritissue', self.mockGit.config) - self.assertNotIn('branch.main.gerritserver', self.mockGit.config) + self.assertIsNone(scm.GIT.GetConfig('root', 'branch.main.gerritissue')) + self.assertIsNone(scm.GIT.GetConfig('root', 'branch.main.gerritserver')) def test_cmd_issue_erase_existing_with_change_id(self): - self.mockGit.config['branch.main.gerritissue'] = '123' - self.mockGit.config['branch.main.gerritserver'] = ( - 'https://chromium-review.googlesource.com') + scm.GIT.SetConfig('', 'branch.main.gerritissue', '123') + scm.GIT.SetConfig('', 'branch.main.gerritserver', + 'https://chromium-review.googlesource.com') mock.patch( 'git_cl.Changelist.FetchDescription', lambda _: 'This is a description\n\nChange-Id: Ideadbeef').start() @@ -2974,15 +2946,15 @@ class TestGitCl(unittest.TestCase): 'This is a description\n'], ), ''), ] self.assertEqual(0, git_cl.main(['issue', '0'])) - self.assertNotIn('branch.main.gerritissue', self.mockGit.config) - self.assertNotIn('branch.main.gerritserver', self.mockGit.config) + self.assertIsNone(scm.GIT.GetConfig('root', 'branch.main.gerritissue')) + self.assertIsNone(scm.GIT.GetConfig('root', 'branch.main.gerritserver')) def test_cmd_issue_json(self): - self.mockGit.config['branch.main.gerritissue'] = '123' - self.mockGit.config['branch.main.gerritserver'] = ( - 'https://chromium-review.googlesource.com') - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/chromium/src') + scm.GIT.SetConfig('', 'branch.main.gerritissue', '123') + scm.GIT.SetConfig('', 'branch.main.gerritserver', + 'https://chromium-review.googlesource.com') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/chromium/src') self.calls = [( ( 'write_json', @@ -3046,9 +3018,9 @@ class TestGitCl(unittest.TestCase): cl._GerritCommitMsgHookCheck(offer_removal=True) def test_GerritCmdLand(self): - self.mockGit.config['branch.main.gerritsquashhash'] = 'deadbeaf' - self.mockGit.config['branch.main.gerritserver'] = ( - 'chromium-review.googlesource.com') + scm.GIT.SetConfig('', 'branch.main.gerritsquashhash', 'deadbeaf') + scm.GIT.SetConfig('', 'branch.main.gerritserver', + 'chromium-review.googlesource.com') self.calls += [ ((['git', 'diff', 'deadbeaf'], ), ''), # No diff. ] @@ -3222,9 +3194,9 @@ class TestGitCl(unittest.TestCase): sys.stdout.getvalue()) def test_git_cl_comment_add_gerrit(self): - self.mockGit.branchref = None - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/infra/infra') + git_new_branch.create_new_branch(None) # hits mock from scm_mock.GIT. + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/infra/infra') self.calls = [ (('SetReview', 'chromium-review.googlesource.com', 'infra%2Finfra~10', 'msg', None, None, None), None), @@ -3233,8 +3205,8 @@ class TestGitCl(unittest.TestCase): @mock.patch('git_cl.Changelist.GetBranch', return_value='foo') def test_git_cl_comments_fetch_gerrit(self, *_mocks): - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/infra/infra') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/infra/infra') gerrit_util.GetChangeDetail.return_value = { 'owner': { 'email': 'owner@example.com' @@ -3388,8 +3360,8 @@ class TestGitCl(unittest.TestCase): # git cl comments also fetches robot comments (which are considered a # type of autogenerated comment), and unlike other types of comments, # only robot comments from the latest patchset are shown. - self.mockGit.config['remote.origin.url'] = ( - 'https://x.googlesource.com/infra/infra') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://x.googlesource.com/infra/infra') gerrit_util.GetChangeDetail.return_value = { 'owner': { 'email': 'owner@example.com' @@ -3505,8 +3477,8 @@ class TestGitCl(unittest.TestCase): mock.patch('os.path.isdir', selective_os_path_isdir_mock).start() url = 'https://chromium.googlesource.com/my/repo' - self.mockGit.config['remote.origin.url'] = ('/cache/this-dir-exists') - self.mockGit.config['/cache/this-dir-exists:remote.origin.url'] = (url) + scm.GIT.SetConfig('', 'remote.origin.url', '/cache/this-dir-exists') + scm.GIT.SetConfig('/cache/this-dir-exists', 'remote.origin.url', url) self.calls = [ (('os.path.isdir', '/cache/this-dir-exists'), True), ] @@ -3526,8 +3498,8 @@ class TestGitCl(unittest.TestCase): mock.patch('logging.error', lambda *a: self._mocked_call('logging.error', *a)).start() - self.mockGit.config['remote.origin.url'] = ( - '/cache/this-dir-doesnt-exist') + scm.GIT.SetConfig('', 'remote.origin.url', + '/cache/this-dir-doesnt-exist') self.calls = [ (('os.path.isdir', '/cache/this-dir-doesnt-exist'), False), (('logging.error', @@ -3553,7 +3525,7 @@ class TestGitCl(unittest.TestCase): mock.patch('logging.error', lambda *a: self._mocked_call('logging.error', *a)).start() - self.mockGit.config['remote.origin.url'] = ('/cache/this-dir-exists') + scm.GIT.SetConfig('', 'remote.origin.url', '/cache/this-dir-exists') self.calls = [ (('os.path.isdir', '/cache/this-dir-exists'), True), (('logging.error', @@ -3570,8 +3542,8 @@ class TestGitCl(unittest.TestCase): self.assertIsNone(cl.GetRemoteUrl()) def test_gerrit_change_identifier_with_project(self): - self.mockGit.config['remote.origin.url'] = ( - 'https://chromium.googlesource.com/a/my/repo.git/') + scm.GIT.SetConfig('', 'remote.origin.url', + 'https://chromium.googlesource.com/a/my/repo.git/') cl = git_cl.Changelist(issue=123456) self.assertEqual(cl._GerritChangeIdentifier(), 'my%2Frepo~123456') @@ -3641,11 +3613,10 @@ class ChangelistTest(unittest.TestCase): return_value='project').start() mock.patch('sys.exit', side_effect=SystemExitMock).start() + scm_mock.GIT(self) + self.addCleanup(mock.patch.stopall) self.temp_count = 0 - self.mockGit = GitMocks() - mock.patch('scm.GIT.GetConfig', self.mockGit.GetConfig).start() - mock.patch('scm.GIT.SetConfig', self.mockGit.SetConfig).start() gerrit_util._Authenticator._resolved = None def testRunHook(self): @@ -4238,10 +4209,10 @@ class ChangelistTest(unittest.TestCase): cl.PostUploadUpdates(options, new_upload, '12345') mockSetPatchset.assert_called_once_with(3) self.assertEqual( - self.mockGit.config['root:branch.current-branch.gerritsquashhash'], + scm.GIT.GetConfig('root', 'branch.current-branch.gerritsquashhash'), new_upload.commit_to_push) self.assertEqual( - self.mockGit.config['root:branch.current-branch.last-upload-hash'], + scm.GIT.GetConfig('root', 'branch.current-branch.last-upload-hash'), new_upload.new_last_uploaded_commit) mockAddReviewers.assert_called_once_with('chromium', diff --git a/tests/git_common_test.py b/tests/git_common_test.py index 6aac4cbaa..786010031 100755 --- a/tests/git_common_test.py +++ b/tests/git_common_test.py @@ -458,7 +458,7 @@ class GitMutableFunctionsTest(git_test_utils.GitRepoReadWriteTestBase, []) self.repo.git('config', '--add', 'happy.derpies', 'cat') - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual( self.repo.run(self.gc.get_config_list, 'happy.derpies'), ['food', 'cat']) @@ -482,19 +482,19 @@ class GitMutableFunctionsTest(git_test_utils.GitRepoReadWriteTestBase, self.repo.git('config', 'depot-tools.upstream', 'catfood') - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual('catfood', self.repo.run(self.gc.root)) self.repo.git('config', '--add', 'core.fsmonitor', 'true') - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual(True, self.repo.run(self.gc.is_fsmonitor_enabled)) self.repo.git('config', '--add', 'core.fsmonitor', 't') - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual(False, self.repo.run(self.gc.is_fsmonitor_enabled)) self.repo.git('config', '--add', 'core.fsmonitor', 'false') - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual(False, self.repo.run(self.gc.is_fsmonitor_enabled)) def testRoot(self): @@ -659,7 +659,7 @@ class GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase, self.repo.run(self.gc.get_or_create_merge_base, 'branch_L', 'branch_K')) - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual( self.repo['B'], self.repo.run(self.gc.get_config, 'branch.branch_K.base')) @@ -680,7 +680,7 @@ class GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase, self.repo.run(self.gc.manual_merge_base, 'branch_K', self.repo['I'], 'branch_G') - self.gc.scm.GIT._clear_config(self.repo.repo_path) + self.gc.scm.GIT.drop_config_cache() self.assertEqual( self.repo['I'], self.repo.run(self.gc.get_or_create_merge_base, 'branch_K', diff --git a/tests/scm_mock.py b/tests/scm_mock.py new file mode 100644 index 000000000..9244b7f76 --- /dev/null +++ b/tests/scm_mock.py @@ -0,0 +1,54 @@ +# Copyright (c) 2024 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 os +import sys + +from typing import Dict, List, Optional +from unittest import mock +import unittest + +# This is to be able to import scm from the root of the repo. +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import scm + + +def GIT(test: unittest.TestCase, + config: Optional[Dict[str, List[str]]] = None, + branchref: Optional[str] = None): + """Installs fakes/mocks for scm.GIT so that: + + * Initial git config (local scope) is set to `config`. + * GetBranch will just return a fake branchname starting with the value of + branchref. + * git_new_branch.create_new_branch will be mocked to update the value + returned by GetBranch. + + NOTE: The dependency on git_new_branch.create_new_branch seems pretty + circular - this functionality should probably move to scm.GIT? + """ + _branchref = [branchref or 'refs/heads/main'] + + initial_state = {} + if config is not None: + initial_state['local'] = config + + def _newBranch(branchref): + _branchref[0] = branchref + + patches: List[mock._patch] = [ + mock.patch( + 'scm.GIT._new_config_state', + side_effect=lambda root: scm.GitConfigStateTest(initial_state)), + mock.patch('scm.GIT.GetBranchRef', + side_effect=lambda _root: _branchref[0]), + mock.patch('git_new_branch.create_new_branch', side_effect=_newBranch) + ] + + for p in patches: + p.start() + test.addCleanup(p.stop) + + test.addCleanup(scm.GIT.drop_config_cache) diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 9dd0b5a3f..5656e2ffb 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -206,7 +206,9 @@ class RealGitTest(fake_repos.FakeReposTestBase): self.assertEqual('', scm.GIT.GetConfig(self.cwd, key)) self.assertEqual('', scm.GIT.GetConfig(self.cwd, key, 'default-value')) - scm.GIT._clear_config(self.cwd) + # Clear the cache because we externally manipulate the git config with + # the subprocess call. + scm.GIT.drop_config_cache() subprocess.run(['git', 'config', key, 'line 1\nline 2\nline 3'], cwd=self.cwd) self.assertEqual('line 1\nline 2\nline 3',