Refactor exercise unit test by using helper functions to mock repos (#284)

Author: desmondwong1215

clone_repo/test_verify.py

@@ -1,11 +1,8 @@
-import json
-from pathlib import Path
 from unittest.mock import patch
 
 import pytest
 from exercise_utils.test import GitAutograderTestLoader, assert_output
-from git.repo import Repo
-from git_autograder import GitAutograderExercise, GitAutograderWrongAnswerException
+from git_autograder import GitAutograderWrongAnswerException
 from git_autograder.status import GitAutograderStatus
 
 from .verify import (
@@ -28,49 +25,15 @@
 # we directly mock function calls to verify that all branches are covered for us.
 
 
-# TODO: The current tooling isn't mature enough to handle mock GitAutograderExercise in
-# cases like these. We would ideally need some abstraction rather than creating our own.
-
-
-@pytest.fixture
-def exercise(tmp_path: Path) -> GitAutograderExercise:
-    repo_dir = tmp_path / "ignore-me"
-    repo_dir.mkdir()
-
-    Repo.init(repo_dir)
-    with open(tmp_path / ".gitmastery-exercise.json", "a") as config_file:
-        config_file.write(
-            json.dumps(
-                {
-                    "exercise_name": "clone-repo",
-                    "tags": [],
-                    "requires_git": True,
-                    "requires_github": True,
-                    "base_files": {},
-                    "exercise_repo": {
-                        "repo_type": "ignore",
-                        "repo_name": "ignore-me",
-                        "init": True,
-                        "create_fork": None,
-                        "repo_title": None,
-                    },
-                    "downloaded_at": None,
-                }
-            )
-        )
-
-    exercise = GitAutograderExercise(exercise_path=tmp_path)
-    return exercise
-
-
-def test_pass(exercise: GitAutograderExercise):
+def test_pass():
     fake_origin = type(
         "FakeRemote", (), {"url": "https://github.com/dummy/gm-shapes.git"}
     )()
     fake_upstream = type(
         "FakeRemote", (), {"url": "https://github.com/git-mastery/gm-shapes.git"}
     )()
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
@@ -90,56 +53,62 @@ def test_pass(exercise: GitAutograderExercise):
         assert_output(output, GitAutograderStatus.SUCCESSFUL)
 
 
-def test_improper_gh_setup(exercise: GitAutograderExercise):
+def test_improper_gh_setup():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value=None),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
 
-    assert exception.value.message == [IMPROPER_GH_CLI_SETUP]
+        assert exception.value.message == [IMPROPER_GH_CLI_SETUP]
 
 
-def test_no_fork(exercise: GitAutograderExercise):
+def test_no_fork():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=False),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
 
-    assert exception.value.message == [NO_FORK_FOUND]
+        assert exception.value.message == [NO_FORK_FOUND]
 
 
-def test_not_right_parent(exercise: GitAutograderExercise):
+def test_not_right_parent():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=False),
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
 
-    assert exception.value.message == [NOT_GIT_MASTERY_FORK]
+        assert exception.value.message == [NOT_GIT_MASTERY_FORK]
 
 
-def test_missing_shapes_folder(exercise: GitAutograderExercise):
+def test_missing_shapes_folder():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
         patch("clone_repo.verify.has_shapes_folder", return_value=False),
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
-    assert exception.value.message == [CLONE_MISSING]
 
+        assert exception.value.message == [CLONE_MISSING]
 
