mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(scripts): count subdirectory-only dirs as non-empty in PowerShell (parity with bash) (#3137)
* fix(scripts): count subdirectory-only dirs as non-empty in PowerShell
Test-DirHasFiles (the documented PowerShell twin of bash check_dir) tested
non-emptiness with `Get-ChildItem | Where-Object { -not $_.PSIsContainer }`,
counting only top-level FILES and ignoring subdirectories. Bash check_dir
(`-n $(ls -A ...)`) and the PowerShell JSON-path contracts checks
(check-prerequisites.ps1 / setup-tasks.ps1, no PSIsContainer filter) both
count ANY entry. So a contracts/ directory whose only contents are
subdirectories (e.g. contracts/v1/openapi.yaml) was reported present by
bash, by bash JSON, and by PowerShell JSON, but [FAIL]/absent by PowerShell
text mode — the lone outlier.
Drop the PSIsContainer filter so Test-DirHasFiles counts any entry, matching
the other three code paths.
Add bash + PowerShell parity tests asserting a subdir-only contracts/ dir is
reported non-empty in both shells.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: accurate non-empty comment + drop doubled test prefix
Address review feedback on Test-DirHasFiles parity fix:
- Reword the common.ps1 comment so it no longer claims exact `ls -A` parity (Get-ChildItem omits hidden entries without -Force); it now points at the in-repo PowerShell JSON contracts checks as the matching reference and keeps the subdir-only-is-non-empty rationale.
- Rename test_test_dir_has_files_ps_... -> test_dir_has_files_ps_... to drop the doubled 'test_' prefix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: assert dir-non-emptiness via stdout marker, not exit code
Address Copilot review: check_dir always exits 0 (it echoes the marker rather than setting an exit code) and Test-DirHasFiles returns a boolean (pwsh still exits 0 when it returns $false), so 'result.returncode == 0' validated nothing. Drop the misleading assertion and rely on the [OK]/checkmark marker in stdout, which is the actual behavioral signal; document why inline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: keep common.ps1 ASCII-only (PowerShell 5.1 compatibility)
My reworded Test-DirHasFiles comment introduced an em dash (U+2014), which tripped tests/test_ps1_encoding.py::test_ps1_file_is_ascii_only -- .ps1 files must stay ASCII for Windows PowerShell 5.1. Replace it with '--', matching the existing comment style in this file (e.g. the Resolve-SpecifyInitDir docstring).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: decode dir-parity subprocess output as UTF-8 explicitly
Address Copilot review: check_dir echoes the non-ASCII markers ✓/✗, and subprocess.run with text=True but no encoding decodes via the platform locale (cp1252 on Windows), which can raise UnicodeDecodeError or mangle stdout. Pin encoding='utf-8' on both the bash and PowerShell dir-parity helpers so decoding is deterministic across CI runners.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -209,7 +209,13 @@ function Test-FileExists {
|
|||||||
|
|
||||||
function Test-DirHasFiles {
|
function Test-DirHasFiles {
|
||||||
param([string]$Path, [string]$Description)
|
param([string]$Path, [string]$Description)
|
||||||
if ((Test-Path -Path $Path -PathType Container) -and (Get-ChildItem -Path $Path -ErrorAction SilentlyContinue | Where-Object { -not $_.PSIsContainer } | Select-Object -First 1)) {
|
# A directory counts as non-empty when Get-ChildItem returns any entry
|
||||||
|
# (files or subdirectories) -- matching the JSON contracts checks in
|
||||||
|
# check-prerequisites.ps1 / setup-tasks.ps1, and treating a directory whose
|
||||||
|
# only contents are subdirectories (e.g. contracts/v1/openapi.yaml) as
|
||||||
|
# non-empty like bash check_dir. Filtering out subdirectories would
|
||||||
|
# mis-report such a directory as empty.
|
||||||
|
if ((Test-Path -Path $Path -PathType Container) -and (Get-ChildItem -Path $Path -ErrorAction SilentlyContinue | Select-Object -First 1)) {
|
||||||
Write-Output " [OK] $Description"
|
Write-Output " [OK] $Description"
|
||||||
return $true
|
return $true
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -840,3 +840,54 @@ def test_setup_tasks_ps_errors_without_feature_context(
|
|||||||
output = result.stderr + result.stdout
|
output = result.stderr + result.stdout
|
||||||
assert result.returncode != 0
|
assert result.returncode != 0
|
||||||
assert "Feature directory not found" in output
|
assert "Feature directory not found" in output
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Directory non-emptiness parity: a dir whose only contents are subdirectories
|
||||||
|
# (e.g. contracts/v1/openapi.yaml) must count as non-empty in both shells.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def _run_bash_check_dir(repo: Path, target: Path) -> subprocess.CompletedProcess:
|
||||||
|
script = repo / ".specify" / "scripts" / "bash" / "common.sh"
|
||||||
|
return subprocess.run(
|
||||||
|
["bash", "-c", 'source "$1"; check_dir "$2" "contracts/"', "bash", str(script), str(target)],
|
||||||
|
# check_dir echoes the non-ASCII markers ✓/✗; decode UTF-8 explicitly so
|
||||||
|
# the result does not depend on the platform locale (e.g. cp1252 on Windows).
|
||||||
|
cwd=repo, capture_output=True, text=True, encoding="utf-8", check=False, env=_clean_env(),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _run_powershell_test_dir(repo: Path, target: Path) -> subprocess.CompletedProcess:
|
||||||
|
script = repo / ".specify" / "scripts" / "powershell" / "common.ps1"
|
||||||
|
exe = "pwsh" if HAS_PWSH else _WINDOWS_POWERSHELL
|
||||||
|
return subprocess.run(
|
||||||
|
[exe, "-NoProfile", "-Command",
|
||||||
|
'& { param($common, $dir) . $common; Test-DirHasFiles -Path $dir -Description "contracts/" }',
|
||||||
|
str(script), str(target)],
|
||||||
|
cwd=repo, capture_output=True, text=True, encoding="utf-8", check=False, env=_clean_env(),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@requires_bash
|
||||||
|
def test_check_dir_bash_counts_subdir_only_contracts(tasks_repo: Path) -> None:
|
||||||
|
"""bash check_dir treats a dir containing only subdirectories as non-empty."""
|
||||||
|
contracts = tasks_repo / "contracts" / "v1"
|
||||||
|
contracts.mkdir(parents=True)
|
||||||
|
(contracts / "openapi.yaml").write_text("openapi: 3.0\n", encoding="utf-8")
|
||||||
|
result = _run_bash_check_dir(tasks_repo, tasks_repo / "contracts")
|
||||||
|
# check_dir always exits 0 (it echoes ✓/✗ instead of setting an exit code),
|
||||||
|
# so the ✓ marker in stdout — not the return code — is what proves non-emptiness.
|
||||||
|
assert "✓" in result.stdout and "✗" not in result.stdout, result.stderr + result.stdout
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(not (HAS_PWSH or _WINDOWS_POWERSHELL), reason="no PowerShell available")
|
||||||
|
def test_dir_has_files_ps_counts_subdir_only_contracts(tasks_repo: Path) -> None:
|
||||||
|
"""Test-DirHasFiles must match bash: a subdir-only dir counts as non-empty."""
|
||||||
|
contracts = tasks_repo / "contracts" / "v1"
|
||||||
|
contracts.mkdir(parents=True)
|
||||||
|
(contracts / "openapi.yaml").write_text("openapi: 3.0\n", encoding="utf-8")
|
||||||
|
result = _run_powershell_test_dir(tasks_repo, tasks_repo / "contracts")
|
||||||
|
# Test-DirHasFiles returns a boolean and pwsh still exits 0 when it returns
|
||||||
|
# $false, so the [OK] marker in stdout — not the return code — is what proves
|
||||||
|
# non-emptiness.
|
||||||
|
assert "[OK]" in result.stdout and "[FAIL]" not in result.stdout, result.stderr + result.stdout
|
||||||
|
|||||||
Reference in New Issue
Block a user