Skip to content

Fix create_remotes on GitHub and make the API tests configurable - #148

Merged
hannahlanzrath merged 3 commits into
mainfrom
fix/github-create-remotes
Oct 5, 2026
Merged

hannahlanzrath merged 3 commits into
mainfrom
fix/github-create-remotes

Conversation

@hannahlanzrath

@hannahlanzrath hannahlanzrath commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Bug fixes

  • create_remotes on GitHub: it read ssh_url_to_repo from the created repositories, an attribute only GitLab projects have. On GitHub it created both repositories and then failed with an AttributeError, so repo.create_remotes(...) / rdm remote create never worked there. Each remote class now has ssh_url(response), which returns the SSH URL for its host.
  • GitLabRemote.delete_remote: its name and namespace checks ended in pass instead of continue, so it deleted every project the search returned. It now only deletes the exact project namespace/name.

Configurable API tests (tests/test_gitlab_api.py → tests/test_remote_api.py)

  • Accounts: the server_api tests were hardcoded to one developer's accounts and used a fixed repository name. They now read their accounts from environment variables:
    • CADET_RDM_TEST_GITLAB_NAMESPACE, CADET_RDM_TEST_GITHUB_NAMESPACE
    • optional: CADET_RDM_TEST_GITLAB_URL, CADET_RDM_TEST_GITLAB_USERNAME, CADET_RDM_TEST_GITHUB_USERNAME
  • Skipping: tests for a host without a namespace are skipped with a message saying which variable to set.
  • No collisions: every run uses random repository names and deletes them in a finally block, so runs by different developers can't interfere.
  • Coverage: the tests now run against GitHub and GitLab alike; before, only GitLab had an integration test, which is why the GitHub bug went unnoticed.
  • Docs: CONTRIBUTING.md describes the variables, tokens and SSH requirements, and the release checklist template points there.

Offline tests (tests/test_remote_integration.py, run in CI)

  • create_remotes adds the right SSH URLs for faked GitHub and GitLab responses.
  • delete_remote deletes only the exact match.
  • Both tests fail on the old code.

The CI test selection passes locally (76 passed).

Run against GitHub: with CADET_RDM_TEST_GITHUB_NAMESPACE=hannahlanzrath, both GitHub tests pass. That includes create_remotes, which confirms the fix against the real API, and no test repositories were left behind. The GitLab tests were skipped because no GitLab token was available.

Cleanup safety: the tests only delete repositories they created themselves (recorded after a successful create_remote), and they warn about any repository they couldn't delete.

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.
@hannahlanzrath
hannahlanzrath added this pull request to stack #149 October 5, 2026 14:49
This was referenced Oct 5, 2026
@codecov-commenter

codecov-commenter commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cadetrdm/repositories.py 50.00% 2 Missing ⚠️
cadetrdm/remote_integration.py 92.85% 1 Missing ⚠️
Files with missing lines Coverage Δ
cadetrdm/remote_integration.py 46.15% <92.85%> (+18.99%) ⬆️
cadetrdm/repositories.py 61.64% <50.00%> (+0.95%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.
BaseRepo requires CADET-RDM metadata, which a newly created remote does
not have.
@hannahlanzrath
hannahlanzrath merged commit 6267bea into main Oct 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants