Fix bugs with multiple configured remotes; debug-log return code

Pull Request: #18 (main)

git-grok

@@ -95,11 +95,10 @@ class InRebaseInteractiveData:
 
     @staticmethod
     def parse(str: str) -> InRebaseInteractiveData | None:
-        match = re.match(r"^(\d+)/(\d+)$", str)
-        if match:
+        if m := re.match(r"^(\d+)/(\d+)$", str):
             return InRebaseInteractiveData(
-                commit_index_one_based=int(match.group(1)),
-                total_commits_in_stack=int(match.group(2)),
+                commit_index_one_based=int(m.group(1)),
+                total_commits_in_stack=int(m.group(2)),
             )
         return None
 
@@ -161,6 +160,7 @@ class Main:
             self.self_update()
 
         # For debug purposes only.
+        self.shell_no_throw(["git", "--version"])
         self.shell_no_throw(["git", "status"])
         self.shell_no_throw(["git", "log", "--graph", "--decorate", "-3"])
         # Below is the real work.
@@ -500,11 +500,10 @@ class Main:
             assert (
                 c.url is not None
             ), f"commit {c.hash} PR URL is expected to be in the message at this point"
-            match = re.search(r"/(\d+)$", c.url)
-            if match:
-                pr_numbers.append(int(match.group(1)))
+            if m := re.search(r"/(\d+)$", c.url):
+                pr_numbers.append(int(m.group(1)))
                 if c.hash == commit.hash:
-                    pr_number_current = int(match.group(1))
+                    pr_number_current = int(m.group(1))
         assert (
             commit.url is not None
         ), f"commit {commit.hash} PR URL is expected to be in the message at this point"
@@ -525,7 +524,27 @@ class Main:
     # Returns current git folder remote (most often "origin").
     #
     def git_get_current_remote(self) -> str:
-        return self.shell(["git", "remote", "show"])
+        self.shell_no_throw(["bash", "-c", "set"])
+        [_, symbolic_ref, _] = self.shell_no_throw(
+            ["git", "symbolic-ref", "-q", "HEAD"]
+        )
+        if not symbolic_ref:
+            raise UserException(
+                'To run git-grok, you must be on a branch. Check your "git status".'
+            )
+        push_short = self.shell(
+            ["git", "for-each-ref", "--format=%(push:short)", symbolic_ref]
+        )
+        if not (m := re.match(r"^([^/]+)/.+$", push_short)):
+            raise UserException(
+                f'fatal: No configured push destination for symbolic ref "{symbolic_ref}".\n'
+                + "\n"
+                + "Git infers the current remote from the branch name using one of the following ways:\n"
+                + '1. It checks whether "git config branch.<branch>.remote" is set explicitly.\n'
+                + '2. Or, if "git config push.default" is "current" or "matching", there must be\n'
+                + "   only one remote configured.\n"
+            )
+        return m.group(1)
 
     #
     # Returns the current remote branch name on GitHub.
@@ -534,11 +553,9 @@ class Main:
         branch = self.shell(["git", "branch", "--show-current"])
         if not branch:
             path = ".git/rebase-merge/head-name"
-            file = find_file_in_parents(path)
-            if file:
+            if file := find_file_in_parents(path):
                 branch = open(file).readline().strip()
-                m = re.match(r"refs/heads/(.+)", branch)
-                if not m:
+                if not (m := re.match(r"refs/heads/(.+)", branch)):
                     raise UserException(f"File {path} doesn't contain refs/heads/*")
                 branch = m.group(1)
             else:
@@ -626,12 +643,12 @@ class Main:
             if returncode == 0:
                 return output, "created"
 
-            m = re.match(r".* already exists[^\n]\n(\S+)", stderr, flags=re.S)
-            if m:
+            if m := re.match(r".* already exists[^\n]\n(\S+)", stderr, flags=re.S):
                 return m.group(1), "up-to-date"
 
-            m = re.match(r".* was submitted too quickly", stderr, flags=re.S)
-            if m and attempt < 3:
+            if attempt < 3 and (
+                m := re.match(r".* was submitted too quickly", stderr, flags=re.S)
+            ):
                 dt = 3 + random.random() * 3
                 self.debug_log_text(text=f"Waiting for {dt} seconds and retrying...")
                 sleep(dt)
@@ -781,13 +798,17 @@ class Main:
         commits: list[Commit] = []
         for commit in re.split(r"^(?=commit )", out, flags=re.M)[1:]:
             try:
-                m = re.match(r"commit (\w+)[^\n]*\n(.*?)\n\n(.*)$", commit, flags=re.S)
-                if not m:
+
+                if not (
+                    m := re.match(
+                        r"commit (\w+)[^\n]*\n(.*?)\n\n(.*)$", commit, flags=re.S
+                    )
+                ):
                     raise UserException("Can't parse commit-headers-body.")
+
                 hash, _headers, body = m.group(1), m.group(2), m.group(3).rstrip()
 
-                m = re.match(r"([^\n]+)\n?(.*)$", body, flags=re.S)
-                if not m:
+                if not (m := re.match(r"([^\n]+)\n?(.*)$", body, flags=re.S)):
                     raise UserException(f"Can't extract commit title and description.")
                 title, description = m.group(1).strip(), m.group(2)
 
