Report every unresolvable SDK package, not just the first

docs/sdk-pinning.md

@@ -30,6 +30,12 @@ target for the rest of the repository rather than being rolled backwards.
 Edits are textual and scoped to the version that follows the package name, so key order,
 formatting, comments, unrelated entries and the trailing newline all survive.
 
+A package whose version cannot be resolved — a feed hiccup, a package that was never published,
+a name that has since been retired — fails the run. Every package is still looked at first, so
+one run names every unresolvable package rather than one per run, and the run fails before
+anything is written. Reporting the failures and converging the rest would leave the repository
+on two versions of the family, which is the state this exists to repair.
+
 ## What it does not do
 
 It does not open a pull request. It builds and tests the repository with the new pins and

scripts/tests/update-sdks.tests.ps1

@@ -6,8 +6,10 @@
 	Every case here is a defect observed in the previous workflow against the real feed,
 	not a hypothetical. Run with pwsh: ./scripts/tests/update-sdks.tests.ps1
 
-	-Version is used throughout so the cases do not depend on the network or on which
-	version happens to be current the day they run.
+	-Version is used for the rewriting cases so they do not depend on the network or on which
+	version happens to be current the day they run. The resolution cases deliberately omit it,
+	because that is the only path that calls the feed at all, and they point the script at a
+	closed port or at an in-process stub feed rather than at nuget.org.
 #>
 [CmdletBinding()]
 param()
@@ -60,6 +62,123 @@ function Test-Case {
 	}
 }
 
+# A flat container index, served in process. The feed is the only thing the resolution path
+# touches that the rewriting cases never reach, so it is the only thing that has to be faked.
+# Versions maps a lower-case package id -- the flat container addresses packages in lower case --
+# to the versions it publishes, and an id that is absent is answered with 404 so a lookup
+# failure and a package with no release can both be provoked in one run.
+function Start-StubFeed {
+	param([hashtable]$Versions)
+
+	$listener = [System.Net.HttpListener]::new()
+	$port = 0
+	foreach ($candidate in 18080..18179) {
+		try {
+			$listener.Prefixes.Clear()
+			$listener.Prefixes.Add("http://127.0.0.1:$candidate/")
+			$listener.Start()
+			$port = $candidate
+			break
+		}
+		catch {
+			# The port is taken. Try the next one.
+		}
+	}
+
+	if ($port -eq 0) {
+		throw 'No free port in 18080-18179 for the stub feed.'
+	}
+
+	$job = Start-ThreadJob -ScriptBlock {
+		param($Listener, $Map)
+
+		try {
+			while ($Listener.IsListening) {
+				$context = $Listener.GetContext()
+				# /<id>/index.json
+				$id = $context.Request.Url.Segments[1].TrimEnd('/')
+
+				if ($Map.ContainsKey($id)) {
+					$body = '{"versions":["' + ($Map[$id] -join '","') + '"]}'
+					$bytes = [System.Text.Encoding]::UTF8.GetBytes($body)
+					$context.Response.ContentType = 'application/json'
+					$context.Response.OutputStream.Write($bytes, 0, $bytes.Length)
+				}
+				else {
+					$context.Response.StatusCode = 404
+				}
+
+				$context.Response.Close()
+			}
+		}
+		catch {
+			# Stopping the listener is how this loop ends.
+		}
+	} -ArgumentList $listener, $Versions
+
+	return [pscustomobject]@{ Listener = $listener; Job = $job; Url = "http://127.0.0.1:$port" }
+}
+
+function Stop-StubFeed {
+	param($Feed)
+
+	$Feed.Listener.Stop()
+	$Feed.Listener.Close()
+	Remove-Job -Job $Feed.Job -Force
+}
+
+# The resolution path fails the run rather than rewriting anything, so these cases assert on the
+# message the run fails with and on the fixture being left alone, not on a rewritten file.
+function Test-ResolutionFailure {
+	param([string]$Name, [hashtable]$Files, [string]$FeedUrl, [string[]]$Reports, [hashtable]$Unchanged)
+
+	$script:Count++
+	$root = New-Fixture -Files $Files
+	try {
+		$message = $null
+		try {
+			& $script:Script -Path $root -FeedUrl $FeedUrl 6>$null 2>$null | Out-Null
+		}
+		catch {
+			$message = $_.Exception.Message
+		}
+
+		if ($null -eq $message) {
+			$script:Failures++
+			Write-Host "FAIL  $Name" -ForegroundColor Red
+			Write-Host '      the run was expected to fail, and did not.' -ForegroundColor Red
+			return
+		}
+
+		$missing = @($Reports | Where-Object { $message -notlike "*$_*" })
+		if ($missing.Count -gt 0) {
+			$script:Failures++
+			Write-Host "FAIL  $Name" -ForegroundColor Red
+			Write-Host "      the failure never named: $($missing -join ', ')" -ForegroundColor Red
+			Write-Host "      it said: $message" -ForegroundColor Red
+			return
+		}
+
+		if ($Unchanged) {
+			foreach ($file in $Unchanged.Keys) {
+				$actual = Get-Content -LiteralPath (Join-Path $root $file) -Raw
+				if ($actual -ne $Unchanged[$file]) {
+					$script:Failures++
+					Write-Host "FAIL  $Name" -ForegroundColor Red
+					Write-Host "      $file was written despite the failure:" -ForegroundColor Red
+					Write-Host "      $actual" -ForegroundColor Red
+					return
+				}
+			}
+		}
+
+		Write-Host "ok    $Name" -ForegroundColor Green
+	}
+	finally {
+		Remove-Item -LiteralPath $root -Recurse -Force
+	}
+}
+
 # The bare ktsu.Sdk entry. "ktsu.Sdk" -like "ktsu.Sdk.*" is False and the old csproj pattern
 # required a suffix, so the package nearly every repository pins was never tracked. VST, which
 # pins only ktsu.Sdk, reported "No ktsu SDKs found" and sat on 2.8.0 while the org moved on.
