diff --git a/src/lando/api/tests/conftest.py b/src/lando/api/tests/conftest.py index cfcc4e1a..89a27f62 100644 --- a/src/lando/api/tests/conftest.py +++ b/src/lando/api/tests/conftest.py @@ -30,8 +30,13 @@ from lando.main.models import SCM_LEVEL_1, SCM_LEVEL_3, Profile, Repo from lando.main.scm import SCM_TYPE_HG from lando.main.support import LegacyAPIException +from lando.main.tests.conftest import git_repo, git_repo_seed from lando.utils.phabricator import PhabricatorClient +# We need some local usage of those imported fixtures to satisfy the linters. +# This is it. +__all__ = ["git_repo", "git_repo_seed"] + PATCH_NORMAL_1 = r""" # HG changeset patch # User Test User @@ -45,7 +50,7 @@ @@ -1,1 +1,2 @@ TEST +adding another line -""".strip() +""".lstrip() PATCH_NORMAL_2 = r""" # HG changeset patch @@ -61,7 +66,7 @@ TEST adding another line +adding one more line -""".strip() +""".lstrip() PATCH_NORMAL_3 = r""" # HG changeset patch @@ -82,7 +87,7 @@ +++ b/blah.txt @@ -0,0 +1,1 @@ +TEST -""".strip() +""".lstrip() @pytest.fixture diff --git a/src/lando/api/tests/test_landings.py b/src/lando/api/tests/test_landings.py index e984702b..01003ba3 100644 --- a/src/lando/api/tests/test_landings.py +++ b/src/lando/api/tests/test_landings.py @@ -1,7 +1,10 @@ import io +import pathlib import textwrap import unittest.mock as mock +from collections.abc import Callable +import py import pytest from lando.api.legacy.workers.landing_worker import ( @@ -15,8 +18,11 @@ add_job_with_revisions, ) from lando.main.models.revision import Revision -from lando.main.scm import SCM_TYPE_HG, HgSCM -from lando.main.scm.hg import LostPushRace +from lando.main.scm import SCM_TYPE_HG +from lando.main.scm.abstract_scm import AbstractSCM +from lando.main.scm.consts import SCM_TYPE_GIT +from lando.main.scm.git import GitSCM +from lando.main.scm.hg import HgSCM, LostPushRace from lando.utils import HgPatchHelper @@ -28,6 +34,13 @@ def create_patch_revision(normal_patch): normal_patch_0 = normal_patch(0) def _create_patch_revision(number, patch=normal_patch_0): + """Create revision number `number`, with patch text `patch`. + + `patch` will default to the first normal patch fixture if unspecified. However, + if explicitly set to None, the `normal_patch` fixture will be used to get + normal patch number `number-1`.""" + if not patch: + patch = normal_patch(number - 1) revision = Revision() revision.revision_id = number revision.diff_id = number @@ -54,7 +67,7 @@ def _create_patch_revision(number, patch=normal_patch_0): @@ -1,1 +1,2 @@ TEST +{LARGE_UTF8_THING} -""".strip() +""".lstrip() PATCH_WITHOUT_STARTLINE = r""" # HG changeset patch @@ -68,7 +81,7 @@ def _create_patch_revision(number, patch=normal_patch_0): @@ -1,1 +1,2 @@ TEST +adding another line -""".strip() +""".lstrip() PATCH_PUSH_LOSER = r""" @@ -85,7 +98,7 @@ def _create_patch_revision(number, patch=normal_patch_0): @@ -1,1 +1,2 @@ TEST +adding one more line again -""".strip() +""".lstrip() PATCH_FORMATTING_PATTERN_PASS = r""" # HG changeset patch @@ -139,7 +152,7 @@ def _create_patch_revision(number, patch=normal_patch_0): + f.write("".join(stdout_content)) + sys.exit(0) -""".strip() +""".lstrip() PATCH_FORMATTING_PATTERN_FAIL = r""" # HG changeset patch @@ -172,7 +185,7 @@ def _create_patch_revision(number, patch=normal_patch_0): +sys.exit("MACH FAILED") + -""".strip() +""".lstrip() PATCH_FORMATTED_1 = r""" # HG changeset patch @@ -190,7 +203,7 @@ def _create_patch_revision(number, patch=normal_patch_0): + + +adding another line -""".strip() +""".lstrip() PATCH_FORMATTED_2 = r""" # HG changeset patch @@ -208,7 +221,7 @@ def _create_patch_revision(number, patch=normal_patch_0): adding another line +add one more line -""".strip() # noqa: W293 +""".lstrip() # noqa: W293 TESTTXT_FORMATTED_1 = b""" TeSt @@ -226,38 +239,145 @@ def _create_patch_revision(number, patch=normal_patch_0): """.lstrip() +@pytest.mark.django_db +def hg_repo_mc( + hg_server: str, + hg_clone: py.path, + *, + approval_required: bool = False, + autoformat_enabled: bool = False, + force_push: bool = False, + push_target: str = "", +) -> Repo: + params = { + "required_permission": SCM_LEVEL_3, + "url": hg_server, + "push_path": hg_server, + "pull_path": hg_server, + "system_path": hg_clone.strpath, + # The option below can be overriden in the parameters + "approval_required": approval_required, + "autoformat_enabled": autoformat_enabled, + "force_push": force_push, + "push_target": push_target, + } + repo = Repo.objects.create( + scm_type=SCM_TYPE_HG, + name="mozilla-central-hg", + **params, + ) + repo.save() + return repo + + +@pytest.mark.django_db +def git_repo_mc( + git_repo: pathlib.Path, + tmp_path: pathlib.Path, + *, + approval_required: bool = False, + autoformat_enabled: bool = False, + force_push: bool = False, + push_target: str = "", +) -> Repo: + repos_dir = tmp_path / "repos" + repos_dir.mkdir() + + params = { + "required_permission": SCM_LEVEL_3, + "url": str(git_repo), + "push_path": str(git_repo), + "pull_path": str(git_repo), + "system_path": repos_dir / "git_repo", + # The option below can be overriden in the parameters + "approval_required": approval_required, + "autoformat_enabled": autoformat_enabled, + "force_push": force_push, + "push_target": push_target, + } + + repo = Repo.objects.create( + scm_type=SCM_TYPE_GIT, + name="mozilla-central-git", + **params, + ) + repo.save() + repo.scm.prepare_repo(repo.pull_path) + return repo + + +@pytest.fixture() +def repo_mc( + # Git + git_repo: pathlib.Path, + tmp_path: pathlib.Path, + # Hg + hg_server: str, + hg_clone: py.path, +) -> Callable: + def factory( + scm_type: str, + *, + approval_required: bool = False, + autoformat_enabled: bool = False, + force_push: bool = False, + push_target: str = "", + ) -> Repo: + params = { + "approval_required": approval_required, + "autoformat_enabled": autoformat_enabled, + "force_push": force_push, + "push_target": push_target, + } + + if scm_type == SCM_TYPE_GIT: + return git_repo_mc(git_repo, tmp_path, **params) + elif scm_type == SCM_TYPE_HG: + return hg_repo_mc(hg_server, hg_clone, **params) + raise Exception(f"Unknown SCM Type {scm_type=}") + + return factory + + @pytest.mark.parametrize( - "revisions_params", + "repo_type,revisions_params", [ - [ - (1, {}), - (2, {}), - ], - [(1, {"patch": LARGE_PATCH})], + # Git + ( + SCM_TYPE_GIT, + [ + (1, {"patch": None}), + (2, {"patch": None}), + ], + ), + (SCM_TYPE_GIT, [(1, {"patch": LARGE_PATCH})]), + # Hg + ( + SCM_TYPE_HG, + [ + (1, {"patch": None}), + (2, {"patch": None}), + ], + ), + (SCM_TYPE_HG, [(1, {"patch": LARGE_PATCH})]), ], ) @pytest.mark.django_db def test_integrated_execute_job( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, revisions_params, ): - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - url=hg_server, - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type) + treestatusdouble.open_tree(repo.name) + revisions = [ create_patch_revision(number, **kwargs) for number, kwargs in revisions_params ] + job_params = { "status": LandingJobStatus.IN_PROGRESS, "requester_email": "test@example.com", @@ -283,26 +403,25 @@ def test_integrated_execute_job( ), "Successful landing should trigger Phab repo update." +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_integrated_execute_job_with_force_push( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - url=hg_server, - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - force_push=True, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type, force_push=True) + treestatusdouble.open_tree(repo.name) scm = repo.scm + job_params = { "status": LandingJobStatus.IN_PROGRESS, "requester_email": "test@example.com", @@ -325,30 +444,29 @@ def test_integrated_execute_job_with_force_push( assert scm.push.call_count == 1 assert len(scm.push.call_args) == 2 assert len(scm.push.call_args[0]) == 1 - assert scm.push.call_args[0][0] == hg_server + assert scm.push.call_args[0][0] == repo.url assert scm.push.call_args[1] == {"push_target": "", "force_push": True} +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_integrated_execute_job_with_bookmark( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - url=hg_server, - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - push_target="@", - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type, push_target="@") + treestatusdouble.open_tree(repo.name) scm = repo.scm + job_params = { "status": LandingJobStatus.IN_PROGRESS, "requester_email": "test@example.com", @@ -371,28 +489,28 @@ def test_integrated_execute_job_with_bookmark( assert scm.push.call_count == 1 assert len(scm.push.call_args) == 2 assert len(scm.push.call_args[0]) == 1 - assert scm.push.call_args[0][0] == hg_server + assert scm.push.call_args[0][0] == repo.url assert scm.push.call_args[1] == {"push_target": "@", "force_push": False} +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_no_diff_start_line( - hg_server, - hg_clone, + repo_mc, treestatusdouble, create_patch_revision, caplog, + repo_type: str, ): - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - url=hg_server, - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type) + treestatusdouble.open_tree(repo.name) + job_params = { "id": 1234, "status": LandingJobStatus.IN_PROGRESS, @@ -411,25 +529,25 @@ def test_no_diff_start_line( assert "Patch without a diff start line." in caplog.text +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_lose_push_race( monkeypatch, - hg_server, - hg_clone, + repo_mc, treestatusdouble, create_patch_revision, + repo_type: str, ): - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - url=hg_server, - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type) + treestatusdouble.open_tree(repo.name) scm = repo.scm + job_params = { "id": 1234, "status": LandingJobStatus.IN_PROGRESS, @@ -456,26 +574,26 @@ def test_lose_push_race( assert job.status == LandingJobStatus.DEFERRED +@pytest.mark.parametrize( + "repo_type", + [ + # Bug 1940876 + # SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_merge_conflict( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, - normal_patch, caplog, + repo_type: str, ): - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - url=hg_server, - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type) + treestatusdouble.open_tree(repo.name) + job_params = { "id": 1234, "status": LandingJobStatus.IN_PROGRESS, @@ -519,27 +637,25 @@ def test_merge_conflict( ), f"Empty or missing reject content for failed path {fp}" +@pytest.mark.parametrize( + "repo_type", + [ + # Bug 1940876 + # SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_failed_landing_job_notification( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): """Ensure that a failed landings triggers a user notification.""" - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - required_permission=SCM_LEVEL_3, - push_path=hg_server, - pull_path=hg_server, - approval_required=True, - autoformat_enabled=False, - system_path=hg_clone.strpath, - ) - + repo = repo_mc(repo_type, approval_required=True, autoformat_enabled=False) + treestatusdouble.open_tree(repo.name) scm = repo.scm # Mock `scm.update_repo` so we can force a failed landing. @@ -629,28 +745,25 @@ def test_landing_worker__extract_error_data(): assert rejects_paths == expected_rejects_paths +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_format_patch_success_unchanged( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, normal_patch, + repo_type: str, ): """Tests automated formatting happy path where formatters made no changes.""" - tree = "mozilla-central" - treestatusdouble.open_tree(tree) - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name=tree, - url=hg_server, - push_path=hg_server, - pull_path=hg_server, - required_permission=SCM_LEVEL_3, - autoformat_enabled=True, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type, autoformat_enabled=True) + treestatusdouble.open_tree(repo.name) revisions = [ create_patch_revision(1, patch=PATCH_FORMATTING_PATTERN_PASS), @@ -685,42 +798,37 @@ def test_format_patch_success_unchanged( ), "Autoformat making no changes should leave `formatted_replacements` empty." +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_format_single_success_changed( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): """Test formatting a single commit via amending.""" - tree = "mozilla-central" - treestatusdouble.open_tree(tree) - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name=tree, - url=hg_server, - push_path=hg_server, - pull_path=hg_server, - required_permission=SCM_LEVEL_3, - autoformat_enabled=True, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type, autoformat_enabled=True) + treestatusdouble.open_tree(repo.name) + scm = repo.scm # Push the `mach` formatting patch. - hgrepo = HgSCM(hg_clone.strpath) - with hgrepo.for_push("test@example.com"): + with scm.for_push("test@example.com"): ph = HgPatchHelper(io.StringIO(PATCH_FORMATTING_PATTERN_PASS)) - hgrepo.apply_patch( + scm.apply_patch( ph.get_diff(), ph.get_commit_description(), ph.get_header("User"), ph.get_header("Date"), ) - hgrepo.push(repo.push_path) - pre_landing_tip = hgrepo.run_hg(["log", "-r", "tip", "-T", "{node}"]).decode( - "utf-8" - ) + scm.push(repo.push_path) + pre_landing_tip = scm.head_ref() # Upload a patch for formatting. job_params = { @@ -750,21 +858,15 @@ def test_format_single_success_changed( mock_trigger_update.call_count == 1 ), "Successful landing should trigger Phab repo update." - with hgrepo.for_push(job.requester_email): - repo_root = hgrepo.run_hg(["root"]).decode("utf-8").strip() - + with scm.for_push(job.requester_email): # Get the commit message. - desc = hgrepo.run_hg(["log", "-r", "tip", "-T", "{desc}"]).decode("utf-8") + desc = _scm_get_last_commit_message(scm) # Get the content of the file after autoformatting. - tip_content = hgrepo.run_hg( - ["cat", "--cwd", repo_root, "-r", "tip", "test.txt"] - ) + tip_content = scm.read_checkout_file("test.txt").encode("utf-8") # Get the hash behind the tip commit. - hash_behind_current_tip = hgrepo.run_hg( - ["log", "-r", "tip^", "-T", "{node}"] - ).decode("utf-8") + hash_behind_current_tip = _scm_get_previous_hash(scm) assert tip_content == TESTTXT_FORMATTED_1, "`test.txt` is incorrect in base commit." @@ -777,29 +879,36 @@ def test_format_single_success_changed( ), "Autoformat via amending should only land a single commit." +def _scm_get_previous_hash(scm: AbstractSCM) -> str: + """Get the second last commit hash. + + This is implementation-specific code, but is only used in the tests. Rather than + extending the AbstractSCM for the sole purpose of support test assertions, we keep + this in the test code instead.""" + if isinstance(scm, HgSCM): + return HgSCM.run_hg(scm, ["log", "-r", "tip^", "-T", "{node}"]).decode("utf-8") + return GitSCM._git_run("rev-parse", "HEAD^", cwd=scm.path) + + +@pytest.mark.parametrize( + "repo_type", + [ + SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_format_stack_success_changed( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): """Test formatting a stack via an autoformat tip commit.""" - tree = "mozilla-central" - treestatusdouble.open_tree(tree) - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name=tree, - url=hg_server, - push_path=hg_server, - pull_path=hg_server, - required_permission=SCM_LEVEL_3, - autoformat_enabled=True, - system_path=hg_clone.strpath, - ) - - hgrepo = HgSCM(hg_clone.strpath) + repo = repo_mc(repo_type, autoformat_enabled=True) + treestatusdouble.open_tree(repo.name) + scm = repo.scm revisions = [ create_patch_revision(1, patch=PATCH_FORMATTING_PATTERN_PASS), @@ -831,16 +940,12 @@ def test_format_stack_success_changed( mock_trigger_update.call_count == 1 ), "Successful landing should trigger Phab repo update." - with hgrepo.for_push(job.requester_email): - repo_root = hgrepo.run_hg(["root"]).decode("utf-8").strip() - + with scm.for_push(job.requester_email): # Get the commit message. - desc = hgrepo.run_hg(["log", "-r", "tip", "-T", "{desc}"]).decode("utf-8") + desc = _scm_get_last_commit_message(scm) # Get the content of the file after autoformatting. - rev3_content = hgrepo.run_hg( - ["cat", "--cwd", repo_root, "-r", "tip", "test.txt"] - ) + rev3_content = scm.read_checkout_file("test.txt").encode("utf-8") assert ( rev3_content == TESTTXT_FORMATTED_2 @@ -855,27 +960,36 @@ def test_format_stack_success_changed( ), "Autoformat commit has incorrect commit message." +def _scm_get_last_commit_message(scm: AbstractSCM) -> str: + """Get the last commit message. + + This is implementation-specific code, but is only used in the tests. Rather than + extending the AbstractSCM for the sole purpose of support test assertions, we keep + this in the test code instead.""" + if isinstance(scm, HgSCM): + return HgSCM.run_hg(scm, ["log", "-r", "tip", "-T", "{desc}"]).decode("utf-8") + return GitSCM._git_run("log", "--pretty=%B", "HEAD^..", cwd=scm.path) + + +@pytest.mark.parametrize( + "repo_type", + [ + # Bug 1940876 + # SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_format_patch_fail( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): """Tests automated formatting failures before landing.""" - tree = "mozilla-central" - treestatusdouble.open_tree(tree) - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name=tree, - required_permission=SCM_LEVEL_3, - url=hg_server, - push_path=hg_server, - pull_path=hg_server, - autoformat_enabled=True, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type, autoformat_enabled=True) + treestatusdouble.open_tree(repo.name) revisions = [ create_patch_revision(1, patch=PATCH_FORMATTING_PATTERN_FAIL), @@ -913,26 +1027,25 @@ def test_format_patch_fail( ), "User should be notified their landing was unsuccessful due to autoformat." +@pytest.mark.parametrize( + "repo_type", + [ + # Bug 1940876 + # SCM_TYPE_GIT, + SCM_TYPE_HG, + ], +) @pytest.mark.django_db def test_format_patch_no_landoini( - hg_server, - hg_clone, + repo_mc, treestatusdouble, monkeypatch, create_patch_revision, + repo_type: str, ): """Tests behaviour of Lando when the `.lando.ini` file is missing.""" - treestatusdouble.open_tree("mozilla-central") - repo = Repo.objects.create( - scm_type=SCM_TYPE_HG, - name="mozilla-central", - required_permission=SCM_LEVEL_3, - url=hg_server, - push_path=hg_server, - pull_path=hg_server, - autoformat_enabled=True, - system_path=hg_clone.strpath, - ) + repo = repo_mc(repo_type, autoformat_enabled=True) + treestatusdouble.open_tree(repo.name) revisions = [ create_patch_revision(1), diff --git a/src/lando/main/scm/git.py b/src/lando/main/scm/git.py index 5fa2adb5..64d96282 100644 --- a/src/lando/main/scm/git.py +++ b/src/lando/main/scm/git.py @@ -14,6 +14,7 @@ from lando.main.scm.consts import SCM_TYPE_GIT from lando.main.scm.exceptions import SCMException +from lando.settings import LANDO_USER_EMAIL, LANDO_USER_NAME from .abstract_scm import AbstractSCM @@ -69,6 +70,12 @@ def clone(self, source: str): """Clone a repository from a source.""" # When cloning, self.path doesn't exist yet, so we need to use another CWD. self._git_run("clone", source, self.path, cwd="/") + self._git_setup_user() + + def _git_setup_user(self): + """Configure the git user locally to repo_dir so as not to mess with the real user's configuration.""" + self._git_run("config", "user.name", LANDO_USER_NAME, cwd=self.path) + self._git_run("config", "user.email", LANDO_USER_EMAIL, cwd=self.path) def push( self, @@ -145,8 +152,10 @@ def apply_patch( self, diff: str, commit_description: str, commit_author: str, commit_date: str ): """Apply the given patch to the current repository.""" - f_msg = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+") - f_diff = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+") + f_msg = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+", suffix=".msg") + f_diff = tempfile.NamedTemporaryFile( + encoding="utf-8", mode="w+", suffix=".diff" + ) with f_msg, f_diff: f_msg.write(commit_description) f_msg.flush() @@ -210,8 +219,17 @@ def update_repo(self, pull_path: str, target_cset: Optional[str] = None) -> str: """ branch = target_cset or self.default_branch self.clean_repo() - self._git_run("pull", "--prune", pull_path, cwd=self.path) - self._git_run("checkout", "--force", "-B", branch, cwd=self.path) + # Fetch all refs at the given pull_path, and overwrite the `origin` references. + self._git_run( + "fetch", + "--prune", + pull_path, + "+refs/heads/*:refs/remotes/origin/*", + cwd=self.path, + ) + self._git_run( + "checkout", "--force", "-B", branch, f"origin/{branch}", cwd=self.path + ) return self.head_ref() def clean_repo(self, *, strip_non_public_commits: bool = True): @@ -229,7 +247,13 @@ def format_stack_amend(self) -> Optional[list[str]]: def format_stack_tip(self, commit_message: str) -> Optional[list[str]]: """Add an autoformat commit to the top of the patch stack.""" - self._git_run("commit", "--all", "--message", commit_message, cwd=self.path) + try: + self._git_run("commit", "--all", "--message", commit_message, cwd=self.path) + except SCMException as exc: + if "nothing to commit, working tree clean" in exc.out: + return [] + else: + raise exc return [self.get_current_node()] def get_current_node(self) -> str: diff --git a/src/lando/main/scm/hg.py b/src/lando/main/scm/hg.py index 4d0ed89b..6cb89d68 100644 --- a/src/lando/main/scm/hg.py +++ b/src/lando/main/scm/hg.py @@ -232,8 +232,8 @@ def apply_patch( """Apply the given patch to the current repository.""" # Import the diff to apply the changes then commit separately to # ensure correct parsing of the commit message. - f_msg = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+") - f_diff = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+") + f_msg = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+", suffix="msg") + f_diff = tempfile.NamedTemporaryFile(encoding="utf-8", mode="w+", suffix="diff") with f_msg, f_diff: f_msg.write(commit_description) f_msg.flush() diff --git a/src/lando/main/tests/conftest.py b/src/lando/main/tests/conftest.py index cc44bf02..d001e23e 100644 --- a/src/lando/main/tests/conftest.py +++ b/src/lando/main/tests/conftest.py @@ -1,11 +1,22 @@ -import pathlib import subprocess +from pathlib import Path import pytest @pytest.fixture -def git_repo(tmp_path: pathlib.Path): +def git_repo_seed() -> Path: + """ + Return the path to a patch to set up a base git repo for tests. + + The diff can apply on an empty repo to create a known base for application + of other patches as part of the tests. + """ + return Path(__file__).parent / "data" / "test-repo.patch" + + +@pytest.fixture +def git_repo(tmp_path: Path, git_repo_seed: Path) -> Path: """ Creates a temporary Git repository for testing purposes. @@ -18,11 +29,11 @@ def git_repo(tmp_path: pathlib.Path): repo_dir = tmp_path / "git_repo" subprocess.run(["git", "init", repo_dir], check=True) subprocess.run(["git", "branch", "-m", "main"], check=True, cwd=repo_dir) - file = repo_dir / "first" - file.write_text("first file!") _git_setup_user(repo_dir) - subprocess.run(["git", "add", file.name], check=True, cwd=repo_dir) - subprocess.run(["git", "commit", "-m", "first commit"], check=True, cwd=repo_dir) + _git_ignore_denyCurrentBranch(repo_dir) + subprocess.run(["git", "am", str(git_repo_seed)], check=True, cwd=repo_dir) + subprocess.run(["git", "show"], check=True, cwd=repo_dir) + subprocess.run(["git", "branch"], check=True, cwd=repo_dir) return repo_dir @@ -31,7 +42,7 @@ def git_setup_user(): return _git_setup_user -def _git_setup_user(repo_dir): +def _git_setup_user(repo_dir: Path): """Configure the git user locally to repo_dir so as not to mess with the real user's configuration.""" subprocess.run(["git", "config", "user.name", "Py Test"], check=True, cwd=repo_dir) subprocess.run( @@ -39,3 +50,17 @@ def _git_setup_user(repo_dir): check=True, cwd=repo_dir, ) + + +def _git_ignore_denyCurrentBranch(repo_dir: Path): + """Disable error when pushing to this non-bare repo. + + This is a sane protection in general, but it gets in the way of the tests here, + where we just want a target, and don't care much about the final state of this + target after everything is done. + """ + subprocess.run( + ["git", "config", "receive.denyCurrentBranch", "ignore"], + check=True, + cwd=repo_dir, + ) diff --git a/src/lando/main/tests/data/test-repo.patch b/src/lando/main/tests/data/test-repo.patch new file mode 100644 index 00000000..8bf178df --- /dev/null +++ b/src/lando/main/tests/data/test-repo.patch @@ -0,0 +1,23 @@ +# HG changeset patch +# User Test User +# Date 1736311725 -39600 +# Wed Jan 08 15:48:45 2025 +1100 +# Node ID 4e379f8d3278a6645a5cf706cd8555da195b089e +# Parent 0000000000000000000000000000000000000000 +initial commit + + +add another file + +diff --git a/README b/README +new file mode 100644 +--- /dev/null ++++ b/README +@@ -0,0 +1,1 @@ ++HELLO +diff --git a/test.txt b/test.txt +new file mode 100644 +--- /dev/null ++++ b/test.txt +@@ -0,0 +1,1 @@ ++TEST diff --git a/src/lando/main/tests/test_git.py b/src/lando/main/tests/test_git.py index 5625b8e3..a879714d 100644 --- a/src/lando/main/tests/test_git.py +++ b/src/lando/main/tests/test_git.py @@ -56,7 +56,7 @@ def test_GitSCM_clone(git_repo: Path, tmp_path: Path, monkeypatch): scm.clone(str(git_repo)) - mock_git_run.assert_called_with("clone", str(git_repo), str(clone_path), cwd="/") + mock_git_run.assert_any_call("clone", str(git_repo), str(clone_path), cwd="/") assert clone_path.exists(), f"New git clone {clone_path} wasn't created" assert ( clone_path / ".git" diff --git a/src/lando/tests/conftest.py b/src/lando/tests/conftest.py index c031804d..c9cd6cb0 100644 --- a/src/lando/tests/conftest.py +++ b/src/lando/tests/conftest.py @@ -1,13 +1,13 @@ -import pytest -from django.core.management import call_command - - -@pytest.fixture -def lando_version(): - # The version file may or may not exist in the testing environment. - # We'll explicitly generate it to ensure it's there. - call_command("generate_version_file") - - from lando.version import version - - return version +import pytest +from django.core.management import call_command + + +@pytest.fixture +def lando_version(): + # The version file may or may not exist in the testing environment. + # We'll explicitly generate it to ensure it's there. + call_command("generate_version_file") + + from lando.version import version + + return version