Improve branch helpers with main/master base resolution
- Update pkgmgr.actions.branch.open_branch() to resolve the base branch via _resolve_base_branch(), preferring 'main' and falling back to 'master' when the preferred branch does not exist. - Adjust the open_branch logic to: - fetch from origin - checkout the resolved base branch - pull the resolved base branch - create the feature branch - push the new branch with upstream tracking - Add and refine unit tests in tests/unit/pkgmgr/actions/test_branch.py to cover: - normal branch creation with explicit name and default base - interactive name prompting when no name is provided - error handling when fetch fails after successful base resolution - fallback to 'master' when 'main' is missing. - Clean up and clarify docstrings and comments for open_branch(), close_branch(), and _resolve_base_branch(), and fix the module header comment to match the new package path. This fixes branch opening in repositories that still use 'master' as their primary branch while keeping the default behavior for 'main'. https://chatgpt.com/share/6938838f-7aac-800f-b130-924e07ef48b9
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
# pkgmgr/branch_commands.py
|
||||
# pkgmgr/actions/branch/__init__.py
|
||||
#!/usr/bin/env python3
|
||||
# -*- coding: utf-8 -*-
|
||||
|
||||
@@ -16,30 +16,43 @@ from typing import Optional
|
||||
from pkgmgr.core.git import run_git, GitError, get_current_branch
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Branch creation (open)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def open_branch(
|
||||
name: Optional[str],
|
||||
base_branch: str = "main",
|
||||
fallback_base: str = "master",
|
||||
cwd: str = ".",
|
||||
) -> None:
|
||||
"""
|
||||
Create and push a new feature branch on top of `base_branch`.
|
||||
Create and push a new feature branch on top of a base branch.
|
||||
|
||||
The base branch is resolved by:
|
||||
1. Trying 'base_branch' (default: 'main')
|
||||
2. Falling back to 'fallback_base' (default: 'master')
|
||||
|
||||
Steps:
|
||||
1) git fetch origin
|
||||
2) git checkout <base_branch>
|
||||
3) git pull origin <base_branch>
|
||||
2) git checkout <resolved_base>
|
||||
3) git pull origin <resolved_base>
|
||||
4) git checkout -b <name>
|
||||
5) git push -u origin <name>
|
||||
|
||||
If `name` is None or empty, the user is prompted on stdin.
|
||||
If `name` is None or empty, the user is prompted to enter one.
|
||||
"""
|
||||
|
||||
# Request name interactively if not provided
|
||||
if not name:
|
||||
name = input("Enter new branch name: ").strip()
|
||||
|
||||
if not name:
|
||||
raise RuntimeError("Branch name must not be empty.")
|
||||
|
||||
# Resolve which base branch to use (main or master)
|
||||
resolved_base = _resolve_base_branch(base_branch, fallback_base, cwd=cwd)
|
||||
|
||||
# 1) Fetch from origin
|
||||
try:
|
||||
run_git(["fetch", "origin"], cwd=cwd)
|
||||
@@ -50,18 +63,18 @@ def open_branch(
|
||||
|
||||
# 2) Checkout base branch
|
||||
try:
|
||||
run_git(["checkout", base_branch], cwd=cwd)
|
||||
run_git(["checkout", resolved_base], cwd=cwd)
|
||||
except GitError as exc:
|
||||
raise RuntimeError(
|
||||
f"Failed to checkout base branch {base_branch!r}: {exc}"
|
||||
f"Failed to checkout base branch {resolved_base!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 3) Pull latest changes on base
|
||||
# 3) Pull latest changes for base branch
|
||||
try:
|
||||
run_git(["pull", "origin", base_branch], cwd=cwd)
|
||||
run_git(["pull", "origin", resolved_base], cwd=cwd)
|
||||
except GitError as exc:
|
||||
raise RuntimeError(
|
||||
f"Failed to pull latest changes for base branch {base_branch!r}: {exc}"
|
||||
f"Failed to pull latest changes for base branch {resolved_base!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 4) Create new branch
|
||||
@@ -69,10 +82,10 @@ def open_branch(
|
||||
run_git(["checkout", "-b", name], cwd=cwd)
|
||||
except GitError as exc:
|
||||
raise RuntimeError(
|
||||
f"Failed to create new branch {name!r} from base {base_branch!r}: {exc}"
|
||||
f"Failed to create new branch {name!r} from base {resolved_base!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 5) Push and set upstream
|
||||
# 5) Push new branch to origin
|
||||
try:
|
||||
run_git(["push", "-u", "origin", name], cwd=cwd)
|
||||
except GitError as exc:
|
||||
@@ -81,15 +94,21 @@ def open_branch(
|
||||
) from exc
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Base branch resolver (shared by open/close)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _resolve_base_branch(
|
||||
preferred: str,
|
||||
fallback: str,
|
||||
cwd: str,
|
||||
) -> str:
|
||||
"""
|
||||
Resolve the base branch to use for merging.
|
||||
Resolve the base branch to use.
|
||||
|
||||
Try `preferred` first (default: main),
|
||||
fall back to `fallback` (default: master).
|
||||
|
||||
Try `preferred` (default: main) first, then `fallback` (default: master).
|
||||
Raise RuntimeError if neither exists.
|
||||
"""
|
||||
for candidate in (preferred, fallback):
|
||||
@@ -104,6 +123,10 @@ def _resolve_base_branch(
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Branch closing (merge + deletion)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def close_branch(
|
||||
name: Optional[str],
|
||||
base_branch: str = "main",
|
||||
@@ -111,23 +134,22 @@ def close_branch(
|
||||
cwd: str = ".",
|
||||
) -> None:
|
||||
"""
|
||||
Merge a feature branch into the main/master branch and optionally delete it.
|
||||
Merge a feature branch into the base branch and delete it afterwards.
|
||||
|
||||
Steps:
|
||||
1) Determine branch name (argument or current branch)
|
||||
2) Resolve base branch (prefers `base_branch`, falls back to `fallback_base`)
|
||||
3) Ask for confirmation (y/N)
|
||||
1) Determine the branch name (argument or current branch)
|
||||
2) Resolve base branch (main/master)
|
||||
3) Ask for confirmation
|
||||
4) git fetch origin
|
||||
5) git checkout <base>
|
||||
6) git pull origin <base>
|
||||
7) git merge --no-ff <name>
|
||||
8) git push origin <base>
|
||||
9) Delete branch locally and on origin
|
||||
|
||||
If the user does not confirm with 'y', the operation is aborted.
|
||||
9) Delete branch locally
|
||||
10) Delete branch on origin (best effort)
|
||||
"""
|
||||
|
||||
# 1) Determine which branch to close
|
||||
# 1) Determine which branch should be closed
|
||||
if not name:
|
||||
try:
|
||||
name = get_current_branch(cwd=cwd)
|
||||
@@ -137,7 +159,7 @@ def close_branch(
|
||||
if not name:
|
||||
raise RuntimeError("Branch name must not be empty.")
|
||||
|
||||
# 2) Resolve base branch (main/master)
|
||||
# 2) Resolve base branch
|
||||
target_base = _resolve_base_branch(base_branch, fallback_base, cwd=cwd)
|
||||
|
||||
if name == target_base:
|
||||
@@ -146,7 +168,7 @@ def close_branch(
|
||||
"Please specify a feature branch."
|
||||
)
|
||||
|
||||
# 3) Confirmation prompt
|
||||
# 3) Ask user for confirmation
|
||||
prompt = (
|
||||
f"Merge branch '{name}' into '{target_base}' and delete it afterwards? "
|
||||
"(y/N): "
|
||||
@@ -164,7 +186,7 @@ def close_branch(
|
||||
f"Failed to fetch from origin before closing branch {name!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 5) Checkout base branch
|
||||
# 5) Checkout base
|
||||
try:
|
||||
run_git(["checkout", target_base], cwd=cwd)
|
||||
except GitError as exc:
|
||||
@@ -172,7 +194,7 @@ def close_branch(
|
||||
f"Failed to checkout base branch {target_base!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 6) Pull latest base
|
||||
# 6) Pull latest base state
|
||||
try:
|
||||
run_git(["pull", "origin", target_base], cwd=cwd)
|
||||
except GitError as exc:
|
||||
@@ -180,7 +202,7 @@ def close_branch(
|
||||
f"Failed to pull latest changes for base branch {target_base!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 7) Merge feature branch into base
|
||||
# 7) Merge the feature branch
|
||||
try:
|
||||
run_git(["merge", "--no-ff", name], cwd=cwd)
|
||||
except GitError as exc:
|
||||
@@ -193,22 +215,21 @@ def close_branch(
|
||||
run_git(["push", "origin", target_base], cwd=cwd)
|
||||
except GitError as exc:
|
||||
raise RuntimeError(
|
||||
f"Failed to push base branch {target_base!r} to origin after merge: {exc}"
|
||||
f"Failed to push base branch {target_base!r} after merge: {exc}"
|
||||
) from exc
|
||||
|
||||
# 9) Delete feature branch locally
|
||||
# 9) Delete branch locally
|
||||
try:
|
||||
run_git(["branch", "-d", name], cwd=cwd)
|
||||
except GitError as exc:
|
||||
raise RuntimeError(
|
||||
f"Failed to delete local branch {name!r} after merge: {exc}"
|
||||
f"Failed to delete local branch {name!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
# 10) Delete feature branch on origin (best effort)
|
||||
# 10) Delete branch on origin (best effort)
|
||||
try:
|
||||
run_git(["push", "origin", "--delete", name], cwd=cwd)
|
||||
except GitError as exc:
|
||||
# Remote delete is nice-to-have; surface as RuntimeError for clarity.
|
||||
raise RuntimeError(
|
||||
f"Branch {name!r} was deleted locally, but remote deletion failed: {exc}"
|
||||
) from exc
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.actions.branch import open_branch
|
||||
@@ -13,9 +12,10 @@ class TestOpenBranch(unittest.TestCase):
|
||||
def test_open_branch_with_explicit_name_and_default_base(self, mock_run_git) -> None:
|
||||
"""
|
||||
open_branch(name, base='main') should:
|
||||
- resolve base branch (prefers 'main', falls back to 'master')
|
||||
- fetch origin
|
||||
- checkout base
|
||||
- pull base
|
||||
- checkout resolved base
|
||||
- pull resolved base
|
||||
- create new branch
|
||||
- push with upstream
|
||||
"""
|
||||
@@ -25,6 +25,7 @@ class TestOpenBranch(unittest.TestCase):
|
||||
|
||||
# We expect a specific sequence of Git calls.
|
||||
expected_calls = [
|
||||
(["rev-parse", "--verify", "main"], "/repo"),
|
||||
(["fetch", "origin"], "/repo"),
|
||||
(["checkout", "main"], "/repo"),
|
||||
(["pull", "origin", "main"], "/repo"),
|
||||
@@ -50,7 +51,7 @@ class TestOpenBranch(unittest.TestCase):
|
||||
) -> None:
|
||||
"""
|
||||
If name is None/empty, open_branch should prompt via input()
|
||||
and still perform the full Git sequence.
|
||||
and still perform the full Git sequence on the resolved base.
|
||||
"""
|
||||
mock_run_git.return_value = ""
|
||||
|
||||
@@ -60,6 +61,7 @@ class TestOpenBranch(unittest.TestCase):
|
||||
mock_input.assert_called_once()
|
||||
|
||||
expected_calls = [
|
||||
(["rev-parse", "--verify", "develop"], "/repo"),
|
||||
(["fetch", "origin"], "/repo"),
|
||||
(["checkout", "develop"], "/repo"),
|
||||
(["pull", "origin", "develop"], "/repo"),
|
||||
@@ -76,15 +78,20 @@ class TestOpenBranch(unittest.TestCase):
|
||||
self.assertEqual(kwargs.get("cwd"), cwd_expected)
|
||||
|
||||
@patch("pkgmgr.actions.branch.run_git")
|
||||
def test_open_branch_raises_runtimeerror_on_git_failure(self, mock_run_git) -> None:
|
||||
def test_open_branch_raises_runtimeerror_on_fetch_failure(self, mock_run_git) -> None:
|
||||
"""
|
||||
If a GitError occurs (e.g. fetch fails), open_branch should
|
||||
raise a RuntimeError with a helpful message.
|
||||
If a GitError occurs on fetch, open_branch should raise a RuntimeError
|
||||
with a helpful message.
|
||||
"""
|
||||
|
||||
def side_effect(args, cwd="."):
|
||||
# Simulate a failure on the first call (fetch)
|
||||
# First call: base resolution (rev-parse) should succeed
|
||||
if args == ["rev-parse", "--verify", "main"]:
|
||||
return ""
|
||||
# Second call: fetch should fail
|
||||
if args == ["fetch", "origin"]:
|
||||
raise GitError("simulated fetch failure")
|
||||
return ""
|
||||
|
||||
mock_run_git.side_effect = side_effect
|
||||
|
||||
@@ -95,6 +102,45 @@ class TestOpenBranch(unittest.TestCase):
|
||||
self.assertIn("Failed to fetch from origin", msg)
|
||||
self.assertIn("simulated fetch failure", msg)
|
||||
|
||||
@patch("pkgmgr.actions.branch.run_git")
|
||||
def test_open_branch_uses_fallback_master_if_main_missing(self, mock_run_git) -> None:
|
||||
"""
|
||||
If the preferred base (e.g. 'main') does not exist, open_branch should
|
||||
fall back to the fallback base (default: 'master').
|
||||
"""
|
||||
|
||||
def side_effect(args, cwd="."):
|
||||
# First: rev-parse main -> fails
|
||||
if args == ["rev-parse", "--verify", "main"]:
|
||||
raise GitError("main does not exist")
|
||||
# Second: rev-parse master -> succeeds
|
||||
if args == ["rev-parse", "--verify", "master"]:
|
||||
return ""
|
||||
# Then normal flow on master
|
||||
return ""
|
||||
|
||||
mock_run_git.side_effect = side_effect
|
||||
|
||||
open_branch(name="feature/fallback", base_branch="main", cwd="/repo")
|
||||
|
||||
expected_calls = [
|
||||
(["rev-parse", "--verify", "main"], "/repo"),
|
||||
(["rev-parse", "--verify", "master"], "/repo"),
|
||||
(["fetch", "origin"], "/repo"),
|
||||
(["checkout", "master"], "/repo"),
|
||||
(["pull", "origin", "master"], "/repo"),
|
||||
(["checkout", "-b", "feature/fallback"], "/repo"),
|
||||
(["push", "-u", "origin", "feature/fallback"], "/repo"),
|
||||
]
|
||||
|
||||
self.assertEqual(mock_run_git.call_count, len(expected_calls))
|
||||
for call, (args_expected, cwd_expected) in zip(
|
||||
mock_run_git.call_args_list, expected_calls
|
||||
):
|
||||
args, kwargs = call
|
||||
self.assertEqual(args[0], args_expected)
|
||||
self.assertEqual(kwargs.get("cwd"), cwd_expected)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user