From 387f1e239c51b8e152dc8d3a333b5e25f85fda6f Mon Sep 17 00:00:00 2001 From: Hannah Lanzrath Date: Mon, 5 Oct 2026 16:48:39 +0200 Subject: [PATCH 1/3] Fix create_remotes on GitHub and make the API tests configurable create_remotes read ssh_url_to_repo from the created repositories, which only GitLab projects have, so on GitHub it created both repositories and then failed with an AttributeError. Each remote now returns the SSH URL of its response type. GitLabRemote.delete_remote ended its name and namespace checks in pass, so it deleted every project matching the search; it now only deletes the exact project. The server_api tests used fixed accounts of a single developer and a fixed repository name. They now read the namespaces from environment variables, are skipped when these are not set, use random repository names and always clean up. Offline tests for both fixes run in CI. --- .github/ISSUE_TEMPLATE/release_checklist.md | 3 +- CONTRIBUTING.md | 18 ++- cadetrdm/remote_integration.py | 24 +++- cadetrdm/repositories.py | 8 +- tests/test_gitlab_api.py | 100 ---------------- tests/test_remote_api.py | 121 ++++++++++++++++++++ tests/test_remote_integration.py | 66 +++++++++++ 7 files changed, 228 insertions(+), 112 deletions(-) delete mode 100644 tests/test_gitlab_api.py create mode 100644 tests/test_remote_api.py create mode 100644 tests/test_remote_integration.py diff --git a/.github/ISSUE_TEMPLATE/release_checklist.md b/.github/ISSUE_TEMPLATE/release_checklist.md index 40bd3c1..1fd6d15 100644 --- a/.github/ISSUE_TEMPLATE/release_checklist.md +++ b/.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" ``` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1935f94..79c3ff9 100644 --- a/CONTRIBUTING.md +++ b/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/" +export CADET_RDM_TEST_GITLAB_NAMESPACE= +export CADET_RDM_TEST_GITHUB_NAMESPACE= +keyring set "https://jugit.fz-juelich.de/" +keyring set "https://github.com/" +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 diff --git a/cadetrdm/remote_integration.py b/cadetrdm/remote_integration.py index cc19387..5f5ea60 100644 --- a/cadetrdm/remote_integration.py +++ b/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.""" diff --git a/cadetrdm/repositories.py b/cadetrdm/repositories.py index 4a03df8..569e5c7 100644 --- a/cadetrdm/repositories.py +++ b/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) diff --git a/tests/test_gitlab_api.py b/tests/test_gitlab_api.py deleted file mode 100644 index 3adf06d..0000000 --- a/tests/test_gitlab_api.py +++ /dev/null @@ -1,100 +0,0 @@ -from time import sleep - -import git -import pytest - -from cadetrdm import initialize_repo, ProjectRepo -from cadetrdm.io_utils import delete_path -from cadetrdm.remote_integration import GitHubRemote, GitLabRemote -from cadetrdm.repositories import BaseRepo - - -@pytest.fixture(autouse=True) -def isolated_cwd(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) - - -@pytest.mark.server_api -def test_gitlab_create(): - url = "https://jugit.fz-juelich.de/" - namespace = "r.jaepel" - name = "API_test_project" - remote = GitLabRemote() - - # ensure remote does not exist - remote.delete_remote(url=url, namespace=namespace, name=name, username="r.jaepel") - try: - delete_path("test_repo_remote") - except FileNotFoundError: - pass - - sleep(3) - - response = remote.create_remote(url=url, namespace=namespace, name=name, username="r.jaepel") - - BaseRepo.clone(response.ssh_url_to_repo, "test_repo_remote") - delete_path("test_repo_remote") - - remote.delete_remote(url=url, namespace=namespace, name=name, username="r.jaepel") - - sleep(3) - - with pytest.raises(git.exc.GitCommandError): - BaseRepo.clone(response.ssh_url_to_repo, "test_repo_remote") - - -@pytest.mark.server_api -def test_github_create(): - namespace = "ronald-jaepel" - name = "API_test_project" - remote = GitHubRemote() - - # ensure remote does not exist - try: - remote.delete_remote(namespace=namespace, name=name, username="r.jaepel") - except Exception: - pass - - try: - delete_path("test_repo_remote") - except FileNotFoundError: - pass - - sleep(3) - - response = remote.create_remote(namespace=namespace, name=name, username="r.jaepel") - - sleep(3) - - BaseRepo.clone(response.html_url, "test_repo_remote") - delete_path("test_repo_remote") - - remote.delete_remote(namespace=namespace, name=name, username="r.jaepel") - - with pytest.raises(git.exc.GitCommandError): - BaseRepo.clone(response.ssh_url, "test_repo_remote") - - -@pytest.mark.server_api -def test_repo_gitlab_integration(): - url = "https://jugit.fz-juelich.de/" - namespace = "r.jaepel" - name = "API_test_project" - repo_name = "test_repo_remote" - remote = GitLabRemote() - - # Clean up - remote.delete_remote(url=url, namespace=namespace, name=name, username="r.jaepel") - remote.delete_remote(url=url, namespace=namespace, name=name + "_output", username="r.jaepel") - - try: - delete_path("test_repo_remote") - except FileNotFoundError: - pass - - initialize_repo(repo_name) - - repo = ProjectRepo(repo_name) - repo.create_remotes(url=url, namespace=namespace, name=name, username="r.jaepel") - - assert repo.has_changes_upstream is False diff --git a/tests/test_remote_api.py b/tests/test_remote_api.py new file mode 100644 index 0000000..93348f5 --- /dev/null +++ b/tests/test_remote_api.py @@ -0,0 +1,121 @@ +""" +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 +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, Remote +from cadetrdm.repositories import BaseRepo + +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) + + +def delete_quietly(remote: Remote, account: Account, name: str) -> None: + try: + remote.delete_remote( + url=account.url, namespace=account.namespace, name=name, username=account.username + ) + except Exception: + pass + + +@pytest.mark.server_api +@pytest.mark.parametrize("host", ["gitlab", "github"]) +def test_create_and_delete_remote(host, repo_name, request): + account = request.getfixturevalue(f"{host}_account") + remote = GitLabRemote() if host == "gitlab" else GitHubRemote() + + try: + response = remote.create_remote( + url=account.url, + namespace=account.namespace, + name=repo_name, + username=account.username, + ) + sleep(3) + BaseRepo.clone(remote.ssh_url(response), "cloned_remote") + finally: + delete_quietly(remote, account, repo_name) + + sleep(3) + with pytest.raises(git.exc.GitCommandError): + BaseRepo.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, request): + account = request.getfixturevalue(f"{host}_account") + remote = GitLabRemote() if host == "gitlab" else GitHubRemote() + + initialize_repo("project") + repo = ProjectRepo("project") + try: + repo.create_remotes( + url=account.url, + namespace=account.namespace, + name=repo_name, + username=account.username, + ) + + assert len(repo.remote_urls) == 1 + assert len(repo.output_repo.remote_urls) == 1 + assert repo.has_changes_upstream is False + finally: + delete_quietly(remote, account, repo_name) + delete_quietly(remote, account, repo_name + "_output") diff --git a/tests/test_remote_integration.py b/tests/test_remote_integration.py new file mode 100644 index 0000000..7471066 --- /dev/null +++ b/tests/test_remote_integration.py @@ -0,0 +1,66 @@ +from types import SimpleNamespace + +import pytest + +from cadetrdm import ProjectRepo, initialize_repo +from cadetrdm.remote_integration import GitHubRemote, GitLabRemote + + +@pytest.fixture(autouse=True) +def isolated_cwd(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + + +def fake_github_repository(name, namespace=None, url=None, username=None): + """Mimic the attributes of a PyGithub Repository.""" + return SimpleNamespace(ssh_url=f"git@github.com:{namespace}/{name}.git") + + +def fake_gitlab_project(url, namespace, name, username): + """Mimic the attributes of a python-gitlab Project.""" + return SimpleNamespace(ssh_url_to_repo=f"git@gitlab.example.com:{namespace}/{name}.git") + + +@pytest.mark.parametrize( + "remote_class, fake_create_remote, url, host", + [ + (GitHubRemote, fake_github_repository, "https://api.github.com", "github.com"), + (GitLabRemote, fake_gitlab_project, "https://gitlab.example.com/", "gitlab.example.com"), + ], +) +def test_create_remotes_adds_ssh_urls( + monkeypatch, remote_class, fake_create_remote, url, host +): + monkeypatch.setattr(remote_class, "create_remote", staticmethod(fake_create_remote)) + initialize_repo("project") + repo = ProjectRepo("project") + + repo.create_remotes(name="analysis", namespace="lab", url=url, username="me", push=False) + + assert repo.remote_urls == [f"git@{host}:lab/analysis.git"] + assert repo.output_repo.remote_urls == [f"git@{host}:lab/analysis_output.git"] + + +def test_gitlab_delete_remote_only_deletes_exact_match(monkeypatch): + projects = [ + SimpleNamespace(id=1, name="analysis", namespace={"full_path": "lab"}), + SimpleNamespace(id=2, name="analysis_output", namespace={"full_path": "lab"}), + SimpleNamespace(id=3, name="analysis", namespace={"full_path": "other-lab"}), + ] + deleted = [] + + class FakeGitlab: + def __init__(self, url, private_token): + self.projects = SimpleNamespace( + list=lambda **kwargs: projects, + delete=deleted.append, + ) + + monkeypatch.setattr("cadetrdm.remote_integration.gitlab.Gitlab", FakeGitlab) + monkeypatch.setattr(GitLabRemote, "load_token", staticmethod(lambda *args: "token")) + + GitLabRemote().delete_remote( + url="https://gitlab.example.com/", namespace="Lab", name="analysis", username="me" + ) + + assert deleted == [1] From 3f979bcaa795ecf58d653c40bd42b53e4a8a25db Mon Sep 17 00:00:00 2001 From: Hannah Lanzrath Date: Mon, 5 Oct 2026 17:00:58 +0200 Subject: [PATCH 2/3] Delete only repositories the API tests created Cleanup deleted the test names even if creating them had failed, so an existing repository with the same name would have been removed. A fixture now records repositories after they were created and deletes only those, and warns about repositories it could not delete. --- tests/test_remote_api.py | 98 +++++++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 36 deletions(-) diff --git a/tests/test_remote_api.py b/tests/test_remote_api.py index 93348f5..e4dec09 100644 --- a/tests/test_remote_api.py +++ b/tests/test_remote_api.py @@ -14,6 +14,7 @@ import os import uuid +import warnings from dataclasses import dataclass from time import sleep @@ -21,7 +22,7 @@ import pytest from cadetrdm import initialize_repo, ProjectRepo -from cadetrdm.remote_integration import GitHubRemote, GitLabRemote, Remote +from cadetrdm.remote_integration import GitHubRemote, GitLabRemote from cadetrdm.repositories import BaseRepo GITHUB_API_URL = "https://api.github.com" @@ -65,32 +66,61 @@ def isolated_cwd(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) -def delete_quietly(remote: Remote, account: Account, name: str) -> None: - try: - remote.delete_remote( - url=account.url, namespace=account.namespace, name=name, username=account.username - ) - except Exception: - pass +@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, request): +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() - try: - response = remote.create_remote( - url=account.url, - namespace=account.namespace, - name=repo_name, - username=account.username, - ) - sleep(3) - BaseRepo.clone(remote.ssh_url(response), "cloned_remote") - finally: - delete_quietly(remote, account, repo_name) + response = remote.create_remote( + url=account.url, + namespace=account.namespace, + name=repo_name, + username=account.username, + ) + sleep(3) + BaseRepo.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): @@ -99,23 +129,19 @@ def test_create_and_delete_remote(host, repo_name, request): @pytest.mark.server_api @pytest.mark.parametrize("host", ["gitlab", "github"]) -def test_create_remotes_for_project(host, repo_name, request): +def test_create_remotes_for_project(host, repo_name, created_remotes, request): account = request.getfixturevalue(f"{host}_account") - remote = GitLabRemote() if host == "gitlab" else GitHubRemote() initialize_repo("project") repo = ProjectRepo("project") - try: - repo.create_remotes( - url=account.url, - namespace=account.namespace, - name=repo_name, - username=account.username, - ) - - assert len(repo.remote_urls) == 1 - assert len(repo.output_repo.remote_urls) == 1 - assert repo.has_changes_upstream is False - finally: - delete_quietly(remote, account, repo_name) - delete_quietly(remote, account, repo_name + "_output") + 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 From e790a87b650d75c8d2d1e4e23a0f71ea02db25c4 Mon Sep 17 00:00:00 2001 From: Hannah Lanzrath Date: Mon, 5 Oct 2026 17:05:19 +0200 Subject: [PATCH 3/3] Clone the empty test repositories with plain git BaseRepo requires CADET-RDM metadata, which a newly created remote does not have. --- tests/test_remote_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_remote_api.py b/tests/test_remote_api.py index e4dec09..79efc80 100644 --- a/tests/test_remote_api.py +++ b/tests/test_remote_api.py @@ -23,7 +23,7 @@ from cadetrdm import initialize_repo, ProjectRepo from cadetrdm.remote_integration import GitHubRemote, GitLabRemote -from cadetrdm.repositories import BaseRepo +from cadetrdm.repositories import GitRepo GITHUB_API_URL = "https://api.github.com" @@ -115,7 +115,7 @@ def test_create_and_delete_remote(host, repo_name, created_remotes, request): username=account.username, ) sleep(3) - BaseRepo.clone(remote.ssh_url(response), "cloned_remote") + GitRepo.clone(remote.ssh_url(response), "cloned_remote") remote.delete_remote( url=account.url, namespace=account.namespace, name=repo_name, username=account.username @@ -124,7 +124,7 @@ def test_create_and_delete_remote(host, repo_name, created_remotes, request): sleep(3) with pytest.raises(git.exc.GitCommandError): - BaseRepo.clone(remote.ssh_url(response), "cloned_after_delete") + GitRepo.clone(remote.ssh_url(response), "cloned_after_delete") @pytest.mark.server_api