Browse Source

[scm] Add key canonicalization to scm's CachedGitConfigState.

This will allow `git cl` to be much more consistent with regards to
setting and reading git config key values in both production and
testing.

R=ayatane, yiwzhang

Change-Id: I2f1f1c5c6aaab12e2e9dbcf36c181244706cd4a8
Bug: 357688295
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5762199
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Robert Iannucci 1 year ago
parent
commit
283661a975
4 changed files with 373 additions and 83 deletions
  1. 78 11
      scm.py
  2. 6 6
      tests/git_auth_test.py
  3. 6 0
      tests/scm_mock.py
  4. 283 66
      tests/scm_unittest.py

+ 78 - 11
scm.py

@@ -14,6 +14,7 @@ import re
 import threading
 
 from collections import defaultdict
+from itertools import chain
 from typing import Collection, Iterable, Iterator, Literal, Dict
 from typing import Optional, Sequence, Mapping
 
@@ -64,6 +65,10 @@ class GitConfigStateBase(metaclass=abc.ABCMeta):
 
     In GitConfigStateTest, this is modeled using a set of GitConfigScope-indexed
     dictionaries.
+
+    Implementations MUST ensure that all keys returned in load_config are
+    already canonicalized, and implementations MUST accept non-canonical keys to
+    set_* and unset_* methods.
     """
 
     @abc.abstractmethod
@@ -72,6 +77,9 @@ class GitConfigStateBase(metaclass=abc.ABCMeta):
         observable.
 
         The caller must not mutate the returned value.
+
+        Implementations MUST ensure that all keys returned in load_config are
+        already canonicalized.
         """
 
     @abc.abstractmethod
@@ -82,6 +90,8 @@ class GitConfigStateBase(metaclass=abc.ABCMeta):
 
         If `append` is True, this should add an additional value to the existing
         `key`, if any.
+
+        Implementations MUST accept non-canonical `key` values.
         """
 
     @abc.abstractmethod
@@ -95,6 +105,8 @@ class GitConfigStateBase(metaclass=abc.ABCMeta):
 
         TODO: Make value_pattern an re.Pattern. This wasn't done at the time of
         this refactor to keep the refactor small.
+
+        Implementations MUST accept non-canonical `key` values.
         """
 
     @abc.abstractmethod
@@ -107,6 +119,8 @@ class GitConfigStateBase(metaclass=abc.ABCMeta):
 
         If `key` is multi-valued in this scope, this must raise
         GitConfigUnsetMultipleValues with `key` and `scope`.
+
+        Implementations MUST accept non-canonical `key` values.
         """
 
     @abc.abstractmethod
@@ -122,6 +136,8 @@ class GitConfigStateBase(metaclass=abc.ABCMeta):
 
         TODO: Make value_pattern an re.Pattern. This wasn't done at the time of
         this refactor to keep the refactor small.
+
+        Implementations MUST accept non-canonical `key` values.
         """
 
 
@@ -153,6 +169,33 @@ class GitConfigUnknownScope(ValueError):
         super().__init__(f'Unknown git config scope {scope!r}.')
 
 
+class GitConfigInvalidKey(ValueError):
+
+    def __init__(self, key: str) -> None:
+        super().__init__(
+            f'Invalid git config key {key!r}: does not contain a section.')
+
+
+def canonicalize_git_config_key(key: str) -> str:
+    """Returns the canonicalized form of `key` for git config.
+
+    Git config internally canonicalizes keys (i.e. for
+    'section.subsection.variable', both 'section' and 'variable' will be
+    lowercased, but 'subsection' will not).
+
+    This also normalizes keys in the form 'section.variable' (both 'section' and
+    'variable' will be lowercased).
+    """
+    sections = key.split('.')
+    if len(sections) >= 3:
+        return '.'.join(
+            chain((sections[0].lower(), ), sections[1:-1],
+                  (sections[-1].lower(), )))
+    if len(sections) == 2:
+        return '.'.join((sections[0].lower(), sections[1].lower()))
+    raise GitConfigInvalidKey(key)
+
+
 class CachedGitConfigState(object):
     """This represents the observable git configuration state for a given
     repository (whose top-level path is `root`).
@@ -181,6 +224,8 @@ class CachedGitConfigState(object):
 
     def _maybe_load_config(self) -> GitFlatConfigData:
         if self._config is None:
+            # NOTE: Implementations of self._impl must already ensure that all
+            # keys are canonicalized.
             self._config = self._impl.load_config()
         return self._config
 
@@ -195,6 +240,7 @@ class CachedGitConfigState(object):
 
         If `key` is missing, returns default.
         """
