test: add pytest-mock, introduce fixtures & type hints (#290)

* Added pytest-mock to dev dependencies and pre-commit hooks
* Introduced InvalidGitHubTokenError for clearer token-validation failures
* Refactored tests:
  * Replaced ad-hoc mocks with reusable fixtures
  * Parametrised URL/branch matrices to cut duplication
  * Added type hints throughout
* New coverage:
  * validate_github_token (happy & error paths)
  * create_git_command / create_git_auth_header

Author: filipchristiansen

.pre-commit-config.yaml

@@ -99,6 +99,7 @@ repos:
             "fastapi[standard]>=0.109.1",
             pydantic,
             pytest-asyncio,
+            pytest-mock,
             python-dotenv,
             slowapi,
             starlette>=0.40.0,
@@ -117,6 +118,7 @@ repos:
             "fastapi[standard]>=0.109.1",
             pydantic,
             pytest-asyncio,
+            pytest-mock,
             python-dotenv,
             slowapi,
             starlette>=0.40.0,

requirements-dev.txt

@@ -5,3 +5,4 @@ pre-commit
 pylint
 pytest
 pytest-asyncio
+pytest-mock

src/gitingest/utils/exceptions.py

@@ -35,3 +35,13 @@ class InvalidNotebookError(Exception):
 
     def __init__(self, message: str) -> None:
         super().__init__(message)
+
+
+class InvalidGitHubTokenError(ValueError):
+    """Exception raised when a GitHub Personal Access Token is malformed."""
+
+    def __init__(self) -> None:
+        super().__init__(
+            "Invalid GitHub token format. Token should start with 'github_pat_' or 'ghp_' "
+            "followed by at least 36 characters of letters, numbers, and underscores."
+        )

src/gitingest/utils/git_utils.py

@@ -5,6 +5,8 @@
 import re
 from typing import List, Optional, Tuple
 
+from gitingest.utils.exceptions import InvalidGitHubTokenError
+
 GITHUB_PAT_PATTERN = r"^(?:github_pat_|ghp_)[A-Za-z0-9_]{36,}$"
 
 
@@ -256,11 +258,8 @@ def validate_github_token(token: str) -> None:
 
     Raises
     ------
-    ValueError
+    InvalidGitHubTokenError
         If the token format is invalid
     """
     if not re.match(GITHUB_PAT_PATTERN, token):
-        raise ValueError(
-            "Invalid GitHub token format. Token should start with 'github_pat_' or 'ghp_' "
-            "followed by at least 36 characters of letters, numbers, and underscores."
-        )
+        raise InvalidGitHubTokenError()

tests/conftest.py

@@ -7,14 +7,19 @@
 
 import json
 from pathlib import Path
-from typing import Any, Callable, Dict
+from typing import Any, Callable, Dict, List
+from unittest.mock import AsyncMock
 
 import pytest
+from pytest_mock import MockerFixture
 
 from gitingest.query_parsing import IngestionQuery
 
 WriteNotebookFunc = Callable[[str, Dict[str, Any]], Path]
 
+DEMO_URL = "https://github.com/user/repo"
+LOCAL_REPO_PATH = "/tmp/repo"
+
 
 @pytest.fixture
 def sample_query() -> IngestionQuery:
@@ -129,3 +134,51 @@ def _write_notebook(name: str, content: Dict[str, Any]) -> Path:
         return notebook_path
 
     return _write_notebook
+
+
+@pytest.fixture
+def stub_branches(mocker: MockerFixture) -> Callable[[List[str]], None]:
+    """Return a function that stubs git branch discovery to *branches*."""
+
+    def _factory(branches: List[str]) -> None:
+        mocker.patch(
+            "gitingest.utils.git_utils.run_command",
+            new_callable=AsyncMock,
+            return_value=("\n".join(f"refs/heads/{b}" for b in branches).encode() + b"\n", b""),
+        )
+        mocker.patch(
+            "gitingest.utils.git_utils.fetch_remote_branch_list",
+            new_callable=AsyncMock,
+            return_value=branches,
+        )
+
+    return _factory
+
+
+@pytest.fixture
+def repo_exists_true(mocker: MockerFixture) -> AsyncMock:
+    """Patch `gitingest.cloning.check_repo_exists` to always return ``True``.
+
+    Many cloning-related tests assume that the remote repository exists. This fixture centralises
+    that behaviour so individual tests no longer need to repeat the same ``mocker.patch`` call.
+    The mock object is returned so that tests can make assertions on how it was used or override
+    its behaviour when needed.
+    """
+    return mocker.patch("gitingest.cloning.check_repo_exists", return_value=True)
+
+
+@pytest.fixture
+def run_command_mock(mocker: MockerFixture) -> AsyncMock:
+    """Patch `gitingest.cloning.run_command` with an ``AsyncMock``.
+
+    The mocked function returns a dummy process whose ``communicate`` method yields generic
+    *stdout* / *stderr* bytes. Tests can still access / tweak the mock via the fixture argument.
+    """
+    mock_exec = mocker.patch("gitingest.cloning.run_command", new_callable=AsyncMock)
+
+    # Provide a default dummy process so most tests don't have to create one.
+    dummy_process = AsyncMock()
+    dummy_process.communicate.return_value = (b"output", b"error")
+    mock_exec.return_value = dummy_process
+
+    return mock_exec

tests/query_parser/test_git_host_agnostic.py

@@ -5,91 +5,60 @@
 Bitbucket, Gitea, and Codeberg, even if the host is omitted.
 """
 
-from typing import List
+from typing import List, Tuple
 
 import pytest
 
 from gitingest.query_parsing import parse_query
 
+# Repository matrix: (host, user, repo)
+_REPOS: List[Tuple[str, str, str]] = [
+    ("github.com", "tiangolo", "fastapi"),
+    ("gitlab.com", "gitlab-org", "gitlab-runner"),
+    ("bitbucket.org", "na-dna", "llm-knowledge-share"),
+    ("gitea.com", "xorm", "xorm"),
+    ("codeberg.org", "forgejo", "forgejo"),
+]
 
-@pytest.mark.parametrize(
-    "urls, expected_user, expected_repo, expected_url",
-    [
-        (
-            [
-                "https://github.com/tiangolo/fastapi",
-                "github.com/tiangolo/fastapi",
-                "tiangolo/fastapi",
-            ],
-            "tiangolo",
-            "fastapi",
-            "https://github.com/tiangolo/fastapi",
-        ),
-        (
-            [
-                "https://gitlab.com/gitlab-org/gitlab-runner",
-                "gitlab.com/gitlab-org/gitlab-runner",
-                "gitlab-org/gitlab-runner",
-            ],
-            "gitlab-org",
-            "gitlab-runner",
-            "https://gitlab.com/gitlab-org/gitlab-runner",
-        ),
-        (
-            [
-                "https://bitbucket.org/na-dna/llm-knowledge-share",
-                "bitbucket.org/na-dna/llm-knowledge-share",
-                "na-dna/llm-knowledge-share",
-            ],
-            "na-dna",
-            "llm-knowledge-share",
-            "https://bitbucket.org/na-dna/llm-knowledge-share",
-        ),
-        (
-            [
-                "https://gitea.com/xorm/xorm",
-                "gitea.com/xorm/xorm",
-                "xorm/xorm",
-            ],
-            "xorm",
-            "xorm",
-            "https://gitea.com/xorm/xorm",
-        ),
-        (
-            [
-                "https://codeberg.org/forgejo/forgejo",
-                "codeberg.org/forgejo/forgejo",
-                "forgejo/forgejo",
-            ],
-            "forgejo",
-            "forgejo",
-            "https://codeberg.org/forgejo/forgejo",
-        ),
-    ],
-)
+
+# Generate cartesian product of repository tuples with URL variants.
+@pytest.mark.parametrize("host, user, repo", _REPOS, ids=[f"{h}:{u}/{r}" for h, u, r in _REPOS])
+@pytest.mark.parametrize("variant", ["full", "noscheme", "slug"])
 @pytest.mark.asyncio
 async def test_parse_query_without_host(
-    urls: List[str],
-    expected_user: str,
-    expected_repo: str,
-    expected_url: str,
+    host: str,
+    user: str,
+    repo: str,
+    variant: str,
 ) -> None:
-    """
-    Test `parse_query` for Git host agnosticism.
+    """Verify that `parse_query` handles URLs, host-omitted URLs and raw slugs."""
+
+    # Build the input URL based on the selected variant
+    if variant == "full":
+        url = f"https://{host}/{user}/{repo}"
+    elif variant == "noscheme":
+        url = f"{host}/{user}/{repo}"
+    else:  # "slug"
+        url = f"{user}/{repo}"
+
+    expected_url = f"https://{host}/{user}/{repo}"
+
+    query = await parse_query(url, max_file_size=50, from_web=True)
+
+    # Compare against the canonical dict while ignoring unpredictable fields.
+    actual = query.model_dump(exclude={"id", "local_path", "ignore_patterns"})
 
-    Given multiple URL variations for the same user/repo on different Git hosts (with or without host names):
-    When `parse_query` is called with each variation,
-    Then the parser should correctly identify the user, repo, canonical URL, and other default fields.
-    """
-    for url in urls:
-        query = await parse_query(url, max_file_size=50, from_web=True)
+    expected = {
+        "user_name": user,
+        "repo_name": repo,
+        "url": expected_url,
+        "slug": f"{user}-{repo}",
+        "subpath": "/",
+        "type": None,
+        "branch": None,
+        "commit": None,
+        "max_file_size": 50,
+        "include_patterns": None,
+    }
 
-        assert query.user_name == expected_user
-        assert query.repo_name == expected_repo
-        assert query.url == expected_url
-        assert query.slug == f"{expected_user}-{expected_repo}"
-        assert query.id is not None
-        assert query.subpath == "/"
-        assert query.branch is None
-        assert query.commit is None
-        assert query.type is None
+    assert actual == expected