Fix create_remotes on GitHub and make the API tests configurable

.github/ISSUE_TEMPLATE/release_checklist.md

@@ -26,7 +26,8 @@ The following checklist describes the steps to execute sequentially for creating
   - `cadetrdm/__init__.py`
   - `.zenodo.json` (two places: `title` and `version`)
 - [ ] Update `.zenodo.json` if authorship or other metadata changed.
-- [ ] Run the tests that CI does not run, which need network access and API tokens:
+- [ ] Run the tests that CI does not run, which need network access and API tokens
+  (see *Tests* in `CONTRIBUTING.md` for the required environment variables and tokens):
   ```bash
   pytest tests -m "slow or server_api"
   ```

CONTRIBUTING.md

@@ -96,13 +96,25 @@ CI also measures test coverage with pytest-cov and uploads it to [Codecov](https
 To see the coverage locally, add `--cov=cadetrdm --cov-report=term-missing` to the pytest call.
 The marked subsets require credentials or network access and are expected to be run deliberately, not by default.
 
-The `server_api` tests create and delete repositories through the GitLab and GitHub APIs.
-They read a Personal Access Token from the Python keyring, stored under the URL of the instance and your username:
+The `server_api` tests create and delete repositories through the GitLab and GitHub APIs, using your own accounts.
+They need, for each host you want to test:
+
+- the namespace (user, group or organization) to create the test repositories in, set as an environment variable,
+- a Personal Access Token in the Python keyring, stored under the URL of the host and a username,
+- SSH access to the host, since the tests clone and push over SSH.
 
 ```bash
-keyring set "https://jugit.fz-juelich.de/" <username>
+export CADET_RDM_TEST_GITLAB_NAMESPACE=<gitlab user or group>
+export CADET_RDM_TEST_GITHUB_NAMESPACE=<github user or organization>
+keyring set "https://jugit.fz-juelich.de/" <gitlab username>
+keyring set "https://github.com/" <github username>
+pytest tests -m server_api
 ```
 
+`CADET_RDM_TEST_GITLAB_URL` selects another GitLab instance (default: `https://jugit.fz-juelich.de/`), and `CADET_RDM_TEST_GITLAB_USERNAME` and `CADET_RDM_TEST_GITHUB_USERNAME` set the keyring usernames if they differ from the namespaces.
+Tests for a host without a namespace are skipped.
+Every test run uses repositories with a random name and deletes them afterwards.
+
 On GitHub, create a fine-grained token with
 
 - *Repository access* set to *All repositories*, and

cadetrdm/remote_integration.py

@@ -53,10 +53,21 @@ def delete_remote(self, url: str, namespace: str, name: str, username: str) -> N
         """Delete a remote repository."""
         return
 
+    @staticmethod
+    @abstractmethod
+    def ssh_url(response: Any) -> str:
+        """Return the SSH URL of a repository returned by create_remote()."""
+        return
+
 
 class GitLabRemote(Remote):
     """Remote repositories on a GitLab instance."""
 
+    @staticmethod
+    def ssh_url(response: Any) -> str:
+        """Return the SSH URL of a GitLab project."""
+        return response.ssh_url_to_repo
+
     @property
     def url_fallbacks(self) -> list[str]:
         """Keyring service names tried after the instance URL."""
@@ -132,13 +143,13 @@ def delete_remote(self, url: str, namespace: str, name: str, username: str) -> N
         token = self.load_token([url] + self.url_fallbacks, username)
         gl = gitlab.Gitlab(url, private_token=token)
 
-        potential_projects = gl.projects.list(get_all=True, search=[namespace, name])
+        potential_projects = gl.projects.list(get_all=True, search=name)
 
         for project in potential_projects:
             if project.name != name:
-                pass
-            if project.namespace["name"] != namespace:
-                pass
+                continue
+            if project.namespace["full_path"].lower() != namespace.lower():
+                continue
 
             gl.projects.delete(project.id)
         return
@@ -147,6 +158,11 @@ def delete_remote(self, url: str, namespace: str, name: str, username: str) -> N
 class GitHubRemote(Remote):
     """Remote repositories on GitHub."""
 
+    @staticmethod
+    def ssh_url(response: Any) -> str:
+        """Return the SSH URL of a GitHub repository."""
+        return response.ssh_url
+
     @property
     def url_fallbacks(self) -> list[str]:
         """Keyring service names tried after the API URL."""

cadetrdm/repositories.py

@@ -1246,17 +1246,17 @@ def create_remotes(
         )
         errors_encountered = 0
         try:
-            self.add_remote(response_project.ssh_url_to_repo)
+            self.add_remote(remote.ssh_url(response_project))
         except RuntimeError as e:
             errors_encountered += 1
             print(e)
-            print("Please fix the error above and re-run_yml repo.add_remote()")
+            print("Please fix the error above and re-run repo.add_remote()")
         try:
-            self.output_repo.add_remote(response_output.ssh_url_to_repo)
+            self.output_repo.add_remote(remote.ssh_url(response_output))
         except RuntimeError as e:
             errors_encountered += 1
             print(e)
-            print("Please fix the error above and re-run_yml repo.output_repo.add_remote()")
+            print("Please fix the error above and re-run repo.output_repo.add_remote()")
         if errors_encountered == 0 and push:
             self.push(push_all=True)
 

tests/test_remote_api.py

@@ -0,0 +1,147 @@
+"""
+Tests that create and delete repositories through the GitLab and GitHub APIs.
+
+They are marked `server_api` and need an account, a token in the Python keyring and
+SSH access to the host. Configure the accounts with environment variables; tests for
+a host whose namespace is not set are skipped:
+
+- CADET_RDM_TEST_GITLAB_URL: GitLab instance (default: https://jugit.fz-juelich.de/)
+- CADET_RDM_TEST_GITLAB_NAMESPACE: user or group to create test projects in
+- CADET_RDM_TEST_GITLAB_USERNAME: keyring username of the token (default: namespace)
+- CADET_RDM_TEST_GITHUB_NAMESPACE: user or organization to create test repositories in
+- CADET_RDM_TEST_GITHUB_USERNAME: keyring username of the token (default: namespace)
+"""
+
+import os
+import uuid
+import warnings
+from dataclasses import dataclass
+from time import sleep
+
+import git
+import pytest
+
+from cadetrdm import initialize_repo, ProjectRepo
+from cadetrdm.remote_integration import GitHubRemote, GitLabRemote
+from cadetrdm.repositories import GitRepo
+
+GITHUB_API_URL = "https://api.github.com"
+
+
+@dataclass
+class Account:
+    url: str
+    namespace: str
+    username: str
+
+
+def account_from_environment(host: str, default_url: str) -> Account:
+    namespace = os.environ.get(f"CADET_RDM_TEST_{host}_NAMESPACE")
+    if not namespace:
+        pytest.skip(f"Set CADET_RDM_TEST_{host}_NAMESPACE to run {host} API tests.")
+    return Account(
+        url=os.environ.get(f"CADET_RDM_TEST_{host}_URL", default_url),
+        namespace=namespace,
+        username=os.environ.get(f"CADET_RDM_TEST_{host}_USERNAME", namespace),
+    )
+
+
+@pytest.fixture
+def gitlab_account():
+    return account_from_environment("GITLAB", "https://jugit.fz-juelich.de/")
+
+
+@pytest.fixture
+def github_account():
+    return account_from_environment("GITHUB", GITHUB_API_URL)
+
+
+@pytest.fixture
+def repo_name():
+    return f"cadet_rdm_api_test_{uuid.uuid4().hex[:8]}"
+
+
+@pytest.fixture(autouse=True)
+def isolated_cwd(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+
+
+@pytest.fixture
+def created_remotes(monkeypatch):
+    """
+    Record the repositories created during a test and delete only those afterwards.
+
+    Repositories are recorded after create_remote() succeeded, so a name that already
+    existed on the host is never deleted.
+    """
+    created = []
+    for remote_class in (GitHubRemote, GitLabRemote):
+        original_create_remote = remote_class.create_remote
+
+        def create_and_record(self, _original=original_create_remote, **kwargs):
+            response = _original(self, **kwargs)
+            created.append((self, kwargs))
+            return response
+
+        monkeypatch.setattr(remote_class, "create_remote", create_and_record)
+
+    yield created
+
+    for remote, kwargs in created:
+        try:
+            remote.delete_remote(
+                url=kwargs["url"],
+                namespace=kwargs["namespace"],
+                name=kwargs["name"],
+                username=kwargs["username"],
+            )
+        except Exception as error:
+            warnings.warn(
+                f"Could not delete test repository {kwargs['namespace']}/{kwargs['name']}: "
+                f"{error}"
+            )
+
+
+@pytest.mark.server_api
+@pytest.mark.parametrize("host", ["gitlab", "github"])
+def test_create_and_delete_remote(host, repo_name, created_remotes, request):
+    account = request.getfixturevalue(f"{host}_account")
+    remote = GitLabRemote() if host == "gitlab" else GitHubRemote()
+
+    response = remote.create_remote(
+        url=account.url,
+        namespace=account.namespace,
+        name=repo_name,
+        username=account.username,
+    )
+    sleep(3)
+    GitRepo.clone(remote.ssh_url(response), "cloned_remote")
+
+    remote.delete_remote(
+        url=account.url, namespace=account.namespace, name=repo_name, username=account.username
+    )
+    created_remotes.clear()
+
+    sleep(3)
+    with pytest.raises(git.exc.GitCommandError):
+        GitRepo.clone(remote.ssh_url(response), "cloned_after_delete")
+
+
+@pytest.mark.server_api
+@pytest.mark.parametrize("host", ["gitlab", "github"])
+def test_create_remotes_for_project(host, repo_name, created_remotes, request):
+    account = request.getfixturevalue(f"{host}_account")
+
+    initialize_repo("project")
+    repo = ProjectRepo("project")
+    repo.create_remotes(
+        url=account.url,
+        namespace=account.namespace,
+        name=repo_name,
+        username=account.username,
+    )
+
+    assert len(created_remotes) == 2
+    assert len(repo.remote_urls) == 1
+    assert len(repo.output_repo.remote_urls) == 1
+    assert repo.has_changes_upstream is False