+        key = canonicalize_git_config_key(key)
         values = self._maybe_load_config().get(key, None)
         if not values:
             return default
@@ -211,24 +257,30 @@ class CachedGitConfigState(object):
 
     def GetConfigList(self, key: str) -> list[str]:
         """Returns all values of `key` as a list of strings."""
-        return list(self._maybe_load_config().get(key, []))
+        key = canonicalize_git_config_key(key)
+        return list(self._maybe_load_config().get(key, ()))
 
     def YieldConfigRegexp(self,
-                          pattern: Optional[str]) -> Iterable[tuple[str, str]]:
+                          pattern: Optional[str] = None
+                          ) -> 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.
 
-        If pattern is None, this returns all config items.
+        If `pattern` is None, this returns all config items.
+
+        Note that `pattern` is always matched against the canonicalized key
+        value (i.e. for 'section.[subsection.]variable', both 'section' and
+        'variable' will be lowercased, but 'subsection', if present, will not).
         """
         if pattern is None:
             pred = lambda _: True
         else:
             pred = re.compile(pattern).match
-        for name, values in sorted(self._maybe_load_config().items()):
-            if pred(name):
+        for key, values in sorted(self._maybe_load_config().items()):
+            if pred(key):
                 for value in values:
-                    yield name, value
+                    yield key, value
 
     def SetConfig(self,
                   key,
@@ -316,6 +368,7 @@ class GitConfigStateReal(GitConfigStateBase):
         self.root = root
 
     def load_config(self) -> GitFlatConfigData:
+        # NOTE: `git config --list` already canonicalizes keys.
         try:
             rawConfig = GIT.Capture(['config', '--list', '-z'],
                                     cwd=self.root,
@@ -335,6 +388,7 @@ class GitConfigStateReal(GitConfigStateBase):
 
     def set_config(self, key: str, value: str, *, append: bool,
                    scope: GitConfigScope):
+        # NOTE: `git config` already canonicalizes key.
         args = ['config', f'--{scope}', key, value]
         if append:
             args.append('--add')
@@ -342,6 +396,7 @@ class GitConfigStateReal(GitConfigStateBase):
 
     def set_config_multi(self, key: str, value: str, *,
                          value_pattern: Optional[str], scope: GitConfigScope):
+        # NOTE: `git config` already canonicalizes key.
         args = ['config', f'--{scope}', '--replace-all', key, value]
         if value_pattern is not None:
             args.append(value_pattern)
@@ -349,6 +404,7 @@ class GitConfigStateReal(GitConfigStateBase):
 
     def unset_config(self, key: str, *, scope: GitConfigScope,
                      missing_ok: bool):
+        # NOTE: `git config` already canonicalizes key.
         accepted_retcodes = (0, 5) if missing_ok else (0, )
         try:
             GIT.Capture(['config', f'--{scope}', '--unset', key],
@@ -363,6 +419,7 @@ class GitConfigStateReal(GitConfigStateBase):
 
     def unset_config_multi(self, key: str, *, value_pattern: Optional[str],
                            scope: GitConfigScope, missing_ok: bool):
+        # NOTE: `git config` already canonicalizes key.
         accepted_retcodes = (0, 5) if missing_ok else (0, )
         args = ['config', f'--{scope}', '--unset-all', key]
         if value_pattern is not None:
@@ -393,6 +450,9 @@ class GitConfigStateTest(GitConfigStateBase):
         """Initializes a new (local, worktree) config state, with a reference to
         a single global `global` state and an optional immutable `system` state.
 
+        All keys in global_state, system_state, local_state and worktree_state
+        MUST already be canonicalized with canonicalize_key().
+
         The caller must supply a single shared Lock, plus a mutable reference to
         the global-state dictionary.
 
