Skip to content

Commit 4378a1e

Browse files
committed
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
1 parent 42243ef commit 4378a1e

File tree

3 files changed

+123
-9
lines changed

3 files changed

+123
-9
lines changed

scripts/bash/create-new-feature.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,9 @@ fi
184184
cd "$REPO_ROOT"
185185

186186
SPECS_DIR="$REPO_ROOT/specs"
187-
mkdir -p "$SPECS_DIR"
187+
if [ "$DRY_RUN" != true ]; then
188+
mkdir -p "$SPECS_DIR"
189+
fi
188190

189191
# Function to generate branch name with stop word filtering and length filtering
190192
generate_branch_name() {

scripts/powershell/create-new-feature.ps1

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,9 @@ $hasGit = Test-HasGit
136136
Set-Location $repoRoot
137137

138138
$specsDir = Join-Path $repoRoot 'specs'
139-
New-Item -ItemType Directory -Path $specsDir -Force | Out-Null
139+
if (-not $DryRun) {
140+
New-Item -ItemType Directory -Path $specsDir -Force | Out-Null
141+
}
140142

141143
# Function to generate branch name with stop word filtering and length filtering
142144
function Get-BranchName {

tests/test_timestamp_branches.py

Lines changed: 117 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -444,20 +444,21 @@ def test_dry_run_no_branch_created(self, git_repo: Path):
444444
capture_output=True,
445445
text=True,
446446
)
447+
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
447448
assert branches.stdout.strip() == "", "branch should not exist after dry-run"
448449

449450
def test_dry_run_no_spec_dir_created(self, git_repo: Path):
450-
"""T011: Dry-run does not create a spec directory."""
451+
"""T011: Dry-run does not create any directories (including root specs/)."""
452+
specs_root = git_repo / "specs"
453+
if specs_root.exists():
454+
shutil.rmtree(specs_root)
455+
assert not specs_root.exists(), "specs/ should not exist before dry-run"
456+
451457
result = run_script(
452458
git_repo, "--dry-run", "--short-name", "no-dir", "No dir feature"
453459
)
454460
assert result.returncode == 0, result.stderr
455-
spec_dirs = [
456-
d.name
457-
for d in (git_repo / "specs").iterdir()
458-
if d.is_dir() and "no-dir" in d.name
459-
] if (git_repo / "specs").exists() else []
460-
assert len(spec_dirs) == 0, f"spec dir should not exist: {spec_dirs}"
461+
assert not specs_root.exists(), "specs/ should not be created during dry-run"
461462

462463
def test_dry_run_empty_repo(self, git_repo: Path):
463464
"""T012: Dry-run returns 001 prefix when no existing specs or branches."""
@@ -584,3 +585,112 @@ def test_dry_run_no_git(self, no_git_dir: Path):
584585
if d.is_dir() and "no-git-dry" in d.name
585586
]
586587
assert len(spec_dirs) == 0
588+
589+
590+
# ── PowerShell Dry-Run Tests ─────────────────────────────────────────────────
591+
592+
593+
def _has_pwsh() -> bool:
594+
"""Check if pwsh is available."""
595+
try:
596+
subprocess.run(["pwsh", "--version"], capture_output=True, check=True)
597+
return True
598+
except (FileNotFoundError, subprocess.CalledProcessError):
599+
return False
600+
601+
602+
def run_ps_script(cwd: Path, *args: str) -> subprocess.CompletedProcess:
603+
"""Run create-new-feature.ps1 with given args."""
604+
cmd = ["pwsh", "-NoProfile", "-File", str(CREATE_FEATURE_PS), *args]
605+
return subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
606+
607+
608+
@pytest.fixture
609+
def ps_git_repo(tmp_path: Path) -> Path:
610+
"""Create a temp git repo with PowerShell scripts and .specify dir."""
611+
subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True)
612+
subprocess.run(
613+
["git", "config", "user.email", "test@example.com"], cwd=tmp_path, check=True
614+
)
615+
subprocess.run(
616+
["git", "config", "user.name", "Test User"], cwd=tmp_path, check=True
617+
)
618+
subprocess.run(
619+
["git", "commit", "--allow-empty", "-m", "init", "-q"],
620+
cwd=tmp_path,
621+
check=True,
622+
)
623+
ps_dir = tmp_path / "scripts" / "powershell"
624+
ps_dir.mkdir(parents=True)
625+
shutil.copy(CREATE_FEATURE_PS, ps_dir / "create-new-feature.ps1")
626+
common_ps = PROJECT_ROOT / "scripts" / "powershell" / "common.ps1"
627+
shutil.copy(common_ps, ps_dir / "common.ps1")
628+
(tmp_path / ".specify" / "templates").mkdir(parents=True)
629+
return tmp_path
630+
631+
632+
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not available")
633+
class TestPowerShellDryRun:
634+
def test_ps_dry_run_outputs_name(self, ps_git_repo: Path):
635+
"""PowerShell -DryRun computes correct branch name."""
636+
(ps_git_repo / "specs" / "001-first").mkdir(parents=True)
637+
result = run_ps_script(
638+
ps_git_repo, "-DryRun", "-ShortName", "ps-feat", "PS feature"
639+
)
640+
assert result.returncode == 0, result.stderr
641+
branch = None
642+
for line in result.stdout.splitlines():
643+
if line.startswith("BRANCH_NAME:"):
644+
branch = line.split(":", 1)[1].strip()
645+
assert branch == "002-ps-feat", f"expected 002-ps-feat, got: {branch}"
646+
647+
def test_ps_dry_run_no_branch_created(self, ps_git_repo: Path):
648+
"""PowerShell -DryRun does not create a git branch."""
649+
result = run_ps_script(
650+
ps_git_repo, "-DryRun", "-ShortName", "no-ps-branch", "No branch"
651+
)
652+
assert result.returncode == 0, result.stderr
653+
branches = subprocess.run(
654+
["git", "branch", "--list", "*no-ps-branch*"],
655+
cwd=ps_git_repo,
656+
capture_output=True,
657+
text=True,
658+
)
659+
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
660+
assert branches.stdout.strip() == "", "branch should not exist after dry-run"
661+
662+
def test_ps_dry_run_no_spec_dir_created(self, ps_git_repo: Path):
663+
"""PowerShell -DryRun does not create specs/ directory."""
664+
specs_root = ps_git_repo / "specs"
665+
if specs_root.exists():
666+
shutil.rmtree(specs_root)
667+
assert not specs_root.exists()
668+
669+
result = run_ps_script(
670+
ps_git_repo, "-DryRun", "-ShortName", "no-ps-dir", "No dir"
671+
)
672+
assert result.returncode == 0, result.stderr
673+
assert not specs_root.exists(), "specs/ should not be created during dry-run"
674+
675+
def test_ps_dry_run_json_includes_field(self, ps_git_repo: Path):
676+
"""PowerShell -DryRun JSON output includes DRY_RUN field."""
677+
import json
678+
679+
result = run_ps_script(
680+
ps_git_repo, "-DryRun", "-Json", "-ShortName", "ps-json", "JSON test"
681+
)
682+
assert result.returncode == 0, result.stderr
683+
data = json.loads(result.stdout)
684+
assert "DRY_RUN" in data, f"DRY_RUN missing from JSON: {data}"
685+
assert data["DRY_RUN"] is True
686+
687+
def test_ps_dry_run_json_absent_without_flag(self, ps_git_repo: Path):
688+
"""PowerShell normal JSON output does NOT include DRY_RUN field."""
689+
import json
690+
691+
result = run_ps_script(
692+
ps_git_repo, "-Json", "-ShortName", "ps-no-dry", "No dry run"
693+
)
694+
assert result.returncode == 0, result.stderr
695+
data = json.loads(result.stdout)
696+
assert "DRY_RUN" not in data, f"DRY_RUN should not be in normal JSON: {data}"

0 commit comments

Comments
 (0)