@@ -110,6 +229,35 @@ Test-Case -Name 'an already-current repository is untouched' -Version '2.29.0' `
 	-Files @{ 'global.json' = "{`n  `"msbuild-sdks`": {`n    `"ktsu.Sdk`": `"2.29.0`"`n  }`n}`n" } `
 	-Expected @{ 'global.json' = "{`n  `"msbuild-sdks`": {`n    `"ktsu.Sdk`": `"2.29.0`"`n  }`n}`n" }
 
+# $ErrorActionPreference is Stop inside the script, which makes a bare Write-Error terminating.
+# The first unresolvable package therefore threw out of the script and the `continue` beneath it
+# never ran, so a maintainer saw one failure per run: fix it, rerun, discover the next. The
+# packages are grouped alphabetically, so ktsu.Sdk failing used to hide the other two entirely.
+$threePackages = '{"msbuild-sdks":{"ktsu.Sdk":"1.0.0","ktsu.Sdk.App":"1.0.0","ktsu.Sdk.Tool":"1.0.0"}}'
+Test-ResolutionFailure -Name 'every unresolvable package is reported, not just the first' `
+	-Files @{ 'global.json' = $threePackages } `
+	-FeedUrl 'http://127.0.0.1:9' `
+	-Reports @('ktsu.Sdk.App', 'ktsu.Sdk.Tool') `
+	-Unchanged @{ 'global.json' = $threePackages }
+
+# The same defect on the other branch, and the reason the run must fail before it writes. Here
+# ktsu.Sdk resolves to a version the fixture is behind, so a fix that reported the failures and
+# carried on would converge that one reference and leave the other two pinned to 1.0.0 -- the
+# partially applied state this script exists to repair. ktsu.Sdk.App 404s (the catch branch) and
+# ktsu.Sdk.Tool publishes only a prerelease (the "no released version" branch).
+$mixed = '{"msbuild-sdks":{"ktsu.Sdk":"1.0.0","ktsu.Sdk.App":"1.0.0","ktsu.Sdk.Tool":"1.0.0"}}'
+$feed = Start-StubFeed -Versions @{ 'ktsu.sdk' = @('2.29.0'); 'ktsu.sdk.tool' = @('1.0.0-pre.1') }
+try {
+	Test-ResolutionFailure -Name 'a resolvable package neither hides a failure nor is written past one' `
+		-Files @{ 'global.json' = $mixed } `
+		-FeedUrl $feed.Url `
+		-Reports @('ktsu.Sdk.App', 'ktsu.Sdk.Tool') `
+		-Unchanged @{ 'global.json' = $mixed }
+}
+finally {
+	Stop-StubFeed -Feed $feed
+}
+
 Write-Host ''
 if ($script:Failures -gt 0) {
 	Write-Host "$script:Failures of $script:Count case(s) failed." -ForegroundColor Red

scripts/update-sdks.ps1

@@ -162,6 +162,14 @@ foreach ($group in $references | Group-Object Name | Sort-Object Name) {
 	Write-Host "  $($group.Name): $pinned"
 }
 
+# A package that cannot be resolved is a failure, but it is not a reason to stop looking at the
+# rest of them. $ErrorActionPreference is Stop in this scope, which makes a bare Write-Error
+# terminating, so the first unresolvable package used to throw straight out of the script and
+# the `continue` beneath it was dead code. A maintainer saw one failure per run and had to fix
+# and rerun to discover the next. Failures are named on the error stream as they happen, and
+# the run fails once, after every package has been looked at.
+$failed = [System.Collections.Generic.List[string]]::new()
+
 $targets = @{}
 foreach ($group in $references | Group-Object Name | Sort-Object Name) {
 	$name = $group.Name
@@ -179,12 +187,14 @@ foreach ($group in $references | Group-Object Name | Sort-Object Name) {
 		# A lookup failure must not be reported as "up to date". That conflation is what let
 		# this run green and do nothing every week: a parse error inside the lookup was caught
 		# and turned into a null, and a null read as "no update available".
-		Write-Error "Could not resolve the latest version of $name from $FeedUrl : $($_.Exception.Message)"
+		Write-Error "Could not resolve the latest version of $name from $FeedUrl : $($_.Exception.Message)" -ErrorAction Continue
+		$failed.Add($name)
 		continue
 	}
 
 	if (-not $latest) {
-		Write-Error "$name has no released version on $FeedUrl."
+		Write-Error "$name has no released version on $FeedUrl." -ErrorAction Continue
+		$failed.Add($name)
 		continue
 	}
 
@@ -201,6 +211,14 @@ foreach ($group in $references | Group-Object Name | Sort-Object Name) {
 	$targets[$name] = $latest
 }
 
+# Failing here rather than earlier is what makes one run report every unresolvable package. It
+# is still before anything is written, so a run that could not resolve part of the family does
+# not leave the repository half-converged -- which is the very state this script exists to
+# repair. The messages above carry the reasons; this carries the verdict.
+if ($failed.Count -gt 0) {
+	throw "Could not resolve $($failed.Count) of $($targets.Count + $failed.Count) $Prefix package(s): $($failed -join ', ')"
+}
+
 $stale = @($references | Where-Object { $targets.ContainsKey($_.Name) -and $_.Version -ne $targets[$_.Name] })
 
 if ($stale.Count -eq 0) {