@@ -456,19 +516,24 @@ class GitConfigStateTest(GitConfigStateBase):
 
     def set_config(self, key: str, value: str, *, append: bool,
                    scope: GitConfigScope):
+        key = canonicalize_git_config_key(key)
         with self._editable_scope(scope) as cfg:
             cur = cfg.get(key)
-            if cur is None or len(cur) == 1:
-                if append:
-                    cfg[key] = (cur or []) + [value]
-                else:
-                    cfg[key] = [value]
+            if cur is None:
+                cfg[key] = [value]
+                return
+            if append:
+                cfg[key] = cur + [value]
+                return
+            if len(cur) == 1:
+                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, *,
                          value_pattern: Optional[str], scope: GitConfigScope):
+        key = canonicalize_git_config_key(key)
         with self._editable_scope(scope) as cfg:
             cur = cfg.get(key)
             if value_pattern is None or cur is None:
@@ -493,6 +558,7 @@ class GitConfigStateTest(GitConfigStateBase):
 
     def unset_config(self, key: str, *, scope: GitConfigScope,
                      missing_ok: bool):
+        key = canonicalize_git_config_key(key)
         with self._editable_scope(scope) as cfg:
             cur = cfg.get(key)
             if cur is None:
@@ -506,6 +572,7 @@ class GitConfigStateTest(GitConfigStateBase):
 
     def unset_config_multi(self, key: str, *, value_pattern: Optional[str],
                            scope: GitConfigScope, missing_ok: bool):
+        key = canonicalize_git_config_key(key)
         with self._editable_scope(scope) as cfg:
             cur = cfg.get(key)
             if cur is None:

+ 6 - 6
tests/git_auth_test.py

@@ -32,7 +32,7 @@ class TestConfigChanger(unittest.TestCase):
             '/some/fake/dir': {
                 'credential.https://chromium.googlesource.com/.helper':
                 ['', 'luci'],
-                'http.cookieFile': [''],
+                'http.cookiefile': [''],
             },
         }
         self.assertEqual(scm.GIT._dump_config_state(), want)
@@ -46,9 +46,9 @@ class TestConfigChanger(unittest.TestCase):
         want = {
             '/some/fake/dir': {
                 'protocol.sso.allow': ['always'],
-                'url.sso://chromium/.insteadOf':
+                'url.sso://chromium/.insteadof':
                 ['https://chromium.googlesource.com/'],
-                'http.cookieFile': [''],
+                'http.cookiefile': [''],
             },
         }
         self.assertEqual(scm.GIT._dump_config_state(), want)
@@ -79,7 +79,7 @@ class TestConfigChanger(unittest.TestCase):
             '/some/fake/dir': {
                 'credential.https://chromium.googlesource.com/.helper':
                 ['', 'luci'],
-                'http.cookieFile': [''],
+                'http.cookiefile': [''],
             },
         }
         self.assertEqual(scm.GIT._dump_config_state(), want)
@@ -98,9 +98,9 @@ class TestConfigChanger(unittest.TestCase):
         want = {
             '/some/fake/dir': {
                 'protocol.sso.allow': ['always'],
-                'url.sso://chromium/.insteadOf':
+                'url.sso://chromium/.insteadof':
                 ['https://chromium.googlesource.com/'],
-                'http.cookieFile': [''],
+                'http.cookiefile': [''],
             },
         }
         self.assertEqual(scm.GIT._dump_config_state(), want)

+ 6 - 0
tests/scm_mock.py

