mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(powershell): ensure UTF-8 templates are written without BOM (#2280)
* fix(powershell): strip BOM from templates and ensure No-BOM output * fix: address review feedback on encoding and naming for all ps scripts * fix: address copilot feedback (encoding detection and variable naming) * fix: remove duplicate comments in setup-plan.ps1 * test: verify spec.md is written without UTF-8 BOM * test: also verify BOM-free output under Windows PowerShell 5.1 * fix * fix: resolve merge conflict with main, add TestDescriptionQuoting * fix: resolve TestDescriptionQuoting string quoting conflict with main * test: restore PowerShell prefix-stripping parity test in TestGetFeaturePathsSinglePrefix * fix: remove trailing whitespace from module docstring blank lines * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * test: seed ps_git_repo with BOM-prefixed template to exercise WriteAllText fix * fix: remove duplicate ps_git_repo fixture, restore ext_ps_git_repo * fix: remove unrelated TestDescriptionQuoting and restore original test_ps_specify_feature_prefixed_resolves_by_prefix --------- Co-authored-by: Nimraakram22 <nimra.akram123451@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -350,7 +350,10 @@ if (-not $DryRun) {
|
|||||||
if (-not (Test-Path -PathType Leaf $specFile)) {
|
if (-not (Test-Path -PathType Leaf $specFile)) {
|
||||||
$template = Resolve-Template -TemplateName 'spec-template' -RepoRoot $repoRoot
|
$template = Resolve-Template -TemplateName 'spec-template' -RepoRoot $repoRoot
|
||||||
if ($template -and (Test-Path $template)) {
|
if ($template -and (Test-Path $template)) {
|
||||||
Copy-Item $template $specFile -Force
|
# Read the template content and write it to the spec file with UTF-8 encoding without BOM
|
||||||
|
$content = [System.IO.File]::ReadAllText($template)
|
||||||
|
$utf8NoBom = New-Object System.Text.UTF8Encoding($false)
|
||||||
|
[System.IO.File]::WriteAllText($specFile, $content, $utf8NoBom)
|
||||||
} else {
|
} else {
|
||||||
New-Item -ItemType File -Path $specFile -Force | Out-Null
|
New-Item -ItemType File -Path $specFile -Force | Out-Null
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -36,8 +36,10 @@ New-Item -ItemType Directory -Path $paths.FEATURE_DIR -Force | Out-Null
|
|||||||
# Copy plan template if it exists, otherwise note it or create empty file
|
# Copy plan template if it exists, otherwise note it or create empty file
|
||||||
$template = Resolve-Template -TemplateName 'plan-template' -RepoRoot $paths.REPO_ROOT
|
$template = Resolve-Template -TemplateName 'plan-template' -RepoRoot $paths.REPO_ROOT
|
||||||
if ($template -and (Test-Path $template)) {
|
if ($template -and (Test-Path $template)) {
|
||||||
Copy-Item $template $paths.IMPL_PLAN -Force
|
# Read the template content and write it to the implementation plan file with UTF-8 encoding without BOM
|
||||||
Write-Output "Copied plan template to $($paths.IMPL_PLAN)"
|
$content = [System.IO.File]::ReadAllText($template)
|
||||||
|
$utf8NoBom = New-Object System.Text.UTF8Encoding($false)
|
||||||
|
[System.IO.File]::WriteAllText($paths.IMPL_PLAN, $content, $utf8NoBom)
|
||||||
} else {
|
} else {
|
||||||
Write-Warning "Plan template not found"
|
Write-Warning "Plan template not found"
|
||||||
# Create a basic plan file if template doesn't exist
|
# Create a basic plan file if template doesn't exist
|
||||||
|
|||||||
@@ -115,6 +115,36 @@ def ext_ps_git_repo(tmp_path: Path) -> Path:
|
|||||||
return tmp_path
|
return tmp_path
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def ps_git_repo(tmp_path: Path) -> Path:
|
||||||
|
"""Create a temp git repo with PowerShell scripts and a BOM-prefixed template."""
|
||||||
|
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")
|
||||||
|
templates_dir = tmp_path / ".specify" / "templates"
|
||||||
|
templates_dir.mkdir(parents=True)
|
||||||
|
# Write a BOM-prefixed template to ensure the WriteAllText fix is actually exercised.
|
||||||
|
# If WriteAllText regresses, the output file will contain the BOM.
|
||||||
|
bom = b"\xef\xbb\xbf"
|
||||||
|
template_content = "# Feature Spec\n\nDescribe the feature here.\n"
|
||||||
|
(templates_dir / "spec-template.md").write_bytes(bom + template_content.encode("utf-8"))
|
||||||
|
return tmp_path
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def no_git_dir(tmp_path: Path) -> Path:
|
def no_git_dir(tmp_path: Path) -> Path:
|
||||||
"""Create a temp directory without git, but with scripts."""
|
"""Create a temp directory without git, but with scripts."""
|
||||||
@@ -381,6 +411,7 @@ class TestGetFeaturePathsSinglePrefix:
|
|||||||
assert result.returncode == 0, result.stderr
|
assert result.returncode == 0, result.stderr
|
||||||
assert result.stdout.strip() == str(tmp_path / "specs" / "001-target-spec")
|
assert result.stdout.strip() == str(tmp_path / "specs" / "001-target-spec")
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed")
|
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed")
|
||||||
def test_ps_specify_feature_prefixed_resolves_by_prefix(self, git_repo: Path):
|
def test_ps_specify_feature_prefixed_resolves_by_prefix(self, git_repo: Path):
|
||||||
"""PowerShell Get-FeaturePathsEnv: same prefix stripping as bash."""
|
"""PowerShell Get-FeaturePathsEnv: same prefix stripping as bash."""
|
||||||
@@ -650,6 +681,45 @@ class TestAllowExistingBranchPowerShell:
|
|||||||
assert "$switchBranchError = git checkout -q $branchName 2>&1 | Out-String" in contents
|
assert "$switchBranchError = git checkout -q $branchName 2>&1 | Out-String" in contents
|
||||||
assert "exists but could not be checked out.`n$($switchBranchError.Trim())" in contents
|
assert "exists but could not be checked out.`n$($switchBranchError.Trim())" in contents
|
||||||
|
|
||||||
|
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed")
|
||||||
|
@pytest.mark.skipif(
|
||||||
|
os.name != "nt" or shutil.which("powershell.exe") is None,
|
||||||
|
reason="Windows PowerShell not installed",
|
||||||
|
)
|
||||||
|
def test_ps_spec_file_written_without_bom(self, ps_git_repo: Path):
|
||||||
|
"""spec.md generated from a BOM-prefixed template must not contain a UTF-8 BOM."""
|
||||||
|
result = subprocess.run(
|
||||||
|
[
|
||||||
|
"powershell.exe",
|
||||||
|
"-NoProfile",
|
||||||
|
"-ExecutionPolicy",
|
||||||
|
"Bypass",
|
||||||
|
"-File",
|
||||||
|
str(CREATE_FEATURE_PS),
|
||||||
|
"-ShortName",
|
||||||
|
"bom-check",
|
||||||
|
"BOM check feature",
|
||||||
|
],
|
||||||
|
cwd=ps_git_repo,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
)
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
|
||||||
|
spec_file = next((ps_git_repo / "specs").rglob("spec.md"), None)
|
||||||
|
assert spec_file is not None, (
|
||||||
|
f"spec.md was not created.\nstdout: {result.stdout}\nstderr: {result.stderr}"
|
||||||
|
)
|
||||||
|
|
||||||
|
raw = spec_file.read_bytes()
|
||||||
|
assert not raw.startswith(b"\xef\xbb\xbf"), (
|
||||||
|
f"spec.md must not start with a UTF-8 BOM — got first 3 bytes: {raw[:3]!r}"
|
||||||
|
)
|
||||||
|
# Verify template content was copied (not just an empty New-Item fallback)
|
||||||
|
assert "Feature Spec" in raw.decode("utf-8"), (
|
||||||
|
"spec.md does not contain template content — WriteAllText path was not exercised"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TestGitExtensionParity:
|
class TestGitExtensionParity:
|
||||||
def test_bash_extension_surfaces_checkout_errors(self):
|
def test_bash_extension_surfaces_checkout_errors(self):
|
||||||
@@ -904,30 +974,6 @@ def run_ps_script(cwd: Path, *args: str) -> subprocess.CompletedProcess:
|
|||||||
return subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
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")
|
@pytest.mark.skipif(not _has_pwsh(), reason="pwsh not available")
|
||||||
class TestPowerShellDryRun:
|
class TestPowerShellDryRun:
|
||||||
def test_ps_dry_run_outputs_name(self, ps_git_repo: Path):
|
def test_ps_dry_run_outputs_name(self, ps_git_repo: Path):
|
||||||
@@ -1259,13 +1305,13 @@ class TestFeatureDirectoryResolution:
|
|||||||
pytest.fail("FEATURE_DIR not found in PowerShell output")
|
pytest.fail("FEATURE_DIR not found in PowerShell output")
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
# ── Description Quoting Tests (issue #2339) ──────────────────────────────────
|
# ── Description Quoting Tests (issue #2339) ──────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
@requires_bash
|
@requires_bash
|
||||||
class TestDescriptionQuoting:
|
class TestDescriptionQuoting:
|
||||||
"""Descriptions with quotes, apostrophes, and backslashes must not break the script.
|
"""Descriptions with quotes, apostrophes, and backslashes must not break the script.
|
||||||
|
|
||||||
Regression tests for https://github.com/github/spec-kit/issues/2339
|
Regression tests for https://github.com/github/spec-kit/issues/2339
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@@ -1273,9 +1319,9 @@ class TestDescriptionQuoting:
|
|||||||
"description",
|
"description",
|
||||||
[
|
[
|
||||||
"Add user's profile page",
|
"Add user's profile page",
|
||||||
"Fix the \"login\" bug",
|
'Fix the "login" bug',
|
||||||
"Handle path\\with\\backslashes",
|
"Handle path\\with\\backslashes",
|
||||||
"It's a \"complex\" feature\\here",
|
'It\'s a "complex" feature\\here',
|
||||||
],
|
],
|
||||||
ids=["apostrophe", "double-quotes", "backslashes", "mixed"],
|
ids=["apostrophe", "double-quotes", "backslashes", "mixed"],
|
||||||
)
|
)
|
||||||
@@ -1290,16 +1336,22 @@ class TestDescriptionQuoting:
|
|||||||
"description",
|
"description",
|
||||||
[
|
[
|
||||||
"Add user's profile page",
|
"Add user's profile page",
|
||||||
"Fix the \"login\" bug",
|
'Fix the "login" bug',
|
||||||
"Handle path\\with\\backslashes",
|
"Handle path\\with\\backslashes",
|
||||||
"It's a \"complex\" feature\\here",
|
'It\'s a "complex" feature\\here',
|
||||||
],
|
],
|
||||||
ids=["apostrophe", "double-quotes", "backslashes", "mixed"],
|
ids=["apostrophe", "double-quotes", "backslashes", "mixed"],
|
||||||
)
|
)
|
||||||
def test_ext_script_handles_special_chars(self, ext_git_repo: Path, description: str):
|
def test_ext_script_handles_special_chars(self, ext_git_repo: Path, description: str):
|
||||||
"""Extension create-new-feature.sh succeeds with special characters in description."""
|
"""Extension create-new-feature.sh succeeds with special characters in description."""
|
||||||
script = (
|
script = (
|
||||||
ext_git_repo / ".specify" / "extensions" / "git" / "scripts" / "bash" / "create-new-feature.sh"
|
ext_git_repo
|
||||||
|
/ ".specify"
|
||||||
|
/ "extensions"
|
||||||
|
/ "git"
|
||||||
|
/ "scripts"
|
||||||
|
/ "bash"
|
||||||
|
/ "create-new-feature.sh"
|
||||||
)
|
)
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["bash", str(script), "--dry-run", "--short-name", "feat", description],
|
["bash", str(script), "--dry-run", "--short-name", "feat", description],
|
||||||
@@ -1321,3 +1373,4 @@ class TestDescriptionQuoting:
|
|||||||
"""Plain description without special characters continues to work."""
|
"""Plain description without special characters continues to work."""
|
||||||
result = run_script(git_repo, "--dry-run", "--short-name", "feat", "Add login feature")
|
result = run_script(git_repo, "--dry-run", "--short-name", "feat", "Add login feature")
|
||||||
assert result.returncode == 0, result.stderr
|
assert result.returncode == 0, result.stderr
|
||||||
|
|
||||||
Reference in New Issue
Block a user