-def test_missing_origin_remote(exercise: GitAutograderExercise):
+
+def test_missing_origin_remote():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
@@ -151,12 +120,14 @@ def test_missing_origin_remote(exercise: GitAutograderExercise):
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
-    assert exception.value.message == [ORIGIN_MISSING]
+
+        assert exception.value.message == [ORIGIN_MISSING]
 
 
-def test_wrong_origin_remote_url(exercise: GitAutograderExercise):
+def test_wrong_origin_remote_url():
     fake_origin = type("FakeRemote", (), {"url": "https://github.com/wrong/repo.git"})()
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
@@ -168,14 +139,16 @@ def test_wrong_origin_remote_url(exercise: GitAutograderExercise):
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
-    assert exception.value.message == [ORIGIN_WRONG]
+
+        assert exception.value.message == [ORIGIN_WRONG]
 
 
-def test_missing_upstream_remote(exercise: GitAutograderExercise):
+def test_missing_upstream_remote():
     fake_origin = type(
         "FakeRemote", (), {"url": "https://github.com/dummy/gm-shapes.git"}
     )()
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
@@ -187,17 +160,19 @@ def test_missing_upstream_remote(exercise: GitAutograderExercise):
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
-    assert exception.value.message == [UPSTREAM_MISSING]
 
+        assert exception.value.message == [UPSTREAM_MISSING]
 
-def test_wrong_upstream_remote_url(exercise: GitAutograderExercise):
+
+def test_wrong_upstream_remote_url():
     fake_origin = type(
         "FakeRemote", (), {"url": "https://github.com/dummy/gm-shapes.git"}
     )()
     fake_upstream = type(
         "FakeRemote", (), {"url": "https://github.com/wrong/repo.git"}
     )()
     with (
+        loader.start_mock_exercise() as exercise,
         patch("clone_repo.verify.get_username", return_value="dummy"),
         patch("clone_repo.verify.has_fork", return_value=True),
         patch("clone_repo.verify.is_parent_git_mastery", return_value=True),
@@ -209,4 +184,5 @@ def test_wrong_upstream_remote_url(exercise: GitAutograderExercise):
         pytest.raises(GitAutograderWrongAnswerException) as exception,
     ):
         verify(exercise)
-    assert exception.value.message == [UPSTREAM_WRONG]
+
+        assert exception.value.message == [UPSTREAM_WRONG]

exercise_utils/test.py

@@ -1,3 +1,4 @@
+import json
 import os
 import tempfile
 from contextlib import contextmanager