@@ -36,6 +36,12 @@ def GIT(test: unittest.TestCase,
     # TODO - add `system_config` - this will be configuration which exists at
     # the 'system installation' level and is immutable.
 
+    if config:
+        config = {
+            scm.canonicalize_git_config_key(k): v
+            for k, v in config.items()
+        }
+
     _branchref = [branchref or 'refs/heads/main']
 
     global_lock = threading.Lock()

+ 283 - 66
tests/scm_unittest.py

@@ -452,222 +452,439 @@ class GitConfigStateTestTest(unittest.TestCase):
         self.assertDictEqual(gs, {})
         self.assertDictEqual(m.load_config(), {})
 
-        gs['key'] = ['override']
-        self.assertDictEqual(m.load_config(), {'key': ['override']})
+        gs['section.key'] = ['override']
+        self.assertDictEqual(m.load_config(), {'section.key': ['override']})
 
     def test_construction_global(self):
-        m, gs = self._make(global_state={'key': ['global']})
-        self.assertDictEqual(gs, {'key': ['global']})
-        self.assertDictEqual(m.load_config(), {'key': ['global']})
+        m, gs = self._make(global_state={'section.key': ['global']})
+        self.assertDictEqual(gs, {'section.key': ['global']})
+        self.assertDictEqual(m.load_config(), {'section.key': ['global']})
 
-        gs['key'] = ['override']
-        self.assertDictEqual(m.load_config(), {'key': ['override']})
+        gs['section.key'] = ['override']
+        self.assertDictEqual(m.load_config(), {'section.key': ['override']})
 
     def test_construction_system(self):
         m, gs = self._make(
-            global_state={'key': ['global']},
-            system_state={'key': ['system']},
+            global_state={'section.key': ['global']},
+            system_state={'section.key': ['system']},
         )
-        self.assertDictEqual(gs, {'key': ['global']})
-        self.assertDictEqual(m.load_config(), {'key': ['system', 'global']})
+        self.assertDictEqual(gs, {'section.key': ['global']})
+        self.assertDictEqual(m.load_config(),
+                             {'section.key': ['system', 'global']})
 
-        gs['key'] = ['override']
-        self.assertDictEqual(m.load_config(), {'key': ['system', 'override']})
+        gs['section.key'] = ['override']
+        self.assertDictEqual(m.load_config(),
+                             {'section.key': ['system', 'override']})
 
     def test_construction_local(self):
         m, gs = self._make(
-            global_state={'key': ['global']},
-            system_state={'key': ['system']},
-            local_state={'key': ['local']},
+            global_state={'section.key': ['global']},
+            system_state={'section.key': ['system']},
+            local_state={'section.key': ['local']},
         )
-        self.assertDictEqual(gs, {'key': ['global']})
+        self.assertDictEqual(gs, {'section.key': ['global']})
         self.assertDictEqual(m.load_config(), {
-            'key': ['system', 'global', 'local'],
+            'section.key': ['system', 'global', 'local'],
         })
 
-        gs['key'] = ['override']
+        gs['section.key'] = ['override']
         self.assertDictEqual(m.load_config(), {
-            'key': ['system', 'override', 'local'],
+            'section.key': ['system', 'override', 'local'],
         })
 
     def test_construction_worktree(self):
         m, gs = self._make(
-            global_state={'key': ['global']},
-            system_state={'key': ['system']},
-            local_state={'key': ['local']},
-            worktree_state={'key': ['worktree']},
+            global_state={'section.key': ['global']},
+            system_state={'section.key': ['system']},
+            local_state={'section.key': ['local']},
+            worktree_state={'section.key': ['worktree']},
         )
-        self.assertDictEqual(gs, {'key': ['global']})
+        self.assertDictEqual(gs, {'section.key': ['global']})
         self.assertDictEqual(m.load_config(), {
-            'key': ['system', 'global', 'local', 'worktree'],
+            'section.key': ['system', 'global', 'local', 'worktree'],
         })
 
-        gs['key'] = ['override']
+        gs['section.key'] = ['override']
         self.assertDictEqual(m.load_config(), {
-            'key': ['system', 'override', 'local', 'worktree'],
+            'section.key': ['system', 'override', 'local', 'worktree'],
         })
 
     def test_set_config_system(self):
         m, _ = self._make()
 
         with self.assertRaises(scm.GitConfigUneditableScope):
-            m.set_config('key', 'new_global', append=False, scope='system')
+            m.set_config('section.key',
+                         'new_global',
+                         append=False,
+                         scope='system')
 
-    def test_set_config_unkown(self):
+    def test_set_config_unknown(self):
         m, _ = self._make()
 
         with self.assertRaises(scm.GitConfigUnknownScope):
-            m.set_config('key', 'new_global', append=False, scope='meepmorp')
+            m.set_config('section.key',
+                         'new_global',
+                         append=False,
+                         scope='meepmorp')
+
+    def test_set_config_global_append_empty(self):
+        m, gs = self._make()
+        self.assertDictEqual(gs, {})
+        self.assertDictEqual(m.load_config(), {})
+
+        m.set_config('section.key', 'new_global', append=True, scope='global')
+        self.assertDictEqual(m.load_config(), {
+            'section.key': ['new_global'],
+        })
 
     def test_set_config_global(self):
         m, gs = self._make()
         self.assertDictEqual(gs, {})
         self.assertDictEqual(m.load_config(), {})
 
-        m.set_config('key', 'new_global', append=False, scope='global')
+        m.set_config('section.key', 'new_global', append=False, scope='global')
         self.assertDictEqual(m.load_config(), {
-            'key': ['new_global'],
+            'section.key': ['new_global'],
         })
 
-        m.set_config('key', 'new_global2', append=True, scope='global')
+        m.set_config('section.key', 'new_global2', append=True, scope='global')
         self.assertDictEqual(m.load_config(), {
-            'key': ['new_global', 'new_global2'],
+            'section.key': ['new_global', 'new_global2'],
         })
 
         self.assertDictEqual(gs, {
-            'key': ['new_global', 'new_global2'],
+            'section.key': ['new_global', 'new_global2'],
         })
 
     def test_set_config_multi_global(self):
         m, gs = self._make(global_state={
-            'key': ['1', '2'],
+            'section.key': ['1', '2'],
         })
 
-        m.set_config_multi('key',
+        m.set_config_multi('section.key',
                            'new_global',
                            value_pattern=None,
                            scope='global')
         self.assertDictEqual(m.load_config(), {
-            'key': ['new_global'],
+            'section.key': ['new_global'],
         })
 
         self.assertDictEqual(gs, {
-            'key': ['new_global'],
+            'section.key': ['new_global'],
         })
 
-        m.set_config_multi('other',
+        m.set_config_multi('othersection.key',
                            'newval',
                            value_pattern=None,
                            scope='global')
         self.assertDictEqual(m.load_config(), {
-            'key': ['new_global'],
-            'other': ['newval'],
+            'section.key': ['new_global'],
+            'othersection.key': ['newval'],
         })
 
         self.assertDictEqual(gs, {
-            'key': ['new_global'],
-            'other': ['newval'],
+            'section.key': ['new_global'],
+            'othersection.key': ['newval'],
         })
 
     def test_set_config_multi_global_pattern(self):
         m, _ = self._make(global_state={
-            'key': ['1', '1', '2', '2', '2', '3'],
+            'section.key': ['1', '1', '2', '2', '2', '3'],
         })
 
-        m.set_config_multi('key',
+        m.set_config_multi('section.key',
                            'new_global',
                            value_pattern='2',
                            scope='global')
         self.assertDictEqual(m.load_config(), {
-            'key': ['1', '1', 'new_global', '3'],
+            'section.key': ['1', '1', 'new_global', '3'],
         })
 
-        m.set_config_multi('key',
+        m.set_config_multi('section.key',
                            'additional',
                            value_pattern='narp',
                            scope='global')
         self.assertDictEqual(m.load_config(), {
-            'key': ['1', '1', 'new_global', '3', 'additional'],
+            'section.key': ['1', '1', 'new_global', '3', 'additional'],
         })
 
     def test_unset_config_global(self):
         m, _ = self._make(global_state={
-            'key': ['someval'],
+            'section.key': ['someval'],
         })
 
-        m.unset_config('key', scope='global', missing_ok=False)
+        m.unset_config('section.key', scope='global', missing_ok=False)
         self.assertDictEqual(m.load_config(), {})
 
         with self.assertRaises(scm.GitConfigUnsetMissingValue):
-            m.unset_config('key', scope='global', missing_ok=False)
+            m.unset_config('section.key', scope='global', missing_ok=False)
 
         self.assertDictEqual(m.load_config(), {})
 
-        m.unset_config('key', scope='global', missing_ok=True)
+        m.unset_config('section.key', scope='global', missing_ok=True)
         self.assertDictEqual(m.load_config(), {})
 
     def test_unset_config_global_extra(self):
         m, _ = self._make(global_state={
-            'key': ['someval'],
+            'section.key': ['someval'],
             'extra': ['another'],
         })
 
-        m.unset_config('key', scope='global', missing_ok=False)
+        m.unset_config('section.key', scope='global', missing_ok=False)
         self.assertDictEqual(m.load_config(), {
             'extra': ['another'],
         })
 
         with self.assertRaises(scm.GitConfigUnsetMissingValue):
-            m.unset_config('key', scope='global', missing_ok=False)
+            m.unset_config('section.key', scope='global', missing_ok=False)
 
         self.assertDictEqual(m.load_config(), {
             'extra': ['another'],
         })
 
-        m.unset_config('key', scope='global', missing_ok=True)
+        m.unset_config('section.key', scope='global', missing_ok=True)
         self.assertDictEqual(m.load_config(), {
             'extra': ['another'],
         })
 
     def test_unset_config_global_multi(self):
         m, _ = self._make(global_state={
-            'key': ['1', '2'],
+            'section.key': ['1', '2'],
         })
 
         with self.assertRaises(scm.GitConfigUnsetMultipleValues):
-            m.unset_config('key', scope='global', missing_ok=True)
+            m.unset_config('section.key', scope='global', missing_ok=True)
 
     def test_unset_config_multi_global(self):
         m, _ = self._make(global_state={
-            'key': ['1', '2'],
+            'section.key': ['1', '2'],
         })
 
-        m.unset_config_multi('key',
+        m.unset_config_multi('section.key',
                              value_pattern=None,
                              scope='global',
                              missing_ok=False)
         self.assertDictEqual(m.load_config(), {})
 
         with self.assertRaises(scm.GitConfigUnsetMissingValue):
-            m.unset_config_multi('key',
+            m.unset_config_multi('section.key',
                                  value_pattern=None,
                                  scope='global',
                                  missing_ok=False)
 
     def test_unset_config_multi_global_pattern(self):
         m, _ = self._make(global_state={
-            'key': ['1', '2', '3', '1', '2'],
+            'section.key': ['1', '2', '3', '1', '2'],
         })
 
-        m.unset_config_multi('key',
+        m.unset_config_multi('section.key',
                              value_pattern='2',
                              scope='global',
                              missing_ok=False)
         self.assertDictEqual(m.load_config(), {
-            'key': ['1', '3', '1'],
+            'section.key': ['1', '3', '1'],
         })
 
 
+class CanonicalizeGitConfigKeyTest(unittest.TestCase):
+
+    def setUp(self) -> None:
+        self.ck = scm.canonicalize_git_config_key
+        return super().setUp()
+
+    def test_many(self):
+        self.assertEqual(self.ck("URL.https://SoMeThInG.example.com.INSTEADOF"),
+                         "url.https://SoMeThInG.example.com.insteadof")
+
+    def test_three(self):
+        self.assertEqual(self.ck("A.B.C"), "a.B.c")
+        self.assertEqual(self.ck("a.B.C"), "a.B.c")
+        self.assertEqual(self.ck("a.b.C"), "a.b.c")
+
+    def test_two(self):
+        self.assertEqual(self.ck("A.B"), "a.b")
+        self.assertEqual(self.ck("a.B"), "a.b")
+        self.assertEqual(self.ck("a.b"), "a.b")
+
+    def test_one(self):
+        with self.assertRaises(scm.GitConfigInvalidKey):
+            self.ck("KEY")
+
+    def test_zero(self):
+        with self.assertRaises(scm.GitConfigInvalidKey):
+            self.ck("")
+
+
+class CachedGitConfigStateTest(unittest.TestCase):
+
+    @staticmethod
+    def _make():
+        return scm.CachedGitConfigState(
+            scm.GitConfigStateTest(threading.Lock(), {}))
+
+    def test_empty(self):
+        gcs = self._make()
+
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [])
+
+    def test_set_single(self):
+        gcs = self._make()
+
+        gcs.SetConfig('SECTION.VARIABLE', 'value')
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'value'),
+        ])
+
+    def test_set_append(self):
+        gcs = self._make()
+
+        gcs.SetConfig('SECTION.VARIABLE', 'value')
+        gcs.SetConfig('SeCtIoN.vArIaBLe', 'value2', append=True)
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'value'),
+            ('section.variable', 'value2'),
+        ])
+
+    def test_set_global(self):
+        gcs = self._make()
+
+        gcs.SetConfig('SECTION.VARIABLE', 'value')
+        gcs.SetConfig('SeCtIoN.vArIaBLe', 'value2', append=True)
+
+        gcs.SetConfig('SeCtIoN.vArIaBLe', 'gvalue', scope='global')
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'gvalue'),
+            ('section.variable', 'value'),
+            ('section.variable', 'value2'),
+        ])
+
+    def test_unset_multi_global(self):
+        gcs = self._make()
+
+        gcs.SetConfig('SECTION.VARIABLE', 'value')
+        gcs.SetConfig('SeCtIoN.vArIaBLe', 'value2', append=True)
+        gcs.SetConfig('SeCtIoN.vArIaBLe', 'gvalue', scope='global')
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'gvalue'),
+            ('section.variable', 'value'),
+            ('section.variable', 'value2'),
+        ])
+
+        gcs.SetConfig('section.variable', None, modify_all=True)
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'gvalue'),
+        ])
+
+    def test_errors(self):
+        gcs = self._make()
+
+        with self.assertRaises(scm.GitConfigInvalidKey):
+            gcs.SetConfig('key', 'value')
+
+        with self.assertRaises(scm.GitConfigUnknownScope):
+            gcs.SetConfig('section.variable', 'value', scope='dude')
+
+        with self.assertRaises(scm.GitConfigUneditableScope):
+            gcs.SetConfig('section.variable', 'value', scope='system')
+
+        with self.assertRaisesRegex(ValueError,
+                                    'value_pattern.*modify_all.*invalid'):
+            gcs.SetConfig('section.variable',
+                          'value',
+                          value_pattern='hi',
+                          modify_all=False)
+
+        with self.assertRaisesRegex(ValueError,
+                                    'value_pattern.*append.*invalid'):
+            gcs.SetConfig('section.variable',
+                          'value',
+                          value_pattern='hi',
+                          modify_all=True,
+                          append=True)
+
+    def test_set_pattern(self):
+        gcs = self._make()
+
+        gcs.SetConfig('section.variable', 'value', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value', append=True)
+
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'value'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value'),
+        ])
+
+        gcs.SetConfig('section.variable',
+                      'poof',
+                      value_pattern='.*_bleem',
+                      modify_all=True)
+
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'value'),
+            ('section.variable', 'poof'),
+            ('section.variable', 'value'),
+        ])
+
+    def test_set_all(self):
+        gcs = self._make()
+
+        gcs.SetConfig('section.variable', 'value', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+        gcs.SetConfig('section.variable', 'value', append=True)
+
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'value'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'value'),
+        ])
+
+        gcs.SetConfig('section.variable', 'poof', modify_all=True)
+
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'poof'),
+        ])
+
+    def test_get_config(self):
+        gcs = self._make()
+
+        gcs.SetConfig('section.variable', 'value', append=True)
+        gcs.SetConfig('section.variable', 'value_bleem', append=True)
+
+        self.assertEqual(gcs.GetConfig('section.varIABLE'), 'value_bleem')
+        self.assertEqual(gcs.GetConfigBool('section.varIABLE'), False)
+
+        self.assertEqual(gcs.GetConfig('section.noexist'), None)
+        self.assertEqual(gcs.GetConfig('section.noexist', 'dflt'), 'dflt')
+
+        gcs.SetConfig('section.variable', 'true', append=True)
+        self.assertEqual(gcs.GetConfigBool('section.varIABLE'), True)
+
+        self.assertListEqual(list(gcs.YieldConfigRegexp()), [
+            ('section.variable', 'value'),
+            ('section.variable', 'value_bleem'),
+            ('section.variable', 'true'),
+        ])
+
+        self.assertListEqual(gcs.GetConfigList('seCTIon.vARIable'), [
+            'value',
+            'value_bleem',
+            'true',
+        ])
+
+
 if __name__ == '__main__':
     if '-v' in sys.argv:
         logging.basicConfig(level=logging.DEBUG)