Merge branch 'zeid/bug-1944562-merge-lando-api' into zeid/bug-1944562…

…-lando-api-fixes

src/lando/main/scm/abstract_scm.py

@@ -153,6 +153,9 @@ def update_repo(self, pull_path: str, target_cset: Optional[str] = None) -> str:
         This method uses the Mercurial command to update the repository
         located at the given pull path to the specified target changeset.
 
+        The target changeset will be used as the base onto which the next commits will
+        be applied.
+
         Args:
             pull_path (str): The path to pull from.
             target_cset (str): The target changeset to update the repository to.

src/lando/main/scm/git.py

@@ -6,6 +6,7 @@
 import tempfile
 import uuid
 from contextlib import contextmanager
+from datetime import datetime
 from pathlib import Path
 from typing import Any, ContextManager, Optional
 
@@ -23,6 +24,9 @@
 
 logger = logging.getLogger(__name__)
 
+
+ISO8601_TIMESTAMP_BASIC = "%Y-%m-%dT%H%M%S%Z"
+
 ENV_COMMITTER_NAME = "GIT_COMMITTER_NAME"
 ENV_COMMITTER_EMAIL = "GIT_COMMITTER_EMAIL"
 
@@ -73,6 +77,7 @@ 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_run("checkout", self.default_branch, cwd=self.path)
         self._git_setup_user()
 
     def _git_setup_user(self):