@@ -275,6 +276,71 @@ def start(
             with test as (ctx, rs, rs_remote):
                 yield ctx, rs
 
+    @contextmanager
+    def start_mock_exercise(
+        self,
+        *,
+        tags: Optional[List[str]] = None,
+        requires_git: bool = True,
+        requires_github: bool = True,
+        base_files: Optional[Dict[str, str]] = None,
+        repo_type: str = "local",
+        repo_name: str = "ignore-me",
+        init: bool = True,
+        create_fork: Optional[bool] = None,
+        repo_title: Optional[str] = None,
+        has_pr_context: bool = False,
+        pr_number: Optional[int] = None,
+        pr_repo_full_name: Optional[str] = None,
+        downloaded_at: Optional[str] = None,
+    ) -> Iterator[GitAutograderExercise]:
+        with tempfile.TemporaryDirectory() as temp_dir:
+            exercise_path = Path(temp_dir)
+            repo_dir = exercise_path / repo_name
+            repo_dir.mkdir(parents=True, exist_ok=True)
+
+            if repo_type == "local":
+                repo_dir.mkdir(parents=True, exist_ok=True)
+                if init:
+                    Repo.init(repo_dir)
+
+            exercise_repo: Dict[str, Any] = {
+                "repo_type": repo_type,
+                "repo_name": repo_name,
+                "init": init,
+                "create_fork": create_fork,
+                "repo_title": repo_title,
+            }
+            config: Dict[str, Any] = {
+                "exercise_name": self.exercise_name,
+                "tags": tags or [],
+                "requires_git": requires_git,
+                "requires_github": requires_github,
+                "base_files": base_files or {},
+                "exercise_repo": exercise_repo,
+                "downloaded_at": downloaded_at,
+            }
+
+            if has_pr_context:
+                # If the user does not provide PR context, dummy values will be used.
+                if pr_number is None:
+                    pr_number = 1
+                if pr_repo_full_name is None:
+                    pr_repo_full_name = "dummy/repo"
+                exercise_repo["pr_number"] = pr_number
+                exercise_repo["pr_repo_full_name"] = pr_repo_full_name
+            with open(exercise_path / ".gitmastery-exercise.json", "w") as f:
+                json.dump(config, f)
+
+            if has_pr_context:
+                with mock.patch(
+                    "git_autograder.pr.fetch_pull_request_data",
+                    return_value={},
+                ):
+                    yield GitAutograderExercise(exercise_path=exercise_path)
+            else:
+                yield GitAutograderExercise(exercise_path=exercise_path)
+
 
 def assert_output(
     output: GitAutograderOutput,

fork_repo/test_verify.py

@@ -1,14 +1,8 @@
-import json
-from pathlib import Path
 from unittest.mock import patch
 
 import pytest
 from exercise_utils.test import GitAutograderTestLoader, assert_output
-from git.repo import Repo
-from git_autograder import (
-    GitAutograderExercise,
-    GitAutograderWrongAnswerException,
-)
+from git_autograder import GitAutograderWrongAnswerException
 from git_autograder.status import GitAutograderStatus
 
 from .verify import IMPROPER_GH_CLI_SETUP, NO_FORK_FOUND, NOT_GIT_MASTERY_FORK, verify
@@ -21,43 +15,9 @@
 # we directly mock function calls to verify that all branches are covered for us.
 
 
-# TODO: The current tooling isn't mature enough to handle mock GitAutograderExercise in
-# cases like these. We would ideally need some abstraction rather than creating our own.
-
-
-@pytest.fixture
-def exercise(tmp_path: Path) -> GitAutograderExercise:
-    repo_dir = tmp_path / "ignore-me"
-    repo_dir.mkdir()
-
-    Repo.init(repo_dir)
-    with open(tmp_path / ".gitmastery-exercise.json", "a") as config_file:
-        config_file.write(
-            json.dumps(
-                {
-                    "exercise_name": "remote-control",
-                    "tags": [],
-                    "requires_git": True,
-                    "requires_github": True,
-                    "base_files": {},
-                    "exercise_repo": {
-                        "repo_type": "local",
-                        "repo_name": "ignore-me",
-                        "init": True,
-                        "create_fork": None,
-                        "repo_title": None,
-                    },
-                    "downloaded_at": None,
-                }
-            )
-        )
-
-    exercise = GitAutograderExercise(exercise_path=tmp_path)
-    return exercise
-
-
-def test_pass(exercise: GitAutograderExercise):
+def test_pass():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("fork_repo.verify.get_username", return_value="dummy"),
         patch("fork_repo.verify.has_fork", return_value=True),
         patch("fork_repo.verify.is_parent_git_mastery", return_value=True),
@@ -66,8 +26,9 @@ def test_pass(exercise: GitAutograderExercise):
         assert_output(output, GitAutograderStatus.SUCCESSFUL)
 
 
-def test_improper_gh_setup(exercise: GitAutograderExercise):
+def test_improper_gh_setup():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("fork_repo.verify.get_username", return_value=None),
         patch("fork_repo.verify.has_fork", return_value=True),
         patch("fork_repo.verify.is_parent_git_mastery", return_value=True),
@@ -78,8 +39,9 @@ def test_improper_gh_setup(exercise: GitAutograderExercise):
     assert exception.value.message == [IMPROPER_GH_CLI_SETUP]
 
 
-def test_no_fork(exercise: GitAutograderExercise):
+def test_no_fork():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("fork_repo.verify.get_username", return_value="dummy"),
         patch("fork_repo.verify.has_fork", return_value=False),
         patch("fork_repo.verify.is_parent_git_mastery", return_value=True),
@@ -90,8 +52,9 @@ def test_no_fork(exercise: GitAutograderExercise):
     assert exception.value.message == [NO_FORK_FOUND]
 
 
-def test_not_right_parent(exercise: GitAutograderExercise):
+def test_not_right_parent():
     with (
+        loader.start_mock_exercise() as exercise,
         patch("fork_repo.verify.get_username", return_value="dummy"),
         patch("fork_repo.verify.has_fork", return_value=True),
         patch("fork_repo.verify.is_parent_git_mastery", return_value=False),