From a157b12278c0dde9ee2f99d329333a3f99178d6c Mon Sep 17 00:00:00 2001 From: Gilbert Sanchez Date: Fri, 12 Jun 2026 21:10:34 -0700 Subject: [PATCH 1/3] fix: warn when Git target exists but is not a valid git repository (#86) Previously, if a directory already existed at the clone target but was not a git repository, `git rev-parse` returned nothing and the handler silently fell into the "we don't support moving versions" branch with null branch/commit values, producing a confusing verbose message and a no-op install. Now emits `Write-Warning` in that case so the user knows why the dependency was skipped. Also confirmed that the nesting bug from #86 (Repo/Repo on repeated runs) does not reproduce on current code; adds two Pester tests to lock in idempotency and the non-git-directory warning path. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 22 +++++++++++++++++--- PSDepend/PSDependScripts/Git.ps1 | 6 +++++- Tests/Git.Type.Tests.ps1 | 35 ++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4904be8..fbe8639 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,15 +9,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `FileDownload` is now supported on all platforms (`windows`, `core`, `macos`, `linux`); there was no Windows-only code blocking this (#98). -- `FileDownload` relative `Target` paths are now rooted against `$PWD` before resolution, matching the intuitive expectation of callers (#49). +- `FileDownload` is now supported on all platforms (`windows`, `core`, + `macos`, `linux`); there was no Windows-only code blocking this (#98). +- `FileDownload` relative `Target` paths are now rooted against `$PWD` + before resolution, matching the intuitive expectation of callers (#49). ### Fixed - `Get-Dependency -InputObject` no longer mutates the caller's hashtable: `PSDependOptions` is now preserved after the call, so a second invocation with the same object still honors global options such as `Target` (#35). -- `FileDownload` no longer misidentifies a directory-like `Target` (no file extension, or trailing slash) as a full file path when its parent happens to exist; the handler now uses a file-extension heuristic to distinguish file targets from container targets and creates the directory when it does not yet exist (#49). +- `FileDownload` no longer misidentifies a directory-like `Target` (no + file extension, or trailing slash) as a full file path when its parent + happens to exist; the handler now uses a file-extension heuristic to + distinguish file targets from container targets and creates the directory + when it does not yet exist (#49). +- `Git` handler no longer silently ignores a pre-existing directory at + the clone target that is not a valid git repository; it now emits a + `Write-Warning` so the user knows why the install was skipped (#86). + +### Tests + +- Added idempotency test: a second install run against an already-cloned + repo does not trigger a second `git clone` (#86). +- Added non-git-directory test: confirms a warning is emitted and no + clone is attempted when the target path exists but is not a git repo (#86). ## [0.4.1] - 2026-06-12 diff --git a/PSDepend/PSDependScripts/Git.ps1 b/PSDepend/PSDependScripts/Git.ps1 index 64c4e7a..ad9b452 100644 --- a/PSDepend/PSDependScripts/Git.ps1 +++ b/PSDepend/PSDependScripts/Git.ps1 @@ -132,7 +132,11 @@ if ($GottaTest) { $Branch = Invoke-ExternalCommand git -Arguments (Write-Output rev-parse --abbrev-ref HEAD) -Passthru $Commit = Invoke-ExternalCommand git -Arguments (Write-Output rev-parse HEAD) -Passthru Pop-Location - if ($Version -eq $Branch -or $Version -eq $Commit) { + if (-not $Branch) { + Write-Warning "[$RepoPath] exists but does not appear to be a valid git repository. Skipping [$DependencyName]." + $GottaInstall = $False + } + elseif ($Version -eq $Branch -or $Version -eq $Commit) { Write-Verbose "[$RepoPath] exists and is already at version [$Version]" if ($PSDependAction -contains 'Test' -and $PSDependAction.count -eq 1) { return $true diff --git a/Tests/Git.Type.Tests.ps1 b/Tests/Git.Type.Tests.ps1 index 848b94b..2175af4 100644 --- a/Tests/Git.Type.Tests.ps1 +++ b/Tests/Git.Type.Tests.ps1 @@ -74,4 +74,39 @@ Describe 'Git script' { $result | Should -Be $false Should -Invoke -CommandName Invoke-ExternalCommand -ModuleName PSDepend -Times 0 } + + It 'Does not clone again when repo already exists at the correct version (idempotency)' { + $targetDir = (New-Item 'TestDrive:/git-idempotent' -ItemType Directory -Force).FullName + # Pre-create the repo directory as if a prior run cloned it + $null = New-Item (Join-Path $targetDir 'repo') -ItemType Directory -Force + $dep = New-PSDependFixture -DependencyName 'https://example.com/user/repo.git' -DependencyType 'Git' -Version 'main' -Target $targetDir + + InModuleScope PSDepend -Parameters @{ Dep = $dep; ScriptPath = $script:ScriptPath } { + # rev-parse returns the branch name matching Version + Mock Invoke-ExternalCommand { + if ($Arguments -contains '--abbrev-ref') { return 'main' } + if ($Arguments -notcontains 'clone' -and $Arguments -notcontains '--abbrev-ref') { return 'abc1234' } + } + & $ScriptPath -Dependency $Dep + } + Should -Invoke -CommandName Invoke-ExternalCommand -ModuleName PSDepend -Times 0 -ParameterFilter { + $Arguments -contains 'clone' + } + } + + It 'Emits a warning and skips install when the repo path exists but is not a git repository' { + $targetDir = (New-Item 'TestDrive:/git-nongit' -ItemType Directory -Force).FullName + # Pre-create the directory but leave it empty (not a git repo) + $null = New-Item (Join-Path $targetDir 'repo') -ItemType Directory -Force + $dep = New-PSDependFixture -DependencyName 'https://example.com/user/repo.git' -DependencyType 'Git' -Version 'main' -Target $targetDir + + InModuleScope PSDepend -Parameters @{ Dep = $dep; ScriptPath = $script:ScriptPath } { + # rev-parse returns nothing (non-git directory) + Mock Invoke-ExternalCommand { } + { & $ScriptPath -Dependency $Dep } | Should -Not -Throw + } + Should -Invoke -CommandName Invoke-ExternalCommand -ModuleName PSDepend -Times 0 -ParameterFilter { + $Arguments -contains 'clone' + } + } } From 12bb12a07005b5e67baf22bcf299b1ebfbe0f898 Mon Sep 17 00:00:00 2001 From: Gilbert Sanchez Date: Sat, 13 Jun 2026 17:27:32 -0700 Subject: [PATCH 2/3] fix: address PR review feedback (#194) - Return $false (not $null) from Test action when target exists but is not a valid git repository, preserving the Boolean contract - Idempotency test now also asserts git checkout is not invoked - Warning test mocks Write-Warning and asserts it is called with the expected message Co-Authored-By: Claude Sonnet 4.6 --- PSDepend/PSDependScripts/Git.ps1 | 3 +++ Tests/Git.Type.Tests.ps1 | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/PSDepend/PSDependScripts/Git.ps1 b/PSDepend/PSDependScripts/Git.ps1 index ad9b452..3edd97f 100644 --- a/PSDepend/PSDependScripts/Git.ps1 +++ b/PSDepend/PSDependScripts/Git.ps1 @@ -134,6 +134,9 @@ if ($GottaTest) { Pop-Location if (-not $Branch) { Write-Warning "[$RepoPath] exists but does not appear to be a valid git repository. Skipping [$DependencyName]." + if ($PSDependAction -contains 'Test' -and $PSDependAction.count -eq 1) { + return $false + } $GottaInstall = $False } elseif ($Version -eq $Branch -or $Version -eq $Commit) { diff --git a/Tests/Git.Type.Tests.ps1 b/Tests/Git.Type.Tests.ps1 index 2175af4..dd4e7e6 100644 --- a/Tests/Git.Type.Tests.ps1 +++ b/Tests/Git.Type.Tests.ps1 @@ -92,6 +92,9 @@ Describe 'Git script' { Should -Invoke -CommandName Invoke-ExternalCommand -ModuleName PSDepend -Times 0 -ParameterFilter { $Arguments -contains 'clone' } + Should -Invoke -CommandName Invoke-ExternalCommand -ModuleName PSDepend -Times 0 -ParameterFilter { + $Arguments -contains 'checkout' + } } It 'Emits a warning and skips install when the repo path exists but is not a git repository' { @@ -103,7 +106,11 @@ Describe 'Git script' { InModuleScope PSDepend -Parameters @{ Dep = $dep; ScriptPath = $script:ScriptPath } { # rev-parse returns nothing (non-git directory) Mock Invoke-ExternalCommand { } - { & $ScriptPath -Dependency $Dep } | Should -Not -Throw + Mock Write-Warning { } + & $ScriptPath -Dependency $Dep + } + Should -Invoke -CommandName Write-Warning -ModuleName PSDepend -Times 1 -ParameterFilter { + $Message -like '*does not appear to be a valid git repository*' } Should -Invoke -CommandName Invoke-ExternalCommand -ModuleName PSDepend -Times 0 -ParameterFilter { $Arguments -contains 'clone' From 4c9df6a021746961b3d93872f83070cbac8e6b9a Mon Sep 17 00:00:00 2001 From: Gilbert Sanchez Date: Sat, 13 Jun 2026 17:34:01 -0700 Subject: [PATCH 3/3] docs: remove invalid Tests changelog section (#86) Keep a Changelog does not define a Tests category; test additions are implementation details, not user-facing changes. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbe8639..fb6335f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,13 +28,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the clone target that is not a valid git repository; it now emits a `Write-Warning` so the user knows why the install was skipped (#86). -### Tests - -- Added idempotency test: a second install run against an already-cloned - repo does not trigger a second `git clone` (#86). -- Added non-git-directory test: confirms a warning is emitted and no - clone is attempted when the target path exists but is not a git repo (#86). - ## [0.4.1] - 2026-06-12 ### Fixed