diff --git a/.github/actions/ci-serialisation/action.yml b/.github/actions/ci-serialisation/action.yml index a21c23c..e3036c8 100644 --- a/.github/actions/ci-serialisation/action.yml +++ b/.github/actions/ci-serialisation/action.yml @@ -198,6 +198,13 @@ runs: git checkout ${{ inputs.base_ref }} Remove-Item "C:\ProgramData\BHoM\Assemblies\*" -Recurse -Force -ErrorAction SilentlyContinue + # prefer_branch is what makes this a baseline. Without it the head branch is preferred on + # both legs, because resolve-dependencies takes PR_BRANCH from the event payload and that + # does not vary within a job. Every dependency carrying a branch of the same name as the + # pull request was therefore resolved to it twice, so the two legs shared dependency code + # and a regression on the dependency side cancelled out. Passing the base branch here, and + # re-resolving already-present clones in the resolver, are both required: either alone + # leaves the baseline on the branch leg's refs. - name: Resolve dependencies (baseline) if: steps.changed.outputs.count != '0' && steps.branch_run.outputs.status == 'Error' uses: BHoM/CI_Toolkit/.github/actions/resolve-dependencies@develop @@ -206,6 +213,7 @@ runs: dotnet_version: ${{ inputs.dotnet_version }} configuration: Release token: ${{ steps.dep-token.outputs.token }} + prefer_branch: ${{ inputs.base_ref }} additional_seeds: | ${{ inputs.test_toolkit_repo }} ${{ inputs.serialisation_engine_repo }} diff --git a/.github/actions/resolve-dependencies/action.yml b/.github/actions/resolve-dependencies/action.yml index 4fea0d5..f7366a1 100644 --- a/.github/actions/resolve-dependencies/action.yml +++ b/.github/actions/resolve-dependencies/action.yml @@ -53,6 +53,21 @@ inputs: required: false default: "" + prefer_branch: + description: | + Branch to try first on every dependency, overriding the pull request's head branch. + + Empty (the default) keeps the existing behaviour: prefer the PR head branch, which is + what a branch-leg build wants and is how cross-repo pull request pairs resolve. + + Set this when a job resolves dependencies more than once and the passes mean different + things. ci-serialisation's baseline leg passes the base branch, because without it the + head branch is preferred on both legs: PR_BRANCH comes from the event payload, which + does not vary within a job, so the baseline leg had no way to ask for anything else. + An explicit owner/repo@ref in dependencies.txt still wins over this. + required: false + default: "" + runs: using: "composite" @@ -129,8 +144,17 @@ runs: - name: Resolve dependency graph shell: pwsh env: - PR_BRANCH: ${{ github.event.pull_request.head.ref }} + # prefer_branch wins when supplied. The event payload is the default because it is + # right for every caller that resolves once; it is not overridable from inside the + # script, which is why the baseline leg needed an input rather than a code change. + PR_BRANCH: ${{ inputs.prefer_branch || github.event.pull_request.head.ref }} BASE_BRANCH: ${{ github.event.pull_request.base.ref }} + # Whether PR_BRANCH came from the input or from the event. The script cannot tell + # from the value alone, and the two mean different things on a repeat invocation: + # an explicit request that changed nothing is a misconfiguration, an inherited + # default that changed nothing is the ordinary case. See the no-op check at the end + # of Resolve-DependencyGraph.ps1. + PREFER_BRANCH_EXPLICIT: ${{ inputs.prefer_branch != '' }} DEP_TOKEN: ${{ inputs.token }} run: | $script = Join-Path $env:GITHUB_ACTION_PATH "scripts/Resolve-DependencyGraph.ps1" diff --git a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 index cb5399c..b6623c0 100644 --- a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 +++ b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 @@ -16,6 +16,13 @@ $shaFile = Join-Path $depsDir "_shas.txt" $orderOut = Join-Path $depsDir "_order.txt" $selectFile = Join-Path $depsDir "_selection.txt" +# Which ref each clone was last checked out to, kept beside the clones rather than in deps/ +# because the action truncates _shas.txt and deletes _selection.txt on every invocation while +# the clone root persists for the whole job. A flat file, not a directory: the junction loop at +# the end of the calling action enumerates directories under the clone root and would otherwise +# link this into the workspace parent as if it were a dependency. +$refMarkerFile = Join-Path $cloneRoot "_selected-refs.txt" + New-Item -ItemType Directory -Force -Path $cloneRoot | Out-Null if (Test-Path $selectFile) { Remove-Item $selectFile -Force } @@ -63,6 +70,9 @@ $nameMap = @{} # owner/repo -> folder $pathMap = @{} # owner/repo -> path $folderUsed = @{} # folder name -> owner/repo, for collision detection +# What each dependency resolved to, and what it was on before, for the no-op warning at the end. +$resolutionLog = New-Object System.Collections.Generic.List[hashtable] + # Use insteadOf to inject the token at the git config level — keeps it out of command # arguments, process listings, and cloned repos' .git/config. Removed in a finally block # to avoid persisting the credential on self-hosted runners even when the script errors. @@ -83,91 +93,133 @@ function Get-FolderName([string]$ownerRepo) { return $name } +# A detached HEAD cannot report the branch it came from, so the ref each clone was checked out +# to is recorded here and read back on a later invocation. Without it a reused clone can only be +# described by its SHA, which does not tell a reader whether the ref was the one this leg asked +# for. See the reporting note on the reuse path in Clone-And-Checkout. +function Set-SelectedRefMarker([string]$name, [string]$ref) { + $kept = @() + if (Test-Path $refMarkerFile) { + $kept = @(Get-Content $refMarkerFile | + Where-Object { $_ -notmatch "^$([regex]::Escape($name))\|" }) + } + ($kept + "$name|$ref") | Set-Content -Path $refMarkerFile -Encoding utf8 +} + +function Get-SelectedRefMarker([string]$name) { + if (-not (Test-Path $refMarkerFile)) { return $null } + $line = Get-Content $refMarkerFile | + Where-Object { $_ -match "^$([regex]::Escape($name))\|" } | + Select-Object -First 1 + if (-not $line) { return $null } + return $line.Split('|', 2)[1] +} + function Clone-And-Checkout([string]$ownerRepo, [string]$ref) { $name = Get-FolderName $ownerRepo $path = Join-Path $cloneRoot $name - if (-not (Test-Path (Join-Path $path ".git"))) { - + # Clone if absent, then resolve the ref UNCONDITIONALLY. + # + # These two used to be one conditional: a clone that already existed was left on whatever + # ref a previous invocation put it on, and only its SHA was re-recorded. ci-serialisation + # invokes this action twice in one job, so its baseline leg inherited the branch leg's + # dependency refs, built the base-branch subject against branch-branch dependencies, and + # compared two legs that shared the same dependency code. A regression introduced on the + # dependency side appeared in both and cancelled out. + # + # This is BHoMBot's shape, deliberately. LoadDependencies (CodeBuild_Engine) cloned if + # absent and then always called CheckoutBranch with the branch as an explicit argument, and + # ResetBuiltRepos() cleared the memo that would otherwise have skipped the re-checkout. The + # divergence from BHoMBot is that the memo has no equivalent here and needs none: there is + # no build-dedup cache in this script, so nothing suppresses the second resolution. + # + # Cost is one shallow fetch per dependency per additional invocation, which only + # ci-serialisation pays. Not optimised away by comparing the remote SHA first: that adds a + # branch to reason about in exchange for saving a fetch nothing is waiting on. + $freshClone = -not (Test-Path (Join-Path $path ".git")) + if ($freshClone) { git clone "https://github.com/$ownerRepo.git" $path --no-tags --depth 1 | Out-Null + } - $selectedRef = $null - Push-Location $path - try { - $used = $false - - if ($ref) { - $hasHead = git ls-remote --heads origin $ref - $hasTag = git ls-remote --tags origin $ref - if ($hasHead -or $hasTag) { - git fetch origin $ref --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$ref)" } - $selectedRef = $ref - $used = $true - } else { - Write-Warning "Explicit ref '$ref' not found on '$ownerRepo' — falling back." - } + $previousRef = if ($freshClone) { $null } else { Get-SelectedRefMarker $name } + + $selectedRef = $null + Push-Location $path + try { + $used = $false + + if ($ref) { + $hasHead = git ls-remote --heads origin $ref + $hasTag = git ls-remote --tags origin $ref + if ($hasHead -or $hasTag) { + git fetch origin $ref --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$ref)" } + $selectedRef = $ref + $used = $true + } else { + Write-Warning "Explicit ref '$ref' not found on '$ownerRepo' — falling back." } + } - if (-not $used) { - $hasPrefer = if ($Prefer) { git ls-remote --heads origin $Prefer } else { $null } - $hasFallback = git ls-remote --heads origin $Fallback - if ($hasPrefer) { - git fetch origin $Prefer --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Prefer)" } - $selectedRef = $Prefer - } elseif ($hasFallback) { - git fetch origin $Fallback --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Fallback)" } - $selectedRef = $Fallback - } else { - # Neither PR nor base branch exists on this dep; fall back to remote default. - git fetch origin HEAD --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (remote default)" } - $defaultRef = (git ls-remote --symref origin HEAD | - Select-String 'ref: refs/heads/(\S+)\s+HEAD' | - ForEach-Object { $_.Matches[0].Groups[1].Value } | - Select-Object -First 1) - $selectedRef = if ($defaultRef) { $defaultRef } else { "(remote default)" } - } + if (-not $used) { + $hasPrefer = if ($Prefer) { git ls-remote --heads origin $Prefer } else { $null } + $hasFallback = git ls-remote --heads origin $Fallback + if ($hasPrefer) { + git fetch origin $Prefer --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Prefer)" } + $selectedRef = $Prefer + } elseif ($hasFallback) { + git fetch origin $Fallback --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Fallback)" } + $selectedRef = $Fallback + } else { + # Neither PR nor base branch exists on this dep; fall back to remote default. + git fetch origin HEAD --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (remote default)" } + $defaultRef = (git ls-remote --symref origin HEAD | + Select-String 'ref: refs/heads/(\S+)\s+HEAD' | + ForEach-Object { $_.Matches[0].Groups[1].Value } | + Select-Object -First 1) + $selectedRef = if ($defaultRef) { $defaultRef } else { "(remote default)" } } - - $sha = (git rev-parse HEAD).Trim() - Add-Content -Path $shaFile -Value "$ownerRepo $sha" - Add-Content -Path $selectFile -Value "$ownerRepo|$name|$selectedRef|$sha" - } - finally { - Pop-Location } - # Write-Host, not ::notice. A ::notice with no file= becomes a check-run - # annotation attributed to ".github" at the log line number, and there is one - # per dependency: nine on a typical ci-build run, competing with the caller's - # own diagnostics for GitHub's per-step annotation cap. The same information - # already reaches the job summary as a table at the end of this script, which - # is where a reader looks for it. - Write-Host "Dependency checkout: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7))" + # _shas.txt is reset between invocations, so this must be written every time, not only + # on a fresh clone. Without it the cache-key computation produces an empty keypart, + # skipping both the assembly cache restore and the dep build, leaving + # C:\ProgramData\BHoM\Assemblies empty. + $sha = (git rev-parse HEAD).Trim() + Add-Content -Path $shaFile -Value "$ownerRepo $sha" + Add-Content -Path $selectFile -Value "$ownerRepo|$name|$selectedRef|$sha" + Set-SelectedRefMarker $name $selectedRef } - else { - # Repo already cloned (e.g. baseline run re-uses branch-build clones). - # _shas.txt is reset between invocations, so write the SHA even though we skip re-cloning. - # Without this the cache-key computation produces an empty keypart, skipping both the - # assembly cache restore and the dep build, leaving C:\ProgramData\BHoM\Assemblies empty. - Push-Location $path - try { - $sha = (git rev-parse HEAD).Trim() - Add-Content -Path $shaFile -Value "$ownerRepo $sha" - } - finally { - Pop-Location - } + finally { + Pop-Location + } + + # Write-Host, not ::notice. A ::notice with no file= becomes a check-run + # annotation attributed to ".github" at the log line number, and there is one + # per dependency: nine on a typical ci-build run, competing with the caller's + # own diagnostics for GitHub's per-step annotation cap. The same information + # already reaches the job summary as a table at the end of this script, which + # is where a reader looks for it. + # + # A re-resolution that moved the clone names both refs, because "which ref is this leg + # building" is the question ci-serialisation's baseline leg could not previously answer. + if ($previousRef -and $previousRef -ne $selectedRef) { + Write-Host "Dependency re-resolved: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7)) (was $previousRef)" + } else { + Write-Host "Dependency checkout: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7))" } + $resolutionLog.Add(@{ Key = $ownerRepo; Previous = $previousRef; Selected = $selectedRef }) | Out-Null + $nameMap[$ownerRepo] = $name $pathMap[$ownerRepo] = $path @@ -326,6 +378,51 @@ if (Test-Path $selectFile) { } } +# A repeat invocation that changed nothing is the shape of the defect this reporting exists for: +# two passes meant to mean different things resolving to the same dependency code, so a regression +# on the dependency side appears in both and cancels out when they are compared. Only reachable on +# a repeat invocation, because a first pass has nothing to compare against. +# +# The annotation level depends on WHO asked, which is why PREFER_BRANCH_EXPLICIT exists. +# +# explicit prefer_branch + nothing moved -> ::warning. A caller deliberately asked for a +# different ref and got the same one, so either the branch does not exist anywhere in the +# closure or the value is wrong. This is the misconfiguration worth interrupting for. +# something moved -> ::notice naming the moves. The normal baseline. +# inherited default + nothing moved -> plain log line, no annotation. This is the +# ordinary case: the pull request branch exists on no dependency, so both passes +# legitimately resolve to the base branch. An earlier version warned here and would have +# fired on most baseline runs, which is worse than not warning at all. +# +# Never an error. Failing on same-refs would red every pull request whose branch name exists on +# no dependency, which is most of them. +$repeated = @($resolutionLog | Where-Object { $_.Previous }) +if ($repeated.Count -gt 0) { + $moved = @($repeated | Where-Object { $_.Previous -ne $_.Selected }) + $wasExplicit = $env:PREFER_BRANCH_EXPLICIT -eq 'true' + + if ($moved.Count -gt 0) { + Write-Host ("::notice title=resolve-dependencies::Repeat resolution moved " + + "$($moved.Count) of $($repeated.Count) already-present dependenc(ies): " + + (($moved | ForEach-Object { "$($_.Key) $($_.Previous)->$($_.Selected)" }) -join ', ')) + } + elseif ($wasExplicit) { + Write-Host ("::warning title=resolve-dependencies::prefer_branch was set explicitly to " + + "'$Prefer' but no dependency moved: all $($repeated.Count) already-present " + + "dependenc(ies) stayed on the ref the previous invocation selected. Either that " + + "branch exists on none of them, which is expected and harmless, or the value is " + + "wrong. If it is wrong, this pass is building against the same dependency code " + + "as the previous one, so any comparison between the two passes cannot see a " + + "regression that came from a dependency: it is present on both sides and " + + "cancels out.") + } + else { + Write-Host ("Repeat resolution changed nothing: all $($repeated.Count) already-present " + + "dependenc(ies) stayed on '$Prefer', which was inherited from the event rather " + + "than requested. Ordinary for a pull request whose branch exists on no dependency.") + } +} + } finally { # Remove the credential rewrite — on self-hosted runners ~/.gitconfig persists between # jobs and would leak the short-lived token to subsequent jobs. The finally block ensures diff --git a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 new file mode 100644 index 0000000..de74724 --- /dev/null +++ b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 @@ -0,0 +1,398 @@ +# Resolve-DependencyGraph.BaselineReuse.Tests.ps1 +# +# ci-serialisation invokes resolve-dependencies twice in one job: once for the pull request +# branch and once for the base it is compared against. Before this change the second invocation +# reused the first invocation's clones without re-resolving them, so both legs were built against +# the BRANCH's dependency code. A regression introduced on the dependency side then appeared in +# both legs, compared equal, and the check passed. +# +# These began as a demonstration of that defect and are now the regression suite for its fix. +# The assertions under "re-resolving an existing clone" and "reporting" originally asserted the +# broken behaviour and have been inverted. The three preceding Contexts were controls and are +# unchanged, which is what makes them useful: they show the fix did not alter resolution for +# callers that invoke the action once, which is every check except ci-serialisation. +# +# This change does not make ci-serialisation a required check. +# +# Why a hermetic test rather than a live pair of repositories. Reproducing this end to end needs +# a subject repo whose dependencies.txt names a dependency in the same organisation, plus a +# branch of the same name on both. No such pair exists that is free to experiment on, and making +# one would mean pushing branches to repositories that are in use. A local bare repo reaches the +# same code path with no network. +# +# What it establishes, in order: +# 1. On a fresh clone root, the resolver honours PR_BRANCH when that branch exists on the +# dependency. That is the intended cross-repo feature and the fix must not disturb it. +# 2. Two invocations that both ask for the same ref agree: the checkout stays where it is and +# the recorded SHA does not change. A control on the always-re-resolve change, since an +# unstable repeat resolution would show up here first. +# 3. The resolver can land on the base branch when asked to, so the defect was never that it +# could not. PR_BRANCH was taken from github.event.pull_request.head.ref in +# resolve-dependencies/action.yml, which is constant for a whole job and had no +# per-invocation override, so the baseline leg had no way to ask for anything else. +# 4. An existing clone is re-resolved, so an explicit base-branch request is honoured, the +# working tree moves and not just the recorded SHA, and the two legs produce different +# cache-key inputs. These assertions originally read the other way round, which is what +# showed both causes had to be addressed together: adding the input alone would have been +# ignored by the shortcut that skipped resolution on an already-present clone. +# 5. The selection table and the ref marker are written on every invocation, not only when a +# clone is created, and a repeat resolution is annotated according to who asked for the +# ref it settled on. +# +# Run locally: pwsh -Command "Invoke-Pester .github/scripts/tests -Output Detailed" +# Run in CI: lint-workflows.yml, the powershell-tests job. + +BeforeAll { + $script:repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../../..')).Path + $script:resolver = Join-Path $repoRoot '.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1' + $script:sandbox = Join-Path ([IO.Path]::GetTempPath()) ("depgraph-" + [Guid]::NewGuid().ToString('N')) + + # The resolver hard-codes https://github.com//.git, so the only way to point + # it at a local fixture is git's insteadOf rewrite. Same mechanism the resolver itself + # uses for tokens. Removed in AfterAll; nothing else in the powershell-tests job clones + # from github.com after this file runs. + $script:remoteRoot = Join-Path $sandbox 'remotes' + New-Item -ItemType Directory -Force -Path $remoteRoot | Out-Null + $script:insteadOfKey = 'url.file:///' + ($remoteRoot -replace '\\', '/') + '/.insteadOf' + git config --global $insteadOfKey 'https://github.com/' + + # The resolver appends a markdown table to the step summary when it clones. Redirect it so + # a test run does not write into the real job summary. + $script:savedSummary = $env:GITHUB_STEP_SUMMARY + $env:GITHUB_STEP_SUMMARY = Join-Path $sandbox 'summary.md' + + function New-FixtureRemote { + # A bare repo with two branches whose tip contents differ, standing in for a BHoM + # dependency that carries a change on a feature branch. + param([string]$OwnerRepo, [string]$Prefer) + + $work = Join-Path $sandbox ('work-' + ($OwnerRepo -replace '/', '-')) + $bare = Join-Path $remoteRoot "$OwnerRepo.git" + New-Item -ItemType Directory -Force -Path (Split-Path $bare) | Out-Null + + git init -q --bare $bare + git init -q $work + Push-Location $work + try { + git config user.email 't@t'; git config user.name 't' + git symbolic-ref HEAD refs/heads/develop + + Set-Content -Path 'Value.cs' -Value '// base' -Encoding utf8 + git add -A; git commit -q -m 'base' + $baseSha = (git rev-parse HEAD).Trim() + + git checkout -q -b $Prefer + Set-Content -Path 'Value.cs' -Value '// branch change, serialisation-affecting' -Encoding utf8 + git add -A; git commit -q -m 'branch' + $branchSha = (git rev-parse HEAD).Trim() + + git remote add origin $bare + git push -q origin develop $Prefer + # HEAD on the bare repo decides the remote-default fallback. + git --git-dir=$bare symbolic-ref HEAD refs/heads/develop + } + finally { Pop-Location } + + return @{ Base = $baseSha; Branch = $branchSha } + } + + function Invoke-Resolver { + # Reproduces exactly what resolve-dependencies/action.yml does around the script: + # create deps/, truncate _shas.txt (New-Item -ItemType File -Force at :83), set + # PR_BRANCH and BASE_BRANCH from the event, then invoke from the workspace root. + param( + [string]$Workspace, + [string]$CloneRoot, + [AllowNull()][string]$Prefer, + [string]$Fallback, + # Return the script's host output instead of discarding it, for the tests that + # assert on the workflow-command lines it emits. + [switch]$Capture, + # Mirrors resolve-dependencies/action.yml's PREFER_BRANCH_EXPLICIT: whether + # PR_BRANCH came from the prefer_branch input or from the event payload. + [switch]$Explicit + ) + + New-Item -ItemType Directory -Force -Path (Join-Path $Workspace 'deps') | Out-Null + New-Item -ItemType File -Force -Path (Join-Path $Workspace 'deps/_shas.txt') | Out-Null + + $env:PR_BRANCH = $Prefer + $env:BASE_BRANCH = $Fallback + $env:DEP_TOKEN = '' + $env:PREFER_BRANCH_EXPLICIT = if ($Explicit) { 'true' } else { 'false' } + + Push-Location $Workspace + try { + if ($Capture) { + return @(& $resolver -DepsFile 'dependencies.txt' -Mode 'caller' -Seeds '' ` + -AdditionalSeeds '' -CloneRoot $CloneRoot 6>&1) + } + & $resolver -DepsFile 'dependencies.txt' -Mode 'caller' -Seeds '' ` + -AdditionalSeeds '' -CloneRoot $CloneRoot | Out-Null + } + finally { Pop-Location } + } + + function Get-CheckedOutSha { + param([string]$CloneRoot, [string]$Name) + Push-Location (Join-Path $CloneRoot $Name) + try { return (git rev-parse HEAD).Trim() } finally { Pop-Location } + } + + function Get-RecordedSha { + param([string]$Workspace, [string]$OwnerRepo) + $line = Get-Content (Join-Path $Workspace 'deps/_shas.txt') | + Where-Object { $_ -like "$OwnerRepo *" } | Select-Object -First 1 + if (-not $line) { return $null } + return $line.Split(' ')[1] + } + + function New-Workspace { + param([string]$Name, [string]$Dependency) + $ws = Join-Path $sandbox $Name + New-Item -ItemType Directory -Force -Path $ws | Out-Null + Set-Content -Path (Join-Path $ws 'dependencies.txt') -Value $Dependency -Encoding utf8 + return $ws + } +} + +AfterAll { + git config --global --unset $insteadOfKey 2>$null + $env:GITHUB_STEP_SUMMARY = $savedSummary + Remove-Item $sandbox -Recurse -Force -ErrorAction SilentlyContinue +} + +Describe 'resolve-dependencies: ref selection across repeat invocations in one job' { + + BeforeAll { + $script:dep = 'Fake/Dep' + $script:prefer = 'feature/dependency-side-change' + $script:shas = New-FixtureRemote -OwnerRepo $dep -Prefer $prefer + } + + Context 'the preferred branch is honoured when the dependency carries it' { + + It 'checks the dependency out at the branch tip, not the base tip' { + $ws = New-Workspace -Name 'prefer-ws' -Dependency $dep + $root = Join-Path $sandbox 'prefer-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch + Get-RecordedSha -Workspace $ws -OwnerRepo $dep | Should -Be $shas.Branch + } + } + + Context 'a second invocation asking for the same ref is stable' { + + # Control, not a defect. Two invocations that both want the branch must agree. This + # guards the ordinary single-meaning case against the always-re-resolve change: if the + # fix had made repeat resolution unstable, this is what would catch it. + It 'stays on the branch tip when both invocations prefer the branch' { + $ws = New-Workspace -Name 'same-ref-ws' -Dependency $dep + $root = Join-Path $sandbox 'same-ref-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $afterFirst = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $afterSecond = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' + + $afterFirst | Should -Be $shas.Branch + $afterSecond | Should -Be $shas.Branch + $afterSecond | Should -Be $afterFirst + } + + It 'records the same SHA when both invocations prefer the same ref' { + $ws = New-Workspace -Name 'same-ref-sha-ws' -Dependency $dep + $root = Join-Path $sandbox 'same-ref-sha-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $first = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + $second | Should -Be $first + $second | Should -Be $shas.Branch + } + } + + Context 'the resolver can reach the base branch, but was never asked to' { + + It 'lands on the base tip when PR_BRANCH does not exist on the dependency' { + $ws = New-Workspace -Name 'base-reach-ws' -Dependency $dep + $root = Join-Path $sandbox 'base-reach-clones' + + # The common case: the PR branch name exists only on the subject repo, so + # $Prefer misses and $Fallback wins. This is why the defect was never observed + # firing: it needs a cross-repo branch pair, and most PRs are not one. + Invoke-Resolver -Workspace $ws -CloneRoot $root ` + -Prefer 'branch/that/exists/nowhere' -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Base + } + + It 'lands on the base tip on a fresh root when asked for the base explicitly' { + $ws = New-Workspace -Name 'base-explicit-ws' -Dependency $dep + $root = Join-Path $sandbox 'base-explicit-clones' + + # Proves the capability exists. resolve-dependencies simply exposes no input + # that would let ci-serialisation's baseline leg request it. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Base + } + } + + Context 're-resolving an existing clone, which is the fix' { + + # This is the assertion the whole change exists for. Before the fix it read + # Should -Be $shas.Branch: the already-cloned path short-circuited before any ref + # resolution, so an explicit base-branch request was ignored and parameterising the + # branch alone would not have helped. Both halves are now in place, so the second + # invocation lands where it was asked to. + It 'honours an explicit base-branch request when the clone is already present' { + $ws = New-Workspace -Name 'reresolve-ws' -Dependency $dep + $root = Join-Path $sandbox 'reresolve-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch + + # What ci-serialisation's baseline leg now does, via prefer_branch: base_ref. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Base + Get-RecordedSha -Workspace $ws -OwnerRepo $dep | Should -Be $shas.Base + } + + It 'moves the working tree, not just the recorded SHA' { + $ws = New-Workspace -Name 'reresolve-tree-ws' -Dependency $dep + $root = Join-Path $sandbox 'reresolve-tree-clones' + $file = Join-Path (Join-Path $root 'Dep') 'Value.cs' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + (Get-Content $file -Raw) | Should -Match 'branch change' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + # Bookkeeping alone would leave the branch's source on disk and the base's SHA in + # _shas.txt, which is worse than the original defect: the cache key would say + # baseline while the assemblies were still built from branch code. + (Get-Content $file -Raw) | Should -Match 'base' + (Get-Content $file -Raw) | Should -Not -Match 'branch change' + } + + It 'gives the two legs different cache-key inputs' { + $ws = New-Workspace -Name 'reresolve-key-ws' -Dependency $dep + $root = Join-Path $sandbox 'reresolve-key-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $branchLegSha = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + $baselineLegSha = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + # The depsasm- key is a SHA-256 over the sorted owner/repo@sha set + # (resolve-dependencies/action.yml). Differing recorded SHAs mean a differing key, + # so the baseline leg now misses the branch leg's assembly cache and builds its own + # closure. Identical SHAs across the legs were the observable signature of the + # defect: the key missed on the branch leg, which built and saved it, then hit on + # the baseline leg, which therefore restored the branch leg's assemblies rather + # than building its own. + $baselineLegSha | Should -Not -Be $branchLegSha + } + } + + Context 'reporting: the resolved ref is recorded on every invocation' { + + It 'writes a selection line on a reused clone, naming the ref it resolved to' { + $ws = New-Workspace -Name 'rep-ws' -Dependency $dep + $root = Join-Path $sandbox 'rep-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + # _selection.txt is deleted at the start of every invocation, so this content is + # the second invocation's alone. It was empty before the reporting change, which + # is why the baseline leg's job summary had no dependency table. + $selection = Get-Content (Join-Path $ws 'deps/_selection.txt') + $selection | Should -Not -BeNullOrEmpty + ($selection -join "`n") | Should -Match ([regex]::Escape("$dep|Dep|develop|")) + } + + It 'updates the marker so a later invocation reports the current ref' { + $ws = New-Workspace -Name 'rep2-ws' -Dependency $dep + $root = Join-Path $sandbox 'rep2-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $marker = Join-Path $root '_selected-refs.txt' + (Get-Content $marker -Raw) | Should -Match ([regex]::Escape("Dep|$prefer")) + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + (Get-Content $marker -Raw) | Should -Match ([regex]::Escape('Dep|develop')) + (Get-Content $marker -Raw) | Should -Not -Match ([regex]::Escape($prefer)) + } + + # The guard distinguishes who asked. An explicit prefer_branch that moved nothing is a + # misconfiguration worth annotating; the same outcome from an inherited event default is + # the ordinary case and must not annotate, because it happens on most baseline runs. + # Never an error either way. + It 'warns when prefer_branch was explicit and nothing moved' { + $ws = New-Workspace -Name 'guard-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Explicit -Capture + + ($second -join "`n") | Should -Match '::warning title=resolve-dependencies::prefer_branch was set explicitly' + ($second -join "`n") | Should -Match 'cancels out' + } + + It 'stays silent, with no annotation, when the default was inherited and nothing moved' { + $ws = New-Workspace -Name 'guard1b-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard1b-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Capture + + # The case that made the first version of this guard useless: it fired here, which is + # most baseline runs. Informational only now. + ($second -join "`n") | Should -Not -Match '::warning' + ($second -join "`n") | Should -Match 'inherited from the event rather than requested' + } + + It 'does not warn on a first invocation, which has nothing to compare against' { + $ws = New-Workspace -Name 'guard2-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard2-clones' + + $first = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Explicit -Capture + + ($first -join "`n") | Should -Not -Match '::warning' + ($first -join "`n") | Should -Not -Match 'Repeat resolution' + } + + It 'reports a notice, not a warning, when a repeat resolution did move' { + $ws = New-Workspace -Name 'guard3-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard3-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' -Explicit -Capture + + ($second -join "`n") | Should -Match 'Repeat resolution moved 1 of 1' + ($second -join "`n") | Should -Not -Match '::warning' + } + + It 'keeps the marker file out of the directories the caller junctions' { + $ws = New-Workspace -Name 'rep3-ws' -Dependency $dep + $root = Join-Path $sandbox 'rep3-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + + # The calling action junctions every DIRECTORY under the clone root into the + # workspace parent. A marker stored as a directory would be linked in as though it + # were a dependency. + Get-ChildItem $root -Directory | ForEach-Object { $_.Name } | Should -Be @('Dep') + } + } +}