@@ -886,8 +907,7 @@ class Main:
             ["git", "show", "--pretty=format:%B", "--no-patch"],
             no_rstrip=True,
         )
-        m = re.search(rf"^{header_name}: [^\n]+", message, flags=re.M)
-        if m:
+        if m := re.search(rf"^{header_name}: [^\n]+", message, flags=re.M):
             new_message = message.replace(m[0], new_header)
         else:
             new_message = f"{message.rstrip()}\n\n{new_header}"
@@ -1253,7 +1273,7 @@ class Main:
     # it's requested.
     #
     def cache_through(self, key: str, func: Callable[[], str]) -> str:
-        var = f"{INTERNAL_ENV_VAR_PREFIX}:{key}"
+        var = f"{INTERNAL_ENV_VAR_PREFIX}_{key}"
         value = os.environ.get(var)
         if value is None:
             value = func()

tests/helpers.py

@@ -9,6 +9,7 @@
 from platform import python_version
 from shutil import rmtree
 from subprocess import Popen, check_output, CalledProcessError, STDOUT, PIPE
+from typing import Literal
 from unittest import TestCase
 
 GIT_GROK_PATH = dirname(dirname(realpath(__file__))) + "/git-grok"
@@ -42,14 +43,18 @@ def check_output_x(*cmd: str) -> str:
         ) from None
 
 
-def git_init_and_cd_to_test_dir(*, dir: str, initial_branch: str):
+def git_init_and_cd_to_test_dir(
+    *,
+    dir: str,
+    initial_branch: str,
+    method: Literal["push.default", "set-upstream"],
+):
     rmtree(dir, ignore_errors=True)
     mkdir(dir)
     chdir(dir)
     check_output_x("git", "init", f"--initial-branch={initial_branch}")
     check_output_x("git", "config", "user.name", "tests")
     check_output_x("git", "config", "user.email", "[email protected]")
-    check_output_x("git", "config", "push.default", "current")
     check_output_x(
         "git",
         "remote",
@@ -73,7 +78,14 @@ def git_init_and_cd_to_test_dir(*, dir: str, initial_branch: str):
         environ["GH_TOKEN"] = token
 
     check_output_x("git", "commit", "--allow-empty", "-m", "Initial commit")
-    git_push()
+
+    if method == "push.default":
+        check_output_x("git", "config", "push.default", "current")
+        check_output_x("git", "push", "-f", TEST_REMOTE)
+    elif method == "set-upstream":
+        check_output_x(
+            "git", "push", "-f", "--set-upstream", TEST_REMOTE, initial_branch
+        )
 
     check_output_x("git", "fetch")
     branches = [
@@ -111,7 +123,7 @@ def git_add_commit(msg: str):
 
 
 def git_push():
-    check_output_x("git", "push", "-f", TEST_REMOTE)
+    check_output_x("git", "push", "-f", "--set-upstream", TEST_REMOTE)
 
 
 def git_get_prs(branch: str) -> str:
@@ -171,12 +183,20 @@ def setUp(self):
         )
         self.cwd = getcwd()
         self.initial_branch = f"{prefix}tests{python_version()}"
-        git_init_and_cd_to_test_dir(
-            dir="/tmp/git-grok-tests", initial_branch=self.initial_branch
-        )
 
     def tearDown(self):
         chdir(self.cwd)
 
+    def git_init_and_cd_to_test_dir(
+        self,
+        *,
+        method: Literal["push.default", "set-upstream"] = "set-upstream",
+    ):
+        git_init_and_cd_to_test_dir(
+            dir="/tmp/git-grok-tests",
+            initial_branch=self.initial_branch,
+            method=method,
+        )
+
 
 git_grok = import_path(GIT_GROK_PATH)

tests/test_get_current_remote.py

@@ -0,0 +1,26 @@
+__import__("sys").dont_write_bytecode = True
+from helpers import (
+    TestCaseWithEmptyTestRepo,
+    git_add_commit,
+    git_touch,
+    run_git_grok,
+)
+from unittest import main
+
+
+class Test(TestCaseWithEmptyTestRepo):
+    def test_set_upstream(self):
+        self.git_init_and_cd_to_test_dir(method="set-upstream")
+        git_touch("commit01")
+        git_add_commit("commit01")
+        run_git_grok(skip_update_prs=True)
+
+    def test_push_default(self):
+        self.git_init_and_cd_to_test_dir(method="push.default")
+        git_touch("commit01")
+        git_add_commit("commit01")
+        run_git_grok(skip_update_prs=True)
+
+
+if __name__ == "__main__":
+    main()

tests/test_middle_pr.py

@@ -12,12 +12,12 @@
 
 class Test(TestCaseWithEmptyTestRepo):
     def test_middle_pr_label_at_initial_prs_creation_and_updates(self):
+        self.git_init_and_cd_to_test_dir()
+
         git_touch("commit01")
         git_add_commit("commit01")
-
         git_touch("commit02")
         git_add_commit("commit02")
-
         git_touch("commit03")
         git_add_commit("commit03")