Skip to content

Commit 86a3cb1

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 github#1998. Assisted-By: 🤖 Claude Code
1 parent b92ace5 commit 86a3cb1

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
@@ -179,7 +179,9 @@ fi
179179
cd "$REPO_ROOT"
180180

181181
SPECS_DIR="$REPO_ROOT/specs"
182-
mkdir -p "$SPECS_DIR"
182+
if [ "$DRY_RUN" != true ]; then
183+
mkdir -p "$SPECS_DIR"
184+
fi
183185

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

scripts/powershell/create-new-feature.ps1

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

136136
$specsDir = Join-Path $repoRoot 'specs'
137-
New-Item -ItemType Directory -Path $specsDir -Force | Out-Null
137+
if (-not $DryRun) {
138+
New-Item -ItemType Directory -Path $specsDir -Force | Out-Null
139+
}
138140

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

tests/test_timestamp_branches.py

Lines changed: 117 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,20 +301,21 @@ def test_dry_run_no_branch_created(self, git_repo: Path):
301301
capture_output=True,
302302
text=True,
303303
)
304+
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
304305
assert branches.stdout.strip() == "", "branch should not exist after dry-run"
305306

306307
def test_dry_run_no_spec_dir_created(self, git_repo: Path):
307-
"""T011: Dry-run does not create a spec directory."""
308+
"""T011: Dry-run does not create any directories (including root specs/)."""
309+
specs_root = git_repo / "specs"
310+
if specs_root.exists():
311+
shutil.rmtree(specs_root)
312+
assert not specs_root.exists(), "specs/ should not exist before dry-run"
313+
308314
result = run_script(
309315
git_repo, "--dry-run", "--short-name", "no-dir", "No dir feature"
310316
)
311317
assert result.returncode == 0, result.stderr
312-
spec_dirs = [
313-
d.name
314-
for d in (git_repo / "specs").iterdir()
315-
if d.is_dir() and "no-dir" in d.name
316-
] if (git_repo / "specs").exists() else []
317-
assert len(spec_dirs) == 0, f"spec dir should not exist: {spec_dirs}"
318+
assert not specs_root.exists(), "specs/ should not be created during dry-run"
318319

319320
def test_dry_run_empty_repo(self, git_repo: Path):
320321
"""T012: Dry-run returns 001 prefix when no existing specs or branches."""
@@ -441,3 +442,112 @@ def test_dry_run_no_git(self, no_git_dir: Path):
441442
if d.is_dir() and "no-git-dry" in d.name
442443
]
443444
assert len(spec_dirs) == 0
445+
446+
447+
# ── PowerShell Dry-Run Tests ─────────────────────────────────────────────────
448+
449+
450+
def _has_pwsh() -> bool:
451+
"""Check if pwsh is available."""
452+
try:
453+
subprocess.run(["pwsh", "--version"], capture_output=True, check=True)
454+
return True
455+
except (FileNotFoundError, subprocess.CalledProcessError):
456+
return False
457+
458+
459+
def run_ps_script(cwd: Path, *args: str) -> subprocess.CompletedProcess:
460+
"""Run create-new-feature.ps1 with given args."""
461+
cmd = ["pwsh", "-NoProfile", "-File", str(CREATE_FEATURE_PS), *args]
462+
return subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
463+
464+
465+
@pytest.fixture
466+
def ps_git_repo(tmp_path: Path) -> Path:
467+
"""Create a temp git repo with PowerShell scripts and .specify dir."""
468+
subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True)
469+
subprocess.run(
470+
["git", "config", "user.email", "test@example.com"], cwd=tmp_path, check=True
471+
)
472+
subprocess.run(
473+
["git", "config", "user.name", "Test User"], cwd=tmp_path, check=True
474+
)
475+
subprocess.run(
476+
["git", "commit", "--allow-empty", "-m", "init", "-q"],
477+
cwd=tmp_path,
478+
check=True,
479+
)
480+
ps_dir = tmp_path / "scripts" / "powershell"
481+
ps_dir.mkdir(parents=True)
482+
shutil.copy(CREATE_FEATURE_PS, ps_dir / "create-new-feature.ps1")
483+
common_ps = PROJECT_ROOT / "scripts" / "powershell" / "common.ps1"
484+
shutil.copy(common_ps, ps_dir / "common.ps1")
485+
(tmp_path / ".specify" / "templates").mkdir(parents=True)
486+
return tmp_path
487+
488+
489+
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not available")
490+
class TestPowerShellDryRun:
491+
def test_ps_dry_run_outputs_name(self, ps_git_repo: Path):
492+
"""PowerShell -DryRun computes correct branch name."""
493+
(ps_git_repo / "specs" / "001-first").mkdir(parents=True)
494+
result = run_ps_script(
495+
ps_git_repo, "-DryRun", "-ShortName", "ps-feat", "PS feature"
496+
)
497+
assert result.returncode == 0, result.stderr
498+
branch = None
499+
for line in result.stdout.splitlines():
500+
if line.startswith("BRANCH_NAME:"):
501+
branch = line.split(":", 1)[1].strip()
502+
assert branch == "002-ps-feat", f"expected 002-ps-feat, got: {branch}"
503+
504+
def test_ps_dry_run_no_branch_created(self, ps_git_repo: Path):
505+
"""PowerShell -DryRun does not create a git branch."""
506+
result = run_ps_script(
507+
ps_git_repo, "-DryRun", "-ShortName", "no-ps-branch", "No branch"
508+
)
509+
assert result.returncode == 0, result.stderr
510+
branches = subprocess.run(
511+
["git", "branch", "--list", "*no-ps-branch*"],
512+
cwd=ps_git_repo,
513+
capture_output=True,
514+
text=True,
515+
)
516+
assert branches.returncode == 0, f"'git branch --list' failed: {branches.stderr}"
517+
assert branches.stdout.strip() == "", "branch should not exist after dry-run"
518+
519+
def test_ps_dry_run_no_spec_dir_created(self, ps_git_repo: Path):
520+
"""PowerShell -DryRun does not create specs/ directory."""
521+
specs_root = ps_git_repo / "specs"
522+
if specs_root.exists():
523+
shutil.rmtree(specs_root)
524+
assert not specs_root.exists()
525+
526+
result = run_ps_script(
527+
ps_git_repo, "-DryRun", "-ShortName", "no-ps-dir", "No dir"
528+
)
529+
assert result.returncode == 0, result.stderr
530+
assert not specs_root.exists(), "specs/ should not be created during dry-run"
531+
532+
def test_ps_dry_run_json_includes_field(self, ps_git_repo: Path):
533+
"""PowerShell -DryRun JSON output includes DRY_RUN field."""
534+
import json
535+
536+
result = run_ps_script(
537+
ps_git_repo, "-DryRun", "-Json", "-ShortName", "ps-json", "JSON test"
538+
)
539+
assert result.returncode == 0, result.stderr
540+
data = json.loads(result.stdout)
541+
assert "DRY_RUN" in data, f"DRY_RUN missing from JSON: {data}"
542+
assert data["DRY_RUN"] is True
543+
544+
def test_ps_dry_run_json_absent_without_flag(self, ps_git_repo: Path):
545+
"""PowerShell normal JSON output does NOT include DRY_RUN field."""
546+
import json
547+
548+
result = run_ps_script(
549+
ps_git_repo, "-Json", "-ShortName", "ps-no-dry", "No dry run"
550+
)
551+
assert result.returncode == 0, result.stderr
552+
data = json.loads(result.stdout)
553+
assert "DRY_RUN" not in data, f"DRY_RUN should not be in normal JSON: {data}"

0 commit comments

Comments
 (0)