diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md index 5b419265a..ccd6e293b 100644 --- a/.github/instructions/tests.instructions.md +++ b/.github/instructions/tests.instructions.md @@ -22,9 +22,17 @@ Secrets: ### APP_ENT — PSModule Enterprise App -Homed in `MSX`. ClientID: `Iv23lieHcDQDwVV3alK1`. +Homed in `MSX` (enterprise slug: `msx`). ClientID: `Iv23lieHcDQDwVV3alK1`. Installed on [psmodule-test-org3](https://github.com/orgs/psmodule-test-org3) (enterprise org) with all permissions and push events. +Required enterprise-scoped permissions (configured on the app): + +- `enterprise_organization_installations: write` — required by `Install-GitHubApp` on enterprise-owned organizations + ([docs](https://docs.github.com/rest/enterprise-admin/organization-installations#install-a-github-app-on-an-enterprise-owned-organization)). + The Organizations test creates an enterprise organization and then installs the app on it; the + endpoint returns 404 (not 403) when this permission is missing, which makes a missing + permission look like a missing resource. See issue #596. + Secrets: `TEST_APP_ENT_CLIENT_ID`, `TEST_APP_ENT_PRIVATE_KEY` ### APP_ORG — PSModule Organization App diff --git a/tests/Actions.Tests.ps1 b/tests/Actions.Tests.ps1 index 3bd3223db..748e0fca4 100644 --- a/tests/Actions.Tests.ps1 +++ b/tests/Actions.Tests.ps1 @@ -54,7 +54,7 @@ Describe 'Actions' { Write-Host ($context | Format-List | Out-String) } } - $repoPrefix = "Test-$os-$TokenType" + $repoPrefix = "$testName-$os-$TokenType" $repoName = "$repoPrefix-$id" LogGroup "Using Repository - [$repoName]" { diff --git a/tests/AfterAll.ps1 b/tests/AfterAll.ps1 index 7bd8e6d24..7719f6538 100644 --- a/tests/AfterAll.ps1 +++ b/tests/AfterAll.ps1 @@ -11,7 +11,6 @@ LogGroup 'AfterAll - Global Test Teardown' { if (-not $env:Settings) { throw 'Settings environment variable is not set. Process-PSModule must populate it with the test suite configuration.' } - $prefix = 'Test' # Derive the list of OS names from the Settings JSON provided by Process-PSModule. try { @@ -30,6 +29,11 @@ LogGroup 'AfterAll - Global Test Teardown' { } Write-Host "Cleaning up test repositories for OSes: $($osNames -join ', ')" + # Source the single authoritative list of test-file repositories so setup and teardown + # always operate on the same set. See tests/Data/TestRepos.ps1. + $testRepos = . "$PSScriptRoot/Data/TestRepos.ps1" + $testNames = $testRepos.TestNames + $testNamesWithExtraRepos = $testRepos.TestNamesWithExtraRepos foreach ($authCase in $authCases) { $authCase.GetEnumerator() | ForEach-Object { Set-Variable -Name $_.Key -Value $_.Value } @@ -46,25 +50,27 @@ LogGroup 'AfterAll - Global Test Teardown' { Write-Host ($context | Format-List | Out-String) foreach ($os in $osNames) { - $repoPrefix = "$prefix-$os-$TokenType" - $repoName = "$repoPrefix-$id" + foreach ($testName in $testNames) { + $repoPrefix = "$testName-$os-$TokenType" + $repoName = "$repoPrefix-$id" - LogGroup "Repository cleanup - $AuthType-$TokenType - $os" { - # Use deterministic name lookups instead of listing all repos to reduce API calls. - $cleanupRepoNames = @($repoName) - if ($OwnerType -eq 'organization') { - $cleanupRepoNames += "$repoName-2", "$repoName-3" - } + LogGroup "Repository cleanup - $AuthType-$TokenType - $os - $testName" { + # Use deterministic name lookups instead of listing all repos to reduce API calls. + $cleanupRepoNames = @($repoName) + if ($OwnerType -eq 'organization' -and $testName -in $testNamesWithExtraRepos) { + $cleanupRepoNames += "$repoName-2", "$repoName-3" + } - foreach ($cleanupRepoName in $cleanupRepoNames) { - switch ($OwnerType) { - 'user' { - Get-GitHubRepository -Name $cleanupRepoName -ErrorAction SilentlyContinue | - Remove-GitHubRepository -Confirm:$false - } - 'organization' { - Get-GitHubRepository -Owner $Owner -Name $cleanupRepoName -ErrorAction SilentlyContinue | - Remove-GitHubRepository -Confirm:$false + foreach ($cleanupRepoName in $cleanupRepoNames) { + switch ($OwnerType) { + 'user' { + Get-GitHubRepository -Name $cleanupRepoName -ErrorAction SilentlyContinue | + Remove-GitHubRepository -Confirm:$false + } + 'organization' { + Get-GitHubRepository -Owner $Owner -Name $cleanupRepoName -ErrorAction SilentlyContinue | + Remove-GitHubRepository -Confirm:$false + } } } } diff --git a/tests/BeforeAll.ps1 b/tests/BeforeAll.ps1 index d1f247999..62aa9edec 100644 --- a/tests/BeforeAll.ps1 +++ b/tests/BeforeAll.ps1 @@ -28,6 +28,12 @@ LogGroup 'BeforeAll - Global Test Setup' { } Write-Host "Creating test repositories for OSes: $($osNames -join ', ')" + # Source the single authoritative list of test-file repositories so setup and teardown + # always operate on the same set. See tests/Data/TestRepos.ps1. + $testRepos = . "$PSScriptRoot/Data/TestRepos.ps1" + $testNames = $testRepos.TestNames + $testNamesWithExtraRepos = $testRepos.TestNamesWithExtraRepos + foreach ($authCase in $authCases) { $authCase.GetEnumerator() | ForEach-Object { Set-Variable -Name $_.Key -Value $_.Value } @@ -43,47 +49,49 @@ LogGroup 'BeforeAll - Global Test Setup' { Write-Host ($context | Format-List | Out-String) foreach ($os in $osNames) { - $repoPrefix = "Test-$os-$TokenType" - $repoName = "$repoPrefix-$id" + foreach ($testName in $testNames) { + $repoPrefix = "$testName-$os-$TokenType" + $repoName = "$repoPrefix-$id" - LogGroup "Repository setup - $AuthType-$TokenType - $os" { - # Clean up repos from a previous attempt of the same run (re-runs). - # Use deterministic name lookups instead of listing all repos to reduce API calls. - $cleanupRepoNames = @($repoName) - if ($OwnerType -eq 'organization') { - $cleanupRepoNames += "$repoName-2", "$repoName-3" - } + LogGroup "Repository setup - $AuthType-$TokenType - $os - $testName" { + # Clean up repos from a previous attempt of the same run (re-runs). + # Use deterministic name lookups instead of listing all repos to reduce API calls. + $cleanupRepoNames = @($repoName) + if ($OwnerType -eq 'organization' -and $testName -in $testNamesWithExtraRepos) { + $cleanupRepoNames += "$repoName-2", "$repoName-3" + } - foreach ($cleanupRepoName in $cleanupRepoNames) { - switch ($OwnerType) { - 'user' { - Get-GitHubRepository -Name $cleanupRepoName -ErrorAction SilentlyContinue | - Remove-GitHubRepository -Confirm:$false - } - 'organization' { - Get-GitHubRepository -Owner $Owner -Name $cleanupRepoName -ErrorAction SilentlyContinue | - Remove-GitHubRepository -Confirm:$false + foreach ($cleanupRepoName in $cleanupRepoNames) { + switch ($OwnerType) { + 'user' { + Get-GitHubRepository -Name $cleanupRepoName -ErrorAction SilentlyContinue | + Remove-GitHubRepository -Confirm:$false + } + 'organization' { + Get-GitHubRepository -Owner $Owner -Name $cleanupRepoName -ErrorAction SilentlyContinue | + Remove-GitHubRepository -Confirm:$false + } } } - } - # Provision the primary shared repository. - $repoParams = @{ - Name = $repoName - AddReadme = $true - License = 'mit' - Gitignore = 'VisualStudio' - } - switch ($OwnerType) { - 'user' { Set-GitHubRepository @repoParams } - 'organization' { Set-GitHubRepository @repoParams -Organization $Owner } - } + # Provision the primary per-test-file repository. + $repoParams = @{ + Name = $repoName + AddReadme = $true + License = 'mit' + Gitignore = 'VisualStudio' + } + switch ($OwnerType) { + 'user' { Set-GitHubRepository @repoParams } + 'organization' { Set-GitHubRepository @repoParams -Organization $Owner } + } - # Provision extra repositories needed by Secrets/Variables SelectedRepository tests. - # Only organization owners need them — those tests are skipped for user owners. - if ($OwnerType -eq 'organization') { - foreach ($suffix in 2, 3) { - Set-GitHubRepository -Organization $Owner -Name "$repoName-$suffix" + # Provision extra repositories needed by Secrets/Variables SelectedRepository tests. + # Only organization owners need them — those tests are skipped for user owners. + if ($OwnerType -eq 'organization' -and $testName -in $testNamesWithExtraRepos) { + foreach ($suffix in 2, 3) { + Set-GitHubRepository -Organization $Owner -Name "$repoName-$suffix" + } } } } diff --git a/tests/Data/TestRepos.ps1 b/tests/Data/TestRepos.ps1 new file mode 100644 index 000000000..a2b072da2 --- /dev/null +++ b/tests/Data/TestRepos.ps1 @@ -0,0 +1,11 @@ +# Test files that require their own per-test-file repository. +# Each test file's per-context BeforeAll also calls Set-GitHubRepository as a safety net, +# so this list is an optimization rather than a hard dependency. BeforeAll.ps1 and +# AfterAll.ps1 both source this file so setup and teardown always operate on the same set. +@{ + # Test files that each need a primary repository. + TestNames = @('Environments', 'Secrets', 'Variables', 'Releases', 'Actions') + + # Subset that also need companion -2/-3 repositories for org-scoped SelectedRepository tests. + TestNamesWithExtraRepos = @('Secrets', 'Variables') +} diff --git a/tests/Environments.Tests.ps1 b/tests/Environments.Tests.ps1 index 8735581ec..8aa9d2ca6 100644 --- a/tests/Environments.Tests.ps1 +++ b/tests/Environments.Tests.ps1 @@ -43,7 +43,7 @@ Describe 'Environments' { Write-Host ($context | Format-List | Out-String) } } - $repoPrefix = "Test-$os-$TokenType" + $repoPrefix = "$testName-$os-$TokenType" $repoName = "$repoPrefix-$id" $environmentName = "$testName-$os-$TokenType-$id" diff --git a/tests/Organizations.Tests.ps1 b/tests/Organizations.Tests.ps1 index 94b884ab6..7c71d318a 100644 --- a/tests/Organizations.Tests.ps1 +++ b/tests/Organizations.Tests.ps1 @@ -26,6 +26,13 @@ BeforeAll { if (-not $id) { throw 'GITHUB_RUN_ID is required to safely scope pre-test cleanup in Organizations.Tests.ps1.' } + # GITHUB_RUN_ATTEMPT increments on each rerun (1, 2, 3...). Enterprise org names go on a + # 90-day hold after deletion, so a rerun of the same GITHUB_RUN_ID would collide if we used + # the run ID alone. Appending the attempt number makes each attempt produce a unique org name. + $attempt = $env:GITHUB_RUN_ATTEMPT + if ($attempt -and $attempt -ne '1') { + $id = "$id-$attempt" + } } Describe 'Organizations' { @@ -41,15 +48,56 @@ Describe 'Organizations' { $orgName = "$orgPrefix$id" if ($AuthType -eq 'APP') { - LogGroup 'Pre-test Cleanup - App Installations' { - Get-GitHubAppInstallation -Context $context | Where-Object { $_.Target.Name -like "$orgName*" } | - Uninstall-GitHubApp -Confirm:$false - } - $installationContext = Connect-GitHubApp @connectAppParams -PassThru -Default -Silent LogGroup 'Context - Installation' { Write-Host ($installationContext | Select-Object * | Out-String) } + + if ($OwnerType -eq 'enterprise') { + # Clean up a stale enterprise org from a previous run attempt with the same + # GITHUB_RUN_ID. DELETE /orgs/{org} requires org-level administration:write, + # so we install the app first to obtain an org-level IAT, then delete. + LogGroup 'Pre-test Cleanup - Stale Enterprise Organization' { + $staleOrg = Get-GitHubOrganization -Name $orgName -ErrorAction SilentlyContinue + if ($staleOrg -and $staleOrg.Name) { + Write-Host "Stale org [$orgName] found from previous run attempt. Removing..." + try { + # Retry Install-GitHubApp: the enterprise apps endpoint can return 404 + # for a short time after the org was originally created. + $maxAttempts = 5 + $retryDelay = 3 + for ($retryAttempt = 1; $retryAttempt -le $maxAttempts; $retryAttempt++) { + try { + $null = Install-GitHubApp -Enterprise $owner -Organization $orgName ` + -ClientID $installationContext.ClientID -RepositorySelection 'all' -ErrorAction Stop + break + } catch { + if ($retryAttempt -lt $maxAttempts) { + Write-Host "Install-GitHubApp attempt $retryAttempt/$maxAttempts failed: $($_.Exception.Message). Retrying in ${retryDelay}s..." + Start-Sleep -Seconds $retryDelay + } else { + throw + } + } + } + $cleanupOrgContext = Connect-GitHubApp -Organization $orgName -Context $context -PassThru -Silent + Remove-GitHubOrganization -Name $orgName -Confirm:$false -Context $cleanupOrgContext + Write-Host "Stale org [$orgName] removed." + } catch { + # Rethrow — if the org exists but we can't remove it, New-GitHubOrganization + # will fail anyway. Failing here gives a clearer root-cause message. + throw "Could not remove stale org [$orgName]: $($_.Exception.Message)" + } + } else { + Write-Host "No stale org found for [$orgName]." + } + } + } + + LogGroup 'Pre-test Cleanup - App Installations' { + Get-GitHubAppInstallation -Context $context | Where-Object { $_.Target.Name -like "$orgName*" } | + Uninstall-GitHubApp -Confirm:$false + } } } @@ -128,10 +176,13 @@ Describe 'Organizations' { Owner = 'MariusStorhaug' BillingEmail = 'post@msx.no' } + $org = New-GitHubOrganization @orgParam LogGroup 'Organization' { - $org = New-GitHubOrganization @orgParam Write-Host ($org | Select-Object * | Out-String) } + $org | Should -Not -BeNullOrEmpty + $org | Should -BeOfType 'GitHubOrganization' + $org.Name | Should -Be $orgName } It 'Update-GitHubOrganization - Updates the organization location using enterprise installation' -Skip:($OwnerType -ne 'enterprise') { @@ -143,7 +194,25 @@ Describe 'Organizations' { } It 'Install-GitHubApp - Installs a GitHub App to an organization' -Skip:($OwnerType -ne 'enterprise') { - $installation = Install-GitHubApp -Enterprise $owner -Organization $orgName -ClientID $installationContext.ClientID -RepositorySelection 'all' + # Retry: the enterprise apps endpoint can return 404 transiently right after + # New-GitHubOrganization, before the new org has propagated. + $maxAttempts = 5 + $retryDelay = 3 + $installation = $null + for ($retryAttempt = 1; $retryAttempt -le $maxAttempts; $retryAttempt++) { + try { + $installation = Install-GitHubApp -Enterprise $owner -Organization $orgName ` + -ClientID $installationContext.ClientID -RepositorySelection 'all' -ErrorAction Stop + break + } catch { + if ($retryAttempt -lt $maxAttempts) { + Write-Host "Install-GitHubApp attempt $retryAttempt/$maxAttempts failed: $($_.Exception.Message). Retrying in ${retryDelay}s..." + Start-Sleep -Seconds $retryDelay + } else { + throw + } + } + } LogGroup 'Installed App' { Write-Host ($installation | Select-Object * | Out-String) } diff --git a/tests/Releases.Tests.ps1 b/tests/Releases.Tests.ps1 index 38cf8cbd1..fad24e4b3 100644 --- a/tests/Releases.Tests.ps1 +++ b/tests/Releases.Tests.ps1 @@ -43,7 +43,7 @@ Describe 'Releases' { Write-Host ($context | Format-Table | Out-String) } } - $repoPrefix = "Test-$os-$TokenType" + $repoPrefix = "$testName-$os-$TokenType" $repoName = "$repoPrefix-$id" LogGroup "Using Repository - [$repoName]" { @@ -63,6 +63,22 @@ Describe 'Releases' { } Write-Host ($repo | Select-Object * | Out-String) } + + # Clean up stale releases from prior runs with the same GITHUB_RUN_ID. + # Idempotent setup must not assume a clean repository — partial reruns can leave + # tags like v1.0/v1.1/v1.3 behind, which would cause New-GitHubRelease to fail + # with 422 (already_exists). + if ($repo) { + LogGroup "Pre-test Cleanup - Existing Releases on [$repoName]" { + $existingReleases = Get-GitHubRelease -Owner $Owner -Repository $repoName -AllVersions -ErrorAction SilentlyContinue + if ($existingReleases) { + Write-Host ($existingReleases | Format-Table | Out-String) + $existingReleases | Remove-GitHubRelease -Confirm:$false + } else { + Write-Host 'No existing releases to clean up.' + } + } + } } AfterAll { diff --git a/tests/Secrets.Tests.ps1 b/tests/Secrets.Tests.ps1 index fee918fd2..6acd3fff3 100644 --- a/tests/Secrets.Tests.ps1 +++ b/tests/Secrets.Tests.ps1 @@ -43,7 +43,7 @@ Describe 'Secrets' { Write-Host ($context | Format-List | Out-String) } } - $repoPrefix = "Test-$os-$TokenType" + $repoPrefix = "$testName-$os-$TokenType" $repoName = "$repoPrefix-$id" $secretPrefix = "$testName`_$os`_$TokenType" $secretName = "$secretPrefix`_$id" @@ -94,10 +94,18 @@ Describe 'Secrets' { LogGroup 'Secrets to remove' { $orgSecrets = Get-GitHubSecret -Owner $owner | Where-Object { $_.Name -like "$secretName*" } Write-Host "$($orgSecrets | Format-List | Out-String)" - $orgSecrets | Remove-GitHubSecret + $orgSecrets | Remove-GitHubSecret -Confirm:$false } } } + # Remove the test environment created on the per-test-file repository so it does + # not leak into other test files or subsequent reruns. + if ($OwnerType -notin ('repository', 'enterprise') -and $repo) { + LogGroup "Environment cleanup - [$environmentName] on [$repoName]" { + Get-GitHubEnvironment -Owner $owner -Repository $repoName -Name $environmentName -ErrorAction SilentlyContinue | + Remove-GitHubEnvironment -Confirm:$false + } + } Get-GitHubContext -ListAvailable | Disconnect-GitHubAccount -Silent Write-Host ('-' * 60) } diff --git a/tests/TEMPLATE.ps1 b/tests/TEMPLATE.ps1 index afc9a6114..6f52290c1 100644 --- a/tests/TEMPLATE.ps1 +++ b/tests/TEMPLATE.ps1 @@ -41,8 +41,8 @@ Describe 'Template' { } } - # Ensure the shared test repository exists. Set-GitHubRepository is idempotent. - $repoPrefix = "Test-$os-$TokenType" + # Ensure this test file's repository exists. Set-GitHubRepository is idempotent. + $repoPrefix = "$testName-$os-$TokenType" $repoName = "$repoPrefix-$id" if ($OwnerType -in ('repository', 'enterprise')) { $repo = $null diff --git a/tests/Variables.Tests.ps1 b/tests/Variables.Tests.ps1 index 904320d51..ae95c05cd 100644 --- a/tests/Variables.Tests.ps1 +++ b/tests/Variables.Tests.ps1 @@ -43,7 +43,7 @@ Describe 'Variables' { Write-Host ($context | Format-List | Out-String) } } - $repoPrefix = "Test-$os-$TokenType" + $repoPrefix = "$testName-$os-$TokenType" $repoName = "$repoPrefix-$id" $variablePrefix = "$testName`_$os`_$TokenType" $variableName = "$variablePrefix`_$id" @@ -94,7 +94,15 @@ Describe 'Variables' { LogGroup 'Variables to remove' { Write-Host "$($variablesToRemove | Format-List | Out-String)" } - $variablesToRemove | Remove-GitHubVariable + $variablesToRemove | Remove-GitHubVariable -Confirm:$false + } + } + # Remove the test environment created on the per-test-file repository so it does + # not leak into other test files or subsequent reruns. + if ($OwnerType -notin ('repository', 'enterprise') -and $repo) { + LogGroup "Environment cleanup - [$environmentName] on [$repoName]" { + Get-GitHubEnvironment -Owner $owner -Repository $repoName -Name $environmentName -ErrorAction SilentlyContinue | + Remove-GitHubEnvironment -Confirm:$false } } Get-GitHubContext -ListAvailable | Disconnect-GitHubAccount -Silent