@@ -111,8 +116,10 @@ def push(
 
         command += [push_path]
 
-        if push_target:
-            command += [f"HEAD:{push_target}"]
+        if not push_target:
+            push_target = self.default_branch
+
+        command += [f"HEAD:{push_target}"]
 
         self._git_run(*command, cwd=self.path)
 
@@ -267,8 +274,17 @@ def update_repo(self, pull_path: str, target_cset: Optional[str] = None) -> str:
 
         This method uses the Git commands to update the repository
         located at the given pull path to the specified target changeset.
+
+        A new work branch will be created, using the current date in its name.
         """
-        branch = target_cset or self.default_branch
+        if not target_cset:
+            target_cset = self.default_branch
+
+        remote_branch = f"origin/{target_cset}"
+        if self._git_run("branch", "--list", "--remote", remote_branch, cwd=self.path):
+            # If the branch exists remotely, make sure we get the up-to-date version.
+            target_cset = remote_branch
+
         self.clean_repo()
         # Fetch all refs at the given pull_path, and overwrite the `origin` references.
         self._git_run(
@@ -278,8 +294,13 @@ def update_repo(self, pull_path: str, target_cset: Optional[str] = None) -> str:
             "+refs/heads/*:refs/remotes/origin/*",
             cwd=self.path,
         )
+
+        # Create a new work branch, named after the current time, to work in.
+        # Ideally, we'd use the revision number, too, but it's not available to the SCM.
+        # A date is good enough for now, if we need to dig into issues.
+        work_branch = f"lando-{datetime.now().strftime(ISO8601_TIMESTAMP_BASIC)}"
         self._git_run(
-            "checkout", "--force", "-B", branch, f"origin/{branch}", cwd=self.path
+            "checkout", "--force", "-B", work_branch, target_cset, cwd=self.path
         )
         return self.head_ref()
 

src/lando/main/tests/conftest.py

@@ -32,8 +32,14 @@ def git_repo(tmp_path: Path, git_repo_seed: Path) -> Path:
     _git_setup_user(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)
+
+    # Create a separate base branch for branch tests.
+    subprocess.run(["git", "checkout", "-b", "dev"], check=True, cwd=repo_dir)
+    subprocess.run(
+        ["git", "commit", "--allow-empty", "-m", "dev"], check=True, cwd=repo_dir
+    )
+
+    subprocess.run(["git", "checkout", "main"], check=True, cwd=repo_dir)
     return repo_dir
 
 

src/lando/main/tests/test_git.py

@@ -1,6 +1,7 @@
 import subprocess
 from collections.abc import Callable
 from pathlib import Path
+from typing import Optional
 from unittest.mock import MagicMock
 
 import pytest
@@ -48,8 +49,13 @@ def test_GitSCM_repo_is_supported(repo_path: str, expected: bool, git_repo: Path
     ), f"{scm} did not correctly report support for {repo_path}"
 
 
-def test_GitSCM_clone(git_repo: Path, tmp_path: Path, monkeypatch):
-    clone_path = tmp_path / "repo_test_GitSCM_clone"
+def test_GitSCM_clone(
+    git_repo: Path,
+    monkeypatch: pytest.MonkeyPatch,
+    request: pytest.FixtureRequest,
+    tmp_path: Path,
+):
+    clone_path = tmp_path / request.node.name
     scm = GitSCM(str(clone_path))
 
     mock_git_run = _monkeypatch_scm(monkeypatch, scm, "_git_run")
@@ -65,25 +71,23 @@ def test_GitSCM_clone(git_repo: Path, tmp_path: Path, monkeypatch):
 
 @pytest.mark.parametrize(
     "strip_non_public_commits",
-    (True, False),
+    [True, False],
 )
 def test_GitSCM_clean_repo(
     git_repo: Path,
-    tmp_path: Path,
     git_setup_user: Callable,
+    monkeypatch: pytest.MonkeyPatch,
+    request: pytest.FixtureRequest,
     strip_non_public_commits: bool,
-    monkeypatch,
+    tmp_path: Path,
 ):
-    clone_path = tmp_path / "repo_test_GitSCM_clean_repo"
+    clone_path = tmp_path / request.node.name
     clone_path.mkdir()
     scm = GitSCM(str(clone_path))
     scm.clone(str(git_repo))
 
     git_setup_user(str(clone_path))
 
-    new_file = clone_path / "new_file"
-    new_file.write_text("test", encoding="utf-8")
-
     original_commit = subprocess.run(
         ["git", "rev-parse", "HEAD"], cwd=str(clone_path), capture_output=True
     ).stdout
@@ -101,19 +105,7 @@ def test_GitSCM_clean_repo(
         check=True,
     )
     # Those two command should not raise exceptions
-    subprocess.run(["git", "add", new_file.name], cwd=str(clone_path), check=True)
-    subprocess.run(
-        [
-            "git",
-            "commit",
-            "-m",
-            "adding new_file",
-            "--author",
-            "Lando <[email protected]>",
-        ],
-        cwd=str(clone_path),
-        check=True,
-    )
+    new_file = _create_git_commit(request, clone_path)
 
     new_untracked_file = clone_path / "new_untracked_file"
     new_untracked_file.write_text("test", encoding="utf-8")
@@ -139,6 +131,101 @@ def test_GitSCM_clean_repo(
     ), f"strip_non_public_commits not honoured for {new_file}"
 
 
+@pytest.mark.parametrize("target_cs", [None, "main", "dev", "git-ref"])
+def test_GitSCM_update_repo(
+    git_repo: Path,
+    git_setup_user: Callable,
+    request: pytest.FixtureRequest,
+    target_cs: str,
+    tmp_path: Path,
+):
+    clone_path = tmp_path / request.node.name
+    clone_path.mkdir()
+    scm = GitSCM(str(clone_path))
+    scm.clone(str(git_repo))
+
+    git_setup_user(str(clone_path))
+
+    original_commit = subprocess.run(
+        ["git", "rev-parse", "HEAD"],
+        cwd=str(clone_path),
+        capture_output=True,
+    ).stdout
+
+    if target_cs == "git-ref":
+        # Special case for a naked git reference
+        target_cs = original_commit.decode("utf-8").strip()
+    elif target_cs:
+        original_commit = subprocess.run(
+            ["git", "rev-parse", f"origin/{target_cs}"],
+            cwd=str(clone_path),
+            capture_output=True,
+        ).stdout
+
+    # Create an empty commit that we expect to see rewound, too
+    subprocess.run(
+        [
+            "git",
+            "commit",
+            "--fixup",
+            "reword:HEAD",
+            "--no-edit",
+        ],
+        cwd=str(clone_path),
+        check=True,
+    )
+    _create_git_commit(request, clone_path)
+
+    scm.update_repo(str(git_repo), target_cs)
+
+    current_commit = subprocess.run(
+        ["git", "rev-parse", "--branch", "HEAD"],
+        cwd=str(clone_path),
+        capture_output=True,
+    ).stdout
+    current_commit = subprocess.run(
+        ["git", "rev-parse", "HEAD"], cwd=str(clone_path), capture_output=True
+    ).stdout
+    assert (
+        current_commit == original_commit
+    ), f"Not on original_commit {original_commit} updating repo: {current_commit}"
+
+
+@pytest.mark.parametrize("push_target", [None, "main", "dev"])
+def test_GitSCM_push(
+    git_repo: Path,
+    git_setup_user: Callable,
+    monkeypatch: pytest.MonkeyPatch,
+    push_target: Optional[str],
+    request: pytest.FixtureRequest,
+    tmp_path: Path,
+):
+    clone_path = tmp_path / request.node.name
+    clone_path.mkdir()
+
+    default_branch = "dev"
+    scm = GitSCM(str(clone_path), default_branch=default_branch)
+    scm.clone(str(git_repo))
+
+    git_setup_user(str(clone_path))
+
+    _create_git_commit(request, clone_path)
+
+    new_untracked_file = clone_path / "new_untracked_file"
+    new_untracked_file.write_text("test", encoding="utf-8")
+
+    mock_git_run = _monkeypatch_scm(monkeypatch, scm, "_git_run")
+
+    # breakpoint()
+    scm.push(str(git_repo), push_target)
+
+    if not push_target:
+        push_target = default_branch
+    mock_git_run.assert_called_with(
+        "push", str(git_repo), f"HEAD:{push_target}", cwd=str(clone_path)
+    )
+
+
 def test_GitSCM_push_get_github_token(git_repo: Path):
     scm = GitSCM(str(git_repo))
     scm._git_run = MagicMock()
@@ -168,7 +255,28 @@ def test_GitSCM_git_run_redact_url_userinfo(git_repo: Path):
     assert userinfo not in exc.value.err
     assert userinfo not in str(exc.value)
     assert userinfo not in repr(exc.value)
-    assert "[REDACTED]" in exc.value.err
+    assert "[REDACTED]" in str(exc.value)
+
+
+def _create_git_commit(request: pytest.FixtureRequest, clone_path: Path):
+    new_file = clone_path / "new_file"
+    new_file.write_text(request.node.name, encoding="utf-8")
+
+    subprocess.run(["git", "add", new_file.name], cwd=str(clone_path), check=True)
+    subprocess.run(
+        [
+            "git",
+            "commit",
+            "-m",
+            "adding new_file",
+            "--author",
+            f"{request.node.name} <pytest@lando>",
+        ],
+        cwd=str(clone_path),
+        check=True,
+    )
+
+    return new_file
 
 
 def _monkeypatch_scm(monkeypatch, scm: GitSCM, method: str) -> MagicMock: