Merge pull request josegonzalez#461 from Iamrodos/fix-cli-ux-and-cleanup

josegonzalez · web-flow · commit 5b52931ebf2f · 2025-12-07T08:58:59.000-05:00
fix: CLI UX improvements and cleanup
diff --git a/README.rst b/README.rst
@@ -281,11 +281,11 @@ If the incremental argument is used, this will result in the next backup only re
 
 It's therefore recommended to only use the incremental argument if the output/result is being actively monitored, or complimented with periodic full non-incremental runs, to avoid unexpected missing data in a regular backup runs.
 
-1. **Starred public repo hooks blocking**
+**Starred public repo hooks blocking**
 
-   Since the ``--all`` argument includes ``--hooks``, if you use ``--all`` and ``--all-starred`` together to clone a users starred public repositories, the backup will likely error and block the backup continuing. 
+Since the ``--all`` argument includes ``--hooks``, if you use ``--all`` and ``--all-starred`` together to clone a users starred public repositories, the backup will likely error and block the backup continuing.
 
-   This is due to needing the correct permission for ``--hooks`` on public repos.
+This is due to needing the correct permission for ``--hooks`` on public repos.
 
 
 "bare" is actually "mirror"
diff --git a/bin/github-backup b/bin/github-backup
@@ -9,6 +9,7 @@ from github_backup.github_backup import (
     backup_repositories,
     check_git_lfs_install,
     filter_repositories,
+    get_auth,
     get_authenticated_user,
     logger,
     mkdir_p,
@@ -37,6 +38,12 @@ logging.basicConfig(level=logging.INFO, handlers=[stdout_handler, stderr_handler
 def main():
     args = parse_args()
 
+    if args.private and not get_auth(args):
+        logger.warning(
+            "The --private flag has no effect without authentication. "
+            "Use -t/--token, -f/--token-fine, or -u/--username to authenticate."
+        )
+
     if args.quiet:
         logger.setLevel(logging.WARNING)
 
diff --git a/github_backup/github_backup.py b/github_backup/github_backup.py
@@ -561,7 +561,7 @@ def get_github_host(args):
 
 
 def read_file_contents(file_uri):
-    return open(file_uri[len(FILE_URI_PREFIX) :], "rt").readline().strip()
+    return open(file_uri[len(FILE_URI_PREFIX):], "rt").readline().strip()
 
 
 def get_github_repo_url(args, repository):
@@ -1672,9 +1672,10 @@ def backup_repositories(args, output_directory, repositories):
         repo_url = get_github_repo_url(args, repository)
 
         include_gists = args.include_gists or args.include_starred_gists
+        include_starred = args.all_starred and repository.get("is_starred")
         if (args.include_repository or args.include_everything) or (
             include_gists and repository.get("is_gist")
-        ):
+        ) or include_starred:
             repo_name = (
                 repository.get("name")
                 if not repository.get("is_gist")
@@ -2023,12 +2024,9 @@ def fetch_repository(
 ):
     if bare_clone:
         if os.path.exists(local_dir):
-            clone_exists = (
-                subprocess.check_output(
-                    ["git", "rev-parse", "--is-bare-repository"], cwd=local_dir
-                )
-                == b"true\n"
-            )
+            clone_exists = subprocess.check_output(
+                ["git", "rev-parse", "--is-bare-repository"], cwd=local_dir
+            ) == b"true\n"
         else:
             clone_exists = False
     else:
@@ -2043,11 +2041,14 @@ def fetch_repository(
         "git ls-remote " + remote_url, stdout=FNULL, stderr=FNULL, shell=True
     )
     if initialized == 128:
-        logger.info(
-            "Skipping {0} ({1}) since it's not initialized".format(
-                name, masked_remote_url
+        if ".wiki.git" in remote_url:
+            logger.info(
+                "Skipping {0} wiki (wiki is enabled but has no content)".format(name)
+            )
+        else:
+            logger.info(
+                "Skipping {0} (repository not accessible - may be empty, private, or credentials invalid)".format(name)
             )
-        )
         return
 
     if clone_exists:
diff --git a/tests/test_all_starred.py b/tests/test_all_starred.py
@@ -0,0 +1,161 @@
+"""Tests for --all-starred flag behavior (issue #225)."""
+
+import pytest
+from unittest.mock import Mock, patch
+
+from github_backup import github_backup
+
+
+class TestAllStarredCloning:
+    """Test suite for --all-starred repository cloning behavior.
+
+    Issue #225: --all-starred should clone starred repos without requiring --repositories.
+    """
+
+    def _create_mock_args(self, **overrides):
+        """Create a mock args object with sensible defaults."""
+        args = Mock()
+        args.user = "testuser"
+        args.output_directory = "/tmp/backup"
+        args.include_repository = False
+        args.include_everything = False
+        args.include_gists = False
+        args.include_starred_gists = False
+        args.all_starred = False
+        args.skip_existing = False
+        args.bare_clone = False
+        args.lfs_clone = False
+        args.no_prune = False
+        args.include_wiki = False
+        args.include_issues = False
+        args.include_issue_comments = False
+        args.include_issue_events = False
+        args.include_pulls = False
+        args.include_pull_comments = False
+        args.include_pull_commits = False
+        args.include_pull_details = False
+        args.include_labels = False
+        args.include_hooks = False
+        args.include_milestones = False
+        args.include_releases = False
+        args.include_assets = False
+        args.include_attachments = False
+        args.incremental = False
+        args.incremental_by_files = False
+        args.github_host = None
+        args.prefer_ssh = False
+        args.token_classic = None
+        args.token_fine = None
+        args.username = None
+        args.password = None
+        args.as_app = False
+        args.osx_keychain_item_name = None
+        args.osx_keychain_item_account = None
+
+        for key, value in overrides.items():
+            setattr(args, key, value)
+
+        return args
+
+    @patch('github_backup.github_backup.fetch_repository')
+    @patch('github_backup.github_backup.get_github_repo_url')
+    def test_all_starred_clones_without_repositories_flag(self, mock_get_url, mock_fetch):
+        """--all-starred should clone starred repos without --repositories flag.
+
+        This is the core fix for issue #225.
+        """
+        args = self._create_mock_args(all_starred=True)
+        mock_get_url.return_value = "https://github.com/otheruser/awesome-project.git"
+
+        # A starred repository (is_starred flag set by retrieve_repositories)
+        starred_repo = {
+            "name": "awesome-project",
+            "full_name": "otheruser/awesome-project",
+            "owner": {"login": "otheruser"},
+            "private": False,
+            "fork": False,
+            "has_wiki": False,
+            "is_starred": True,  # This flag is set for starred repos
+        }
+
+        with patch('github_backup.github_backup.mkdir_p'):
+            github_backup.backup_repositories(args, "/tmp/backup", [starred_repo])
+
+        # fetch_repository should be called for the starred repo
+        assert mock_fetch.called, "--all-starred should trigger repository cloning"
+        mock_fetch.assert_called_once()
+        call_args = mock_fetch.call_args
+        assert call_args[0][0] == "awesome-project"  # repo name
+
+    @patch('github_backup.github_backup.fetch_repository')
+    @patch('github_backup.github_backup.get_github_repo_url')
+    def test_starred_repo_not_cloned_without_all_starred_flag(self, mock_get_url, mock_fetch):
+        """Starred repos should NOT be cloned if --all-starred is not set."""
+        args = self._create_mock_args(all_starred=False)
+        mock_get_url.return_value = "https://github.com/otheruser/awesome-project.git"
+
+        starred_repo = {
+            "name": "awesome-project",
+            "full_name": "otheruser/awesome-project",
+            "owner": {"login": "otheruser"},
+            "private": False,
+            "fork": False,
+            "has_wiki": False,
+            "is_starred": True,
+        }
+
+        with patch('github_backup.github_backup.mkdir_p'):
+            github_backup.backup_repositories(args, "/tmp/backup", [starred_repo])
+
+        # fetch_repository should NOT be called
+        assert not mock_fetch.called, "Starred repos should not be cloned without --all-starred"
+
+    @patch('github_backup.github_backup.fetch_repository')
+    @patch('github_backup.github_backup.get_github_repo_url')
+    def test_non_starred_repo_not_cloned_with_only_all_starred(self, mock_get_url, mock_fetch):
+        """Non-starred repos should NOT be cloned when only --all-starred is set."""
+        args = self._create_mock_args(all_starred=True)
+        mock_get_url.return_value = "https://github.com/testuser/my-project.git"
+
+        # A regular (non-starred) repository
+        regular_repo = {
+            "name": "my-project",
+            "full_name": "testuser/my-project",
+            "owner": {"login": "testuser"},
+            "private": False,
+            "fork": False,
+            "has_wiki": False,
+            # No is_starred flag
+        }
+
+        with patch('github_backup.github_backup.mkdir_p'):
+            github_backup.backup_repositories(args, "/tmp/backup", [regular_repo])
+
+        # fetch_repository should NOT be called for non-starred repos
+        assert not mock_fetch.called, "Non-starred repos should not be cloned with only --all-starred"
+
+    @patch('github_backup.github_backup.fetch_repository')
+    @patch('github_backup.github_backup.get_github_repo_url')
+    def test_repositories_flag_still_works(self, mock_get_url, mock_fetch):
+        """--repositories flag should still clone repos as before."""
+        args = self._create_mock_args(include_repository=True)
+        mock_get_url.return_value = "https://github.com/testuser/my-project.git"
+
+        regular_repo = {
+            "name": "my-project",
+            "full_name": "testuser/my-project",
+            "owner": {"login": "testuser"},
+            "private": False,
+            "fork": False,
+            "has_wiki": False,
+        }
+
+        with patch('github_backup.github_backup.mkdir_p'):
+            github_backup.backup_repositories(args, "/tmp/backup", [regular_repo])
+
+        # fetch_repository should be called
+        assert mock_fetch.called, "--repositories should trigger repository cloning"
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])
diff --git a/tests/test_case_sensitivity.py b/tests/test_case_sensitivity.py
@@ -0,0 +1,112 @@
+"""Tests for case-insensitive username/organization filtering."""
+
+import pytest
+from unittest.mock import Mock
+
+from github_backup import github_backup
+
+
+class TestCaseSensitivity:
+    """Test suite for case-insensitive username matching in filter_repositories."""
+
+    def test_filter_repositories_case_insensitive_user(self):
+        """Should filter repositories case-insensitively for usernames.
+
+        Reproduces issue #198 where typing 'iamrodos' fails to match
+        repositories with owner.login='Iamrodos' (the canonical case from GitHub API).
+        """
+        # Simulate user typing lowercase username
+        args = Mock()
+        args.user = "iamrodos"  # lowercase (what user typed)
+        args.repository = None
+        args.name_regex = None
+        args.languages = None
+        args.exclude = None
+        args.fork = False
+        args.private = False
+        args.public = False
+        args.all = True
+
+        # Simulate GitHub API returning canonical case
+        repos = [
+            {
+                "name": "repo1",
+                "owner": {"login": "Iamrodos"},  # Capital I (canonical from API)
+                "private": False,
+                "fork": False,
+            },
+            {
+                "name": "repo2",
+                "owner": {"login": "Iamrodos"},
+                "private": False,
+                "fork": False,
+            },
+        ]
+
+        filtered = github_backup.filter_repositories(args, repos)
+
+        # Should match despite case difference
+        assert len(filtered) == 2
+        assert filtered[0]["name"] == "repo1"
+        assert filtered[1]["name"] == "repo2"
+
+    def test_filter_repositories_case_insensitive_org(self):
+        """Should filter repositories case-insensitively for organizations.
+
+        Tests the example from issue #198 where 'prai-org' doesn't match 'PRAI-Org'.
+        """
+        args = Mock()
+        args.user = "prai-org"  # lowercase (what user typed)
+        args.repository = None
+        args.name_regex = None
+        args.languages = None
+        args.exclude = None
+        args.fork = False
+        args.private = False
+        args.public = False
+        args.all = True
+
+        repos = [
+            {
+                "name": "repo1",
+                "owner": {"login": "PRAI-Org"},  # Different case (canonical from API)
+                "private": False,
+                "fork": False,
+            },
+        ]
+
+        filtered = github_backup.filter_repositories(args, repos)
+
+        # Should match despite case difference
+        assert len(filtered) == 1
+        assert filtered[0]["name"] == "repo1"
+
+    def test_filter_repositories_case_variations(self):
+        """Should handle various case combinations correctly."""
+        args = Mock()
+        args.user = "TeSt-UsEr"  # Mixed case
+        args.repository = None
+        args.name_regex = None
+        args.languages = None
+        args.exclude = None
+        args.fork = False
+        args.private = False
+        args.public = False
+        args.all = True
+
+        repos = [
+            {"name": "repo1", "owner": {"login": "test-user"}, "private": False, "fork": False},
+            {"name": "repo2", "owner": {"login": "TEST-USER"}, "private": False, "fork": False},
+            {"name": "repo3", "owner": {"login": "TeSt-UsEr"}, "private": False, "fork": False},
+            {"name": "repo4", "owner": {"login": "other-user"}, "private": False, "fork": False},
+        ]
+
+        filtered = github_backup.filter_repositories(args, repos)
+
+        # Should match first 3 (all case variations of same user)
+        assert len(filtered) == 3
+        assert set(r["name"] for r in filtered) == {"repo1", "repo2", "repo3"}
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])
