Merge pull request #254 from mih/credsalad

New `memory_keyring` fixture

datalad_next/commands/tests/test_credentials.py

@@ -19,7 +19,6 @@
 from datalad_next.exceptions import IncompleteResultsError
 from datalad_next.utils import external_versions
 from datalad_next.tests.utils import (
-    MemoryKeyring,
     assert_in,
     assert_in_results,
     assert_raises,
@@ -74,12 +73,11 @@ def test_normalize_specs():
         assert_raises(ValueError, normalize_specs, error)
 
 
-def test_credentials():
+def test_credentials(memory_keyring):
     # we want all tests to bypass the actual system keyring
-    with patch('datalad.support.keyring_.keyring', MemoryKeyring()):
-        check_credentials_cli()
-        check_interactive_entry_set()
-        check_interactive_entry_get()
+    check_credentials_cli()
+    check_interactive_entry_set()
+    check_interactive_entry_get()
 
 
 def test_errorhandling_smoketest():
@@ -185,16 +183,16 @@ def test_result_renderer():
     ))
 
 
-def test_extreme_credential_name():
+def test_extreme_credential_name(memory_keyring):
     cred = Credentials()
     extreme = 'ΔЙקم๗あ |/;&%b5{}"'
-    with patch('datalad.support.keyring_.keyring', MemoryKeyring()):
-        assert_in_results(
-            cred('set',
-                 name=extreme,
-                 # use CLI style spec to exercise more code
-                 spec=[f'someprop={extreme}', f'secret={extreme}'],
-            ),
-            cred_someprop=extreme,
-            cred_secret=extreme,
-        )
+    assert_in_results(
+        cred(
+            'set',
+            name=extreme,
+            # use CLI style spec to exercise more code
+            spec=[f'someprop={extreme}', f'secret={extreme}'],
+        ),
+        cred_someprop=extreme,
+        cred_secret=extreme,
+    )

datalad_next/conftest.py

@@ -1 +1,48 @@
+import pytest
+
 from datalad.conftest import setup_package
+
+
+@pytest.fixture(autouse=False, scope="function")
+def memory_keyring():
+    """Patch keyring to temporarily use a backend that only stores in memory
+
+    No credential read or write actions will impact any existing credential
+    store of any configured backend.
+
+    The patched-in backend is yielded by the fixture. It offers a ``store``
+    attribute, which is a ``dict`` that uses keys of the pattern::
+
+        (datalad-<credential name>, <field name>)
+
+    and the associated secrets as values. For non-legacy credentials the
+    ``<field name>`` is uniformly ``'secret'``. For legacy credentials
+    other values are also used, including fields that are not actually
+    secrets.
+    """
+    import keyring
+    import keyring.backend
+
+    class MemoryKeyring(keyring.backend.KeyringBackend):
+        # high priority
+        priority = 1000
+
+        def __init__(self):
+            self.store = {}
+
+        def set_password(self, servicename, username, password):
+            self.store[(servicename, username)] = password
+
+        def get_password(self, servicename, username):
+            return self.store.get((servicename, username))
+
+        def delete_password(self, servicename, username):
+            del self.store[(servicename, username)]
+
+    old_backend = keyring.get_keyring()
+    new_backend = MemoryKeyring()
+    keyring.set_keyring(new_backend)
+
+    yield new_backend
+
+    keyring.set_keyring(old_backend)

datalad_next/credman/tests/test_credman.py

@@ -18,7 +18,6 @@
     _get_cred_cfg_var,
 )
 from datalad_next.tests.utils import (
-    MemoryKeyring,
     assert_in,
     assert_not_in,
     assert_raises,
@@ -31,13 +30,7 @@
 from datalad_next.utils import chpwd
 
 
-def test_credmanager():
-    # we want all tests to bypass the actual system keyring
-    with patch('datalad.support.keyring_.keyring', MemoryKeyring()):
-        check_credmanager()
-
-
-def check_credmanager():
+def test_credmanager(memory_keyring):
     cfg = ConfigManager()
     credman = CredentialManager(cfg)
     # doesn't work with thing air
@@ -172,13 +165,7 @@ def test_credman_local(path=None):
     credman.remove('notstupid')
 
 
-def test_query():
-    # we want all tests to bypass the actual system keyring
-    with patch('datalad.support.keyring_.keyring', MemoryKeyring()):
-        check_query()
-
-
-def check_query():
+def test_query(memory_keyring):
     cfg = ConfigManager()
     credman = CredentialManager(cfg)
     # set a bunch of credentials with a common realm AND timestamp
@@ -228,13 +215,7 @@ def test_credman_get():
     assert 'mysecret' == res['secret']
 
 
-def test_credman_obtain():
-    # we want all tests to bypass the actual system keyring
-    with patch('datalad.support.keyring_.keyring', MemoryKeyring()):
-        check_obtain()
-
-
-def check_obtain():
+def test_credman_obtain(memory_keyring):
     credman = CredentialManager(ConfigManager())
     # senseless, but valid call
     # could not possibly report a credential without any info
@@ -295,27 +276,27 @@ def check_obtain():
 """
 
 
-def test_legacy_credentials(tmp_path):
-    # we will use a dataset to host a legacy provider config
-    ds = Dataset(tmp_path).create(result_renderer='disabled')
-    provider_path = ds.pathobj / '.datalad' / 'providers' / 'mylegacycred.cfg'
-    provider_path.parent.mkdir(parents=True, exist_ok=True)
-    provider_path.write_text(legacy_provider_cfg)
+def test_legacy_credentials(memory_keyring, tmp_path):
     # - the legacy code will only ever pick up a dataset credential, when
     #   PWD is inside a dataset
     # - we want all tests to bypass the actual system keyring
     #   'datalad.downloaders.credentials.keyring_' is what the UserPassword
     #   credential will use to store the credential
     # - 'datalad.support.keyring_' is what credman uses
-    # - we need to make them one and the same thing
-    tmp_keyring = MemoryKeyring()
-    with chpwd(tmp_path), \
-            patch('datalad.downloaders.credentials.keyring_', tmp_keyring), \
-            patch('datalad.support.keyring_.keyring', tmp_keyring):
-        check_legacy_credentials(ds)
+    # - we need to make them one and the same thing, and the memory_keyring
+    #   fixture does this by replacing the keyring backend for the runtime
+    #   of the test
+    with chpwd(tmp_path):
+        check_legacy_credentials(memory_keyring, tmp_path)
+
 
+def check_legacy_credentials(memory_keyring, tmp_path):
+    # we will use a dataset to host a legacy provider config
+    ds = Dataset(tmp_path).create(result_renderer='disabled')
+    provider_path = ds.pathobj / '.datalad' / 'providers' / 'mylegacycred.cfg'
+    provider_path.parent.mkdir(parents=True, exist_ok=True)
+    provider_path.write_text(legacy_provider_cfg)
 
-def check_legacy_credentials(ds):
     # shortcut
     cname = 'credmanuniquetestcredentialsetup'
 
@@ -355,19 +336,19 @@ def check_legacy_credentials(ds):
         # the legacy backend, in order to keep old code working
         # with the old info
         # confirm starting point: legacy code keeps user in secret store
-        assert credman._keyring.entries[cname]['user'] == 'mike'
+        assert memory_keyring.store[(f'datalad-{cname}', 'user')] == 'mike'
         assert ds.config.get(f'datalad.credential.{cname}.user') is None
         credman.set(cname, **cred)
         # it remains there
-        assert credman._keyring.entries[cname]['user'] == 'mike'
+        assert memory_keyring.store[(f'datalad-{cname}', 'user')] == 'mike'
         # but is also migrated
         assert ds.config.get(f'datalad.credential.{cname}.user') == 'mike'
         # NOTE: This setup is not without problems. Users will update
         # a credential and will leave an outdated (half) as garbage.
         # however, I did not come up with a better approach that gradually
         # brings users over.
         credman.set(cname, user='newmike', secret='othersecret')
-        assert credman._keyring.entries[cname]['user'] == 'mike'
+        assert memory_keyring.store[(f'datalad-{cname}', 'user')] == 'mike'
         # finally check that the update is reported now
         cred = credman.get(cname)
         assert cred['user'] == 'newmike'

datalad_next/tests/utils.py

@@ -40,7 +40,6 @@
 )
 from datalad.tests.test_utils_testrepos import BasicGitTestRepo
 from datalad.cli.tests.test_main import run_main
-from datalad.support.keyring_ import MemoryKeyring
 from datalad_next.utils import (
     CredentialManager,
     optional_args,