mirror of
https://github.com/github/spec-kit.git
synced 2026-07-07 22:56:57 +08:00
feat(scripts): add --dry-run flag to create-new-feature (#1998)
* feat(scripts): add --dry-run flag to create-new-feature scripts Add a --dry-run / -DryRun flag to both bash and PowerShell create-new-feature scripts that computes the next branch name, spec file path, and feature number without creating any branches, directories, or files. This enables external tools to query the next available name before running the full specify workflow. When combined with --json, the output includes a DRY_RUN field. Without --dry-run, behavior is completely unchanged. Closes #1931 Assisted-By: 🤖 Claude Code * fix(scripts): gate specs/ dir creation behind dry-run check Dry-run was unconditionally creating the root specs/ directory via mkdir -p / New-Item before the dry-run guard. This violated the documented contract of zero side effects. Also adds returncode assertion on git branch --list in tests and adds PowerShell dry-run test coverage (skipped when pwsh unavailable). Addresses review comments on #1998. Assisted-By: 🤖 Claude Code * fix: address PR review feedback - Gate `mkdir -p $SPECS_DIR` behind DRY_RUN check (bash + PowerShell) so dry-run creates zero directories - Add returncode assertion on `git branch --list` in test - Strengthen spec dir test to verify root `specs/` is not created - Add PowerShell dry-run test class (5 tests, skipped without pwsh) - Fix run_ps_script to use temp repo copy instead of project root Assisted-By: 🤖 Claude Code * fix: use git ls-remote for remote-aware dry-run numbering Dry-run now queries remote branches via `git ls-remote --heads` (read-only, no fetch) to account for remote-only branches when computing the next sequential number. This prevents dry-run from returning a number that already exists on a remote. Added test verifying dry-run sees remote-only higher-numbered branches and adjusts numbering accordingly. Assisted-By: 🤖 Claude Code * fix(scripts): deduplicate number extraction and branch scanning logic Extract shared _extract_highest_number helper (bash) and Get-HighestNumberFromNames (PowerShell) to eliminate duplicated number extraction patterns between local branch and remote ref scanning. Add SkipFetch/skip_fetch parameter to check_existing_branches / Get-NextBranchNumber so dry-run reuses the same function instead of inlining duplicate max-of-branches-and-specs logic. Assisted-By: 🤖 Claude Code * fix(tests): use isolated paths for remote branch test Move remote.git and second_clone directories under git_repo instead of git_repo.parent to prevent path collisions with parallel test workers. Assisted-By: 🤖 Claude Code * fix: address PR review feedback - Set GIT_TERMINAL_PROMPT=0 for git ls-remote calls to prevent credential prompts from blocking dry-run in automation scenarios - Add returncode assertion to test_dry_run_with_timestamp git branch --list check Assisted-By: 🤖 Claude Code
This commit is contained in:
Roland Huß
@@ -436,3 +436,341 @@ class TestAllowExistingBranchPowerShell:
|
||||
assert "-AllowExistingBranch" in contents
|
||||
# Ensure the flag is referenced in script logic, not just declared
|
||||
assert "AllowExistingBranch" in contents.replace("-AllowExistingBranch", "")
|
||||
|
||||
|
||||
# ── Dry-Run Tests ────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestDryRun:
|
||||
def test_dry_run_sequential_outputs_name(self, git_repo: Path):
|
||||
"""T009: Dry-run computes correct branch name with existing specs."""
|
||||
(git_repo / "specs" / "001-first-feat").mkdir(parents=True)
|
||||
(git_repo / "specs" / "002-second-feat").mkdir(parents=True)
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "new-feat", "New feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch == "003-new-feat", f"expected 003-new-feat, got: {branch}"
|
||||
|
||||
def test_dry_run_no_branch_created(self, git_repo: Path):
|
||||
"""T010: Dry-run does not create a git branch."""
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "no-branch", "No branch feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branches = subprocess.run(
|
||||
["git", "branch", "--list", "*no-branch*"],
|
||||
cwd=git_repo,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
|
||||
assert branches.stdout.strip() == "", "branch should not exist after dry-run"
|
||||
|
||||
def test_dry_run_no_spec_dir_created(self, git_repo: Path):
|
||||
"""T011: Dry-run does not create any directories (including root specs/)."""
|
||||
specs_root = git_repo / "specs"
|
||||
if specs_root.exists():
|
||||
shutil.rmtree(specs_root)
|
||||
assert not specs_root.exists(), "specs/ should not exist before dry-run"
|
||||
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "no-dir", "No dir feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert not specs_root.exists(), "specs/ should not be created during dry-run"
|
||||
|
||||
def test_dry_run_empty_repo(self, git_repo: Path):
|
||||
"""T012: Dry-run returns 001 prefix when no existing specs or branches."""
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "first", "First feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch == "001-first", f"expected 001-first, got: {branch}"
|
||||
|
||||
def test_dry_run_with_short_name(self, git_repo: Path):
|
||||
"""T013: Dry-run with --short-name produces expected name."""
|
||||
(git_repo / "specs" / "001-existing").mkdir(parents=True)
|
||||
(git_repo / "specs" / "002-existing").mkdir(parents=True)
|
||||
(git_repo / "specs" / "003-existing").mkdir(parents=True)
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "user-auth", "Add user authentication"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch == "004-user-auth", f"expected 004-user-auth, got: {branch}"
|
||||
|
||||
def test_dry_run_then_real_run_match(self, git_repo: Path):
|
||||
"""T014: Dry-run name matches subsequent real creation."""
|
||||
(git_repo / "specs" / "001-existing").mkdir(parents=True)
|
||||
# Dry-run first
|
||||
dry_result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "match-test", "Match test"
|
||||
)
|
||||
assert dry_result.returncode == 0, dry_result.stderr
|
||||
dry_branch = None
|
||||
for line in dry_result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
dry_branch = line.split(":", 1)[1].strip()
|
||||
# Real run
|
||||
real_result = run_script(
|
||||
git_repo, "--short-name", "match-test", "Match test"
|
||||
)
|
||||
assert real_result.returncode == 0, real_result.stderr
|
||||
real_branch = None
|
||||
for line in real_result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
real_branch = line.split(":", 1)[1].strip()
|
||||
assert dry_branch == real_branch, f"dry={dry_branch} != real={real_branch}"
|
||||
|
||||
def test_dry_run_accounts_for_remote_branches(self, git_repo: Path):
|
||||
"""Dry-run queries remote refs via ls-remote (no fetch) for accurate numbering."""
|
||||
(git_repo / "specs" / "001-existing").mkdir(parents=True)
|
||||
|
||||
# Set up a bare remote and push (use subdirs of git_repo for isolation)
|
||||
remote_dir = git_repo / "test-remote.git"
|
||||
subprocess.run(
|
||||
["git", "init", "--bare", str(remote_dir)],
|
||||
check=True, capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "remote", "add", "origin", str(remote_dir)],
|
||||
check=True, cwd=git_repo, capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "push", "-u", "origin", "HEAD"],
|
||||
check=True, cwd=git_repo, capture_output=True,
|
||||
)
|
||||
|
||||
# Clone into a second copy, create a higher-numbered branch, push it
|
||||
second_clone = git_repo / "test-second-clone"
|
||||
subprocess.run(
|
||||
["git", "clone", str(remote_dir), str(second_clone)],
|
||||
check=True, capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "user.email", "test@example.com"],
|
||||
cwd=second_clone, check=True, capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "user.name", "Test User"],
|
||||
cwd=second_clone, check=True, capture_output=True,
|
||||
)
|
||||
# Create branch 005 on the remote (higher than local 001)
|
||||
subprocess.run(
|
||||
["git", "checkout", "-b", "005-remote-only"],
|
||||
cwd=second_clone, check=True, capture_output=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "push", "origin", "005-remote-only"],
|
||||
cwd=second_clone, check=True, capture_output=True,
|
||||
)
|
||||
|
||||
# Primary repo: dry-run should see 005 via ls-remote and return 006
|
||||
dry_result = run_script(
|
||||
git_repo, "--dry-run", "--short-name", "remote-test", "Remote test"
|
||||
)
|
||||
assert dry_result.returncode == 0, dry_result.stderr
|
||||
dry_branch = None
|
||||
for line in dry_result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
dry_branch = line.split(":", 1)[1].strip()
|
||||
assert dry_branch == "006-remote-test", f"expected 006-remote-test, got: {dry_branch}"
|
||||
|
||||
def test_dry_run_json_includes_field(self, git_repo: Path):
|
||||
"""T015: JSON output includes DRY_RUN field when --dry-run is active."""
|
||||
import json
|
||||
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--json", "--short-name", "json-test", "JSON test"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
data = json.loads(result.stdout)
|
||||
assert "DRY_RUN" in data, f"DRY_RUN missing from JSON: {data}"
|
||||
assert data["DRY_RUN"] is True
|
||||
|
||||
def test_dry_run_json_absent_without_flag(self, git_repo: Path):
|
||||
"""T016: Normal JSON output does NOT include DRY_RUN field."""
|
||||
import json
|
||||
|
||||
result = run_script(
|
||||
git_repo, "--json", "--short-name", "no-dry", "No dry run"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
data = json.loads(result.stdout)
|
||||
assert "DRY_RUN" not in data, f"DRY_RUN should not be in normal JSON: {data}"
|
||||
|
||||
def test_dry_run_with_timestamp(self, git_repo: Path):
|
||||
"""T017: Dry-run works with --timestamp flag."""
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--timestamp", "--short-name", "ts-feat", "Timestamp feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch is not None, "no BRANCH_NAME in output"
|
||||
assert re.match(r"^\d{8}-\d{6}-ts-feat$", branch), f"unexpected: {branch}"
|
||||
# Verify no side effects
|
||||
branches = subprocess.run(
|
||||
["git", "branch", "--list", f"*ts-feat*"],
|
||||
cwd=git_repo,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
|
||||
assert branches.stdout.strip() == ""
|
||||
|
||||
def test_dry_run_with_number(self, git_repo: Path):
|
||||
"""T018: Dry-run works with --number flag."""
|
||||
result = run_script(
|
||||
git_repo, "--dry-run", "--number", "42", "--short-name", "num-feat", "Number feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch == "042-num-feat", f"expected 042-num-feat, got: {branch}"
|
||||
|
||||
def test_dry_run_no_git(self, no_git_dir: Path):
|
||||
"""T019: Dry-run works in non-git directory."""
|
||||
(no_git_dir / "specs" / "001-existing").mkdir(parents=True)
|
||||
result = run_script(
|
||||
no_git_dir, "--dry-run", "--short-name", "no-git-dry", "No git dry run"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch == "002-no-git-dry", f"expected 002-no-git-dry, got: {branch}"
|
||||
# Verify no spec dir created
|
||||
spec_dirs = [
|
||||
d.name
|
||||
for d in (no_git_dir / "specs").iterdir()
|
||||
if d.is_dir() and "no-git-dry" in d.name
|
||||
]
|
||||
assert len(spec_dirs) == 0
|
||||
|
||||
|
||||
# ── PowerShell Dry-Run Tests ─────────────────────────────────────────────────
|
||||
|
||||
|
||||
def _has_pwsh() -> bool:
|
||||
"""Check if pwsh is available."""
|
||||
try:
|
||||
subprocess.run(["pwsh", "--version"], capture_output=True, check=True)
|
||||
return True
|
||||
except (FileNotFoundError, subprocess.CalledProcessError):
|
||||
return False
|
||||
|
||||
|
||||
def run_ps_script(cwd: Path, *args: str) -> subprocess.CompletedProcess:
|
||||
"""Run create-new-feature.ps1 from the temp repo's scripts directory."""
|
||||
script = cwd / "scripts" / "powershell" / "create-new-feature.ps1"
|
||||
cmd = ["pwsh", "-NoProfile", "-File", str(script), *args]
|
||||
return subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def ps_git_repo(tmp_path: Path) -> Path:
|
||||
"""Create a temp git repo with PowerShell scripts and .specify dir."""
|
||||
subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True)
|
||||
subprocess.run(
|
||||
["git", "config", "user.email", "test@example.com"], cwd=tmp_path, check=True
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "user.name", "Test User"], cwd=tmp_path, check=True
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "commit", "--allow-empty", "-m", "init", "-q"],
|
||||
cwd=tmp_path,
|
||||
check=True,
|
||||
)
|
||||
ps_dir = tmp_path / "scripts" / "powershell"
|
||||
ps_dir.mkdir(parents=True)
|
||||
shutil.copy(CREATE_FEATURE_PS, ps_dir / "create-new-feature.ps1")
|
||||
common_ps = PROJECT_ROOT / "scripts" / "powershell" / "common.ps1"
|
||||
shutil.copy(common_ps, ps_dir / "common.ps1")
|
||||
(tmp_path / ".specify" / "templates").mkdir(parents=True)
|
||||
return tmp_path
|
||||
|
||||
|
||||
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not available")
|
||||
class TestPowerShellDryRun:
|
||||
def test_ps_dry_run_outputs_name(self, ps_git_repo: Path):
|
||||
"""PowerShell -DryRun computes correct branch name."""
|
||||
(ps_git_repo / "specs" / "001-first").mkdir(parents=True)
|
||||
result = run_ps_script(
|
||||
ps_git_repo, "-DryRun", "-ShortName", "ps-feat", "PS feature"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branch = None
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("BRANCH_NAME:"):
|
||||
branch = line.split(":", 1)[1].strip()
|
||||
assert branch == "002-ps-feat", f"expected 002-ps-feat, got: {branch}"
|
||||
|
||||
def test_ps_dry_run_no_branch_created(self, ps_git_repo: Path):
|
||||
"""PowerShell -DryRun does not create a git branch."""
|
||||
result = run_ps_script(
|
||||
ps_git_repo, "-DryRun", "-ShortName", "no-ps-branch", "No branch"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
branches = subprocess.run(
|
||||
["git", "branch", "--list", "*no-ps-branch*"],
|
||||
cwd=ps_git_repo,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
|
||||
assert branches.stdout.strip() == "", "branch should not exist after dry-run"
|
||||
|
||||
def test_ps_dry_run_no_spec_dir_created(self, ps_git_repo: Path):
|
||||
"""PowerShell -DryRun does not create specs/ directory."""
|
||||
specs_root = ps_git_repo / "specs"
|
||||
if specs_root.exists():
|
||||
shutil.rmtree(specs_root)
|
||||
assert not specs_root.exists()
|
||||
|
||||
result = run_ps_script(
|
||||
ps_git_repo, "-DryRun", "-ShortName", "no-ps-dir", "No dir"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert not specs_root.exists(), "specs/ should not be created during dry-run"
|
||||
|
||||
def test_ps_dry_run_json_includes_field(self, ps_git_repo: Path):
|
||||
"""PowerShell -DryRun JSON output includes DRY_RUN field."""
|
||||
import json
|
||||
|
||||
result = run_ps_script(
|
||||
ps_git_repo, "-DryRun", "-Json", "-ShortName", "ps-json", "JSON test"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
data = json.loads(result.stdout)
|
||||
assert "DRY_RUN" in data, f"DRY_RUN missing from JSON: {data}"
|
||||
assert data["DRY_RUN"] is True
|
||||
|
||||
def test_ps_dry_run_json_absent_without_flag(self, ps_git_repo: Path):
|
||||
"""PowerShell normal JSON output does NOT include DRY_RUN field."""
|
||||
import json
|
||||
|
||||
result = run_ps_script(
|
||||
ps_git_repo, "-Json", "-ShortName", "ps-no-dry", "No dry run"
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
data = json.loads(result.stdout)
|
||||
assert "DRY_RUN" not in data, f"DRY_RUN should not be in normal JSON: {data}"
|
||||
|
||||
Reference in New Issue
Block a user