From a487f5d6915d5b360961dfd917d072a51973c6e5 Mon Sep 17 00:00:00 2001 From: Johnathan Walker Date: Fri, 7 Aug 2026 11:40:55 -0400 Subject: [PATCH 01/14] test(amqp): use pinned CFSClean broker Pin broker setup to the azure-amqp commit that owns the CFSClean NuGet config. Restore through that config, build net10.0, and fail with broker output when readiness does not arrive. Check that the clone stays clean, re-enable cleanup, and document the pin plus the external developer and CFSClean build paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/azure_core_amqp/README.md | 45 ++++---- sdk/core/azure_core_amqp/Test-Cleanup.ps1 | 6 - sdk/core/azure_core_amqp/Test-Setup.ps1 | 129 ++++++++++++++++++---- 3 files changed, 131 insertions(+), 49 deletions(-) diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index 9133de3575..3b9cd5f898 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -14,61 +14,66 @@ The AMQP package is tested using the standard `cargo test` command line: cargo test --package azure_core_amqp --all-features ``` -Certain AMQP tests requires that there be a running AMQP broker on the machine at the time of the test (the tests will run without the broker, the relevant tests will just be skipped). +Certain AMQP tests require a running AMQP broker. The tests without a broker still run, and the broker-dependent tests are skipped. One existing AMQP broker is the "TestAMQPBroker" from the azure-amqp GitHub repository. -To launch the TestAMQPBroker, there are two ways of installing and running the TestAmqpBroker, Scripted and Manual. +The broker can be installed and run through the setup script or through the manual steps below. -### Scripted Broker Install +### Scripted broker install -Running the broker from a script requires that you first [install Powershell](https://learn.microsoft.com/powershell/scripting/install/installing-powershell?view=powershell-7.4). -From a running powershell instance, run the powershell script in the sdk/core/azure_core_amqp directory: +Install [PowerShell](https://learn.microsoft.com/powershell/scripting/install/installing-powershell) and the [.NET 10 SDK](https://dot.net/download). Run the setup script from the repository root. ```pwsh ./sdk/core/azure_core_amqp/Test-Setup.ps1 ``` -This will download the TestAmqpBroker, build it and launch the executable in the background. +The script clones Azure/azure-amqp at commit [`239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a`](https://github.com/Azure/azure-amqp/commit/239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a), restores through the broker repository's `nuget.cfsclean.config`, builds `TestAmqpBroker` for .NET 10, and launches it in the background. The config was added in [Azure/azure-amqp#318](https://github.com/Azure/azure-amqp/pull/318). -Note that this requires that you have the [.NET SDK](https://dot.net/download) installed on your machine. +Run the package tests in the same PowerShell process so `TEST_BROKER_ADDRESS` remains available. -You can then run the azure_core_amqp package tests. +```pwsh +cargo test --package azure_core_amqp --all-features +``` -Once you have finished running your tests, you run: +Stop the broker after the tests finish. ```pwsh ./sdk/core/azure_core_amqp/Test-Cleanup.ps1 ``` -which will terminate the test broker. +#### Updating the broker pin -### Manual Broker Install +Update the pin only to an azure-amqp commit that contains `nuget.cfsclean.config` and builds `TestAmqpBroker` for `net10.0`. Change `$repositoryHash` in `Test-Setup.ps1`, run the setup and cleanup scripts, and confirm that setup reports a clean azure-amqp clone. -For Manual testing, first clone the azure-amqp repository to a local directory: +### Manual broker install + +Clone the pinned azure-amqp commit to a local directory. ```pwsh cd -git clone https://github.com/Azure/azure-amqp +git clone https://github.com/Azure/azure-amqp --revision 239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a ``` -Alternately, you can clone to a specific release in the azure-amqp repository: +Normal external developer builds use the repository's standard NuGet configuration. ```pwsh -git clone https://github.com/Azure/azure-amqp.git --branch hotfix +cd azure-amqp +dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug --framework net10.0 ``` -Set an environment variable the test AMQP broker should listen on: +CFSClean builds must authenticate to the `azure-sdk-for-net` Azure Artifacts feed, then run this restore and build sequence from the clone root. ```pwsh -$env:TEST_BROKER_ADDRESS = 'amqp://127.0.0.1:25672' +dotnet restore .\test\TestAmqpBroker\TestAmqpBroker.csproj --configfile .\nuget.cfsclean.config +dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug --framework net10.0 --no-restore ``` -And launch the test broker: +Set the broker address and launch the built assembly. ```pwsh -cd azure-amqp/test/TestAmqpBroker -dotnet run -- $env:TEST_BROKER_ADDRESS +$env:TEST_BROKER_ADDRESS = 'amqp://127.0.0.1:25672' +dotnet exec .\bin\Debug\TestAmqpBroker\net10.0\TestAmqpBroker.dll $env:TEST_BROKER_ADDRESS /headless ``` Now, when you run the cargo tests, the networking functionality of the AMQP APIs will be executed. diff --git a/sdk/core/azure_core_amqp/Test-Cleanup.ps1 b/sdk/core/azure_core_amqp/Test-Cleanup.ps1 index d281c7f254..722ddcf6ec 100644 --- a/sdk/core/azure_core_amqp/Test-Cleanup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Cleanup.ps1 @@ -9,12 +9,6 @@ if ($IsMacOS) { exit 0 } -if ($true) { - Write-Host "AMQP Test Broker tests disabled until test broker is updated." - exit 0 -} - - Write-Host "Test Broker output:" Receive-Job -Id $env:TEST_BROKER_JOBID diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index fa793eddd2..af6e7cc08e 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -6,13 +6,63 @@ # Load common ES scripts . "$PSScriptRoot\..\..\..\eng\common\scripts\common.ps1" -if ($IsMacOS) { - Write-Host "AMQP tests are not supported on macOS. Skipping test setup." - exit 0 +function Wait-TestBroker { + param( + [int]$JobId, + [string]$HostName, + [int]$Port, + [int]$TimeoutSeconds = 30 + ) + + $deadline = (Get-Date).AddSeconds($TimeoutSeconds) + while ((Get-Date) -lt $deadline) { + $job = Get-Job -Id $JobId -ErrorAction SilentlyContinue + if (!$job -or $job.State -ne "Running") { + return $false + } + + $client = [System.Net.Sockets.TcpClient]::new() + try { + $client.Connect($HostName, $Port) + return $true + } + catch { + Start-Sleep -Milliseconds 500 + } + finally { + $client.Dispose() + } + } + + return $false +} + +function Stop-TestBrokerJob { + param([int]$JobId) + + $job = Get-Job -Id $JobId -ErrorAction SilentlyContinue + if (!$job) { + return + } + + if ($job.State -eq "Running") { + Stop-Job -Id $JobId + } + Remove-Job -Id $JobId +} + +function Write-TestBrokerOutput { + param([int]$JobId) + + $job = Get-Job -Id $JobId -ErrorAction SilentlyContinue + if ($job) { + Write-Host "Test broker job state: $($job.State)" + Receive-Job -Id $JobId -Keep + } } -if ($true) { - Write-Host "AMQP Test Broker tests disabled until test broker is updated." +if ($IsMacOS) { + Write-Host "AMQP tests are not supported on macOS. Skipping test setup." exit 0 } @@ -40,24 +90,37 @@ try { } $repositoryUrl = "https://github.com/Azure/azure-amqp.git" - $repositoryHash = "d82a86455c3459c5628bc95b25511f6e8a065598" - $cloneCommand = "git clone $repositoryUrl --revision $repositoryHash --depth=1" + $repositoryHash = "239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a" + $cloneCommand = "git clone --revision $repositoryHash --depth=1 $repositoryUrl `"$repositoryDir`"" Write-Host "Cloning repository from $repositoryUrl..." Invoke-LoggedCommand $cloneCommand - Set-Location -Path "./azure-amqp/test/TestAmqpBroker" + $repositoryHead = Invoke-LoggedCommand "git -C `"$repositoryDir`" rev-parse HEAD" + if ($repositoryHead.Trim() -ne $repositoryHash) { + LogError "Expected azure-amqp commit $repositoryHash, but cloned $repositoryHead." + exit 1 + } - Invoke-LoggedCommand "dotnet build --framework net8.0" - if (-not $?) { - Write-Error "Failed to build TestAmqpBroker." + $brokerProject = [System.IO.Path]::Combine($repositoryDir, "test", "TestAmqpBroker", "TestAmqpBroker.csproj") + $nugetConfig = [System.IO.Path]::Combine($repositoryDir, "nuget.cfsclean.config") + if (!(Test-Path $nugetConfig)) { + LogError "The pinned azure-amqp commit does not contain $nugetConfig." exit 1 } + Invoke-LoggedCommand ` + "dotnet restore `"$brokerProject`" --configfile `"$nugetConfig`"" ` + -GroupOutput + Invoke-LoggedCommand ` + "dotnet build `"$brokerProject`" --configuration Debug --framework net10.0 --no-restore" ` + -GroupOutput + Write-Host "Test broker built successfully." - # now that the Test broker has been built, launch the broker on a local address. - $env:TEST_BROKER_ADDRESS = 'amqp://127.0.0.1:25672' + $brokerHost = "127.0.0.1" + $brokerPort = 25672 + $env:TEST_BROKER_ADDRESS = "amqp://${brokerHost}:${brokerPort}" Write-Host "Starting test broker listening on ${env:TEST_BROKER_ADDRESS} ..." @@ -65,22 +128,42 @@ try { # If we use `dotnet run -f`, the first argument is the csproj file. # Instead, we use `dotnet exec` to run the compiled DLL directly. # This allows us to pass the broker address as the first argument. - Set-Location -Path $WorkingDirectory/azure-amqp/bin/Debug/TestAmqpBroker/net8.0 - $job = dotnet exec ./TestAmqpBroker.dll ${env:TEST_BROKER_ADDRESS} /headless & + $brokerOutputDirectory = [System.IO.Path]::Combine( + $repositoryDir, + "bin", + "Debug", + "TestAmqpBroker", + "net10.0" + ) + $brokerAssembly = [System.IO.Path]::Combine($brokerOutputDirectory, "TestAmqpBroker.dll") + Set-Location -Path $brokerOutputDirectory + $job = dotnet exec $brokerAssembly ${env:TEST_BROKER_ADDRESS} /headless & $env:TEST_BROKER_JOBID = $job.Id - Write-Host "Waiting for test broker to start..." - Start-Sleep -Seconds 3 - - Write-Host "Job Output after wait:" - Receive-Job $job.Id + Write-Host "Waiting up to 30 seconds for the test broker to accept connections..." + if (!(Wait-TestBroker -JobId $job.Id -HostName $brokerHost -Port $brokerPort)) { + Write-TestBrokerOutput -JobId $job.Id + Stop-TestBrokerJob -JobId $job.Id + LogError "Test broker did not become ready at ${env:TEST_BROKER_ADDRESS}." + exit 1 + } - $job = Get-Job -Id $env:TEST_BROKER_JOBID - if ($job.State -ne "Running") { - Write-Host "Test broker failed to start." + Write-TestBrokerOutput -JobId $job.Id + Write-Host "Test broker is ready." + + $repositoryStatus = @( + Invoke-LoggedCommand "git -C `"$repositoryDir`" status --porcelain --untracked-files=all" + ) + if ($repositoryStatus.Count -ne 0) { + Write-Host "Files changed in the azure-amqp clone:" + $repositoryStatus | ForEach-Object { Write-Host $_ } + Stop-TestBrokerJob -JobId $job.Id + LogError "Test broker setup changed files in the azure-amqp clone." exit 1 } + + Write-Host "The azure-amqp clone is clean after setup." } finally { Pop-Location From 8af12cabc6e7847261a4bf03b90c97c8f140c777 Mon Sep 17 00:00:00 2001 From: Johnathan Walker Date: Fri, 7 Aug 2026 11:59:50 -0400 Subject: [PATCH 02/14] fix(amqp): allow broker config terms Teach cspell the broker config file and option names used by the CFSClean setup path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/core/azure_core_amqp/README.md | 2 ++ sdk/core/azure_core_amqp/Test-Setup.ps1 | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index 3b9cd5f898..c957bf5623 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -1,3 +1,5 @@ + + # Azure AMQP library for Rust Azure AMQP crate for consumption of AMQP based packages in the Azure SDK for Rust and C++. diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index af6e7cc08e..5c228ad512 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -1,6 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -# cspell: ignore JOBID depsfile +# cspell: ignore JOBID cfsclean configfile depsfile # Load common ES scripts From a7cce6a2ecf511dc36de19c939cd049ee5f61ced Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 10:12:01 -0400 Subject: [PATCH 03/14] test(amqp): fail on a missing broker and harden the broker pin The broker tests skipped when TEST_BROKER_ADDRESS was absent, and they still passed. The suite therefore stayed green when the broker did not run at all. Add a test_broker helper. TEST_BROKER_REQUIRED makes a missing broker an error, and the pipeline sets it for every job except macOS, where setup never starts a broker. Run the restore and the build from the clone root. The dotnet command reads global.json from the current directory and not from the project directory, so the earlier absolute paths lost the SDK pin of the clone. Stop the broker from the finally block behind a success flag. Invoke-LoggedCommand calls exit from inside itself, so the earlier inline calls did not run on every failure path and left the broker on port 25672. Bound each readiness attempt. The blocking Connect method takes no timeout, so a dropped SYN made the probe run far past its own budget. Add TEST_BROKER_COMMIT to override the pinned commit without a code change, and test whether the pin is reachable from master through the GitHub compare API. An unreachable pin warns, and TEST_BROKER_REQUIRE_MERGED turns that warning into an error. A check that cannot run always continues, because the anonymous rate limit is shared and says nothing about the pin. --- eng/pipelines/templates/jobs/ci.tests.yml | 6 + sdk/core/azure_core_amqp/README.md | 18 +- sdk/core/azure_core_amqp/Test-Cleanup.ps1 | 7 + sdk/core/azure_core_amqp/Test-Setup.ps1 | 196 ++++++++++++++++++-- sdk/core/azure_core_amqp/src/connection.rs | 105 +++++------ sdk/core/azure_core_amqp/src/lib.rs | 2 + sdk/core/azure_core_amqp/src/test_broker.rs | 74 ++++++++ 7 files changed, 338 insertions(+), 70 deletions(-) create mode 100644 sdk/core/azure_core_amqp/src/test_broker.rs diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index 024ed41565..e9bf4d4435 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -78,6 +78,12 @@ jobs: timeoutInMinutes: ${{ parameters.TimeoutInMinutes }} env: CIBW_BUILD_VERBOSITY: 3 + # The azure_core_amqp broker tests skip when TEST_BROKER_ADDRESS is absent. + # In CI they must fail instead, so a broker that stops running turns the + # build red. Test-Setup.ps1 exits early on macOS and never starts the + # broker, so macOS keeps the skip behavior. + ${{ if ne(parameters.OSName, 'macOS') }}: + TEST_BROKER_REQUIRED: 'true' inputs: pwsh: true filePath: $(Build.SourcesDirectory)/eng/scripts/Test-Packages.ps1 diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index c957bf5623..24e8543060 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -18,13 +18,15 @@ cargo test --package azure_core_amqp --all-features Certain AMQP tests require a running AMQP broker. The tests without a broker still run, and the broker-dependent tests are skipped. +Set `TEST_BROKER_REQUIRED` to make a missing broker an error instead of a skip. The broker-dependent tests then fail when `TEST_BROKER_ADDRESS` is absent. The CI pipeline sets `TEST_BROKER_REQUIRED`, so a broker that stops running turns the build red. + One existing AMQP broker is the "TestAMQPBroker" from the azure-amqp GitHub repository. The broker can be installed and run through the setup script or through the manual steps below. ### Scripted broker install -Install [PowerShell](https://learn.microsoft.com/powershell/scripting/install/installing-powershell) and the [.NET 10 SDK](https://dot.net/download). Run the setup script from the repository root. +Install [PowerShell](https://learn.microsoft.com/powershell/scripting/install/installing-powershell), the [.NET 10 SDK](https://dot.net/download), and [Git](https://git-scm.com/downloads) 2.49 or later. Git 2.49 added the `git clone --revision` option that the setup script uses. Run the setup script from the repository root. ```pwsh ./sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -46,7 +48,17 @@ Stop the broker after the tests finish. #### Updating the broker pin -Update the pin only to an azure-amqp commit that contains `nuget.cfsclean.config` and builds `TestAmqpBroker` for `net10.0`. Change `$repositoryHash` in `Test-Setup.ps1`, run the setup and cleanup scripts, and confirm that setup reports a clean azure-amqp clone. +Update the pin only to an azure-amqp commit that contains `nuget.cfsclean.config` and builds `TestAmqpBroker` for `net10.0`. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, update the same SHA in this file, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. + +Set `TEST_BROKER_COMMIT` to try a different commit without a code change. + +```pwsh +$env:TEST_BROKER_COMMIT = '' +``` + +Setup also asks the GitHub compare API whether the pinned commit is reachable from azure-amqp `master`. A reachable pin says nothing. An unreachable pin writes a warning, and `TEST_BROKER_REQUIRE_MERGED` turns that warning into an error. A check that could not run writes a warning and always continues, because the unauthenticated rate limit is 60 requests each hour for each IP address. + +The current pin is the head of the pull request [Azure/azure-amqp#318](https://github.com/Azure/azure-amqp/pull/318), so the warning appears today. It will still appear after that pull request merges. azure-amqp squash-merges, so the head commit of a pull request never lands on `master`. To clear the warning, move the pin to the squash commit on `master`, which is the `merge_commit_sha` of the merged pull request. Do not use the `merge_commit_sha` of an open pull request, because that is a throwaway test-merge commit that disappears. ### Manual broker install @@ -64,7 +76,7 @@ cd azure-amqp dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug --framework net10.0 ``` -CFSClean builds must authenticate to the `azure-sdk-for-net` Azure Artifacts feed, then run this restore and build sequence from the clone root. +CFSClean builds restore from the `azure-sdk-for-net` Azure Artifacts feed. The feed is public and answers anonymous reads, so an external developer needs no credentials to restore a package that the feed has already cached. The CFSClean environment supplies credentials because a cache miss makes the feed fetch the package from upstream, and that fetch needs an authenticated caller. Run this restore and build sequence from the clone root. ```pwsh dotnet restore .\test\TestAmqpBroker\TestAmqpBroker.csproj --configfile .\nuget.cfsclean.config diff --git a/sdk/core/azure_core_amqp/Test-Cleanup.ps1 b/sdk/core/azure_core_amqp/Test-Cleanup.ps1 index 722ddcf6ec..811aec6c58 100644 --- a/sdk/core/azure_core_amqp/Test-Cleanup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Cleanup.ps1 @@ -9,6 +9,13 @@ if ($IsMacOS) { exit 0 } +# Test-Setup.ps1 clears TEST_BROKER_JOBID when it stops the broker itself, so +# there is nothing to clean up. +if ([string]::IsNullOrWhiteSpace($env:TEST_BROKER_JOBID)) { + Write-Host "TEST_BROKER_JOBID is not set. The test broker is not running." + exit 0 +} + Write-Host "Test Broker output:" Receive-Job -Id $env:TEST_BROKER_JOBID diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index 5c228ad512..1607c2a713 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -21,22 +21,94 @@ function Wait-TestBroker { return $false } + # Bound each attempt. The blocking Connect() method takes no timeout, so a + # firewall that drops the SYN makes it wait for the operating system + # default. That default is longer than $TimeoutSeconds on Linux. + $remainingMilliseconds = [int][Math]::Min(1000, ($deadline - (Get-Date)).TotalMilliseconds) + if ($remainingMilliseconds -le 0) { + break + } + $client = [System.Net.Sockets.TcpClient]::new() try { - $client.Connect($HostName, $Port) - return $true + $connectTask = $client.ConnectAsync($HostName, $Port) + if ($connectTask.Wait($remainingMilliseconds) -and $client.Connected) { + return $true + } } catch { - Start-Sleep -Milliseconds 500 + # The connection failed. Try again until the deadline. + Write-Debug "Connection to ${HostName}:${Port} failed: $_" } finally { $client.Dispose() } + + Start-Sleep -Milliseconds 500 } return $false } +function Test-EnvironmentFlag { + param([string]$Name) + + $value = [System.Environment]::GetEnvironmentVariable($Name) + if ([string]::IsNullOrWhiteSpace($value)) { + return $false + } + + $value = $value.Trim() + return -not (($value -eq "0") -or ($value -ieq "false")) +} + +function Test-BrokerPinReachable { + param( + [string]$Repository, + [string]$CommitHash, + [string]$BranchName = "master", + [int]$TimeoutSeconds = 15 + ) + + # `git merge-base --is-ancestor` cannot answer this question. The broker + # clone is shallow, so it holds one commit and no parents, and every commit + # looks unreachable. The GitHub compare API answers it in one call, and that + # call works without a token on a public repository. + # + # The rule is `ahead_by -eq 0`. Do not read the status string: a reachable + # commit reports "identical" when it is the head of the branch, and "behind" + # when it is older. + # + # The function returns $true, $false, or $null when the check did not run. + # The unauthenticated rate limit is 60 requests each hour for each IP + # address, and CI agents share an address, so a failed call must never fail + # the build. Every failure returns $null. + $compareUri = "https://api.github.com/repos/$Repository/compare/$BranchName...$CommitHash" + $headers = @{ + "Accept" = "application/vnd.github+json" + "X-GitHub-Api-Version" = "2022-11-28" + } + + try { + $comparison = Invoke-RestMethod ` + -Uri $compareUri ` + -Method Get ` + -Headers $headers ` + -TimeoutSec $TimeoutSeconds + } + catch { + LogWarning "The request to $compareUri failed: $_" + return $null + } + + if ($null -eq $comparison -or $null -eq $comparison.ahead_by) { + LogWarning "The response from $compareUri does not hold an ahead_by field." + return $null + } + + return [bool]($comparison.ahead_by -eq 0) +} + function Stop-TestBrokerJob { param([int]$JobId) @@ -80,6 +152,13 @@ if (-not (Test-Path $WorkingDirectory)) { Write-Host "Setting current directory to working directory: $WorkingDirectory" Push-Location -Path $WorkingDirectory +# The identifier of the broker job, and the flag that tells the finally block +# whether setup finished. The finally block stops the broker on every path +# that does not finish, because Invoke-LoggedCommand calls `exit` from inside +# itself when a command fails. +$brokerJobId = $null +$setupSucceeded = $false + # Clone and build the Test Amqp Broker. try { @@ -89,32 +168,110 @@ try { Remove-Item $repositoryDir -Force -Recurse | Out-Null } - $repositoryUrl = "https://github.com/Azure/azure-amqp.git" + $repositoryName = "Azure/azure-amqp" + $repositoryUrl = "https://github.com/$repositoryName.git" + + # The pinned azure-amqp commit, as a full 40-character SHA, so that the + # broker build stays reproducible. A tag is not an option here, because + # azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer + # can move a tag to a different commit without a trace. + # + # This SHA comes from refs/pull/318/head of Azure/azure-amqp, the branch + # that adds nuget.cfsclean.config. It is not on master, so the reachability + # check below writes a warning and continues. + # + # To update the pin: + # 1. Pick an azure-amqp commit that contains nuget.cfsclean.config and + # builds TestAmqpBroker for net10.0. + # 2. Put the full 40-character SHA of that commit below, and update the + # comment above with the ref that the SHA comes from. For a merged + # pull request, use the merge_commit_sha, not the head SHA. + # 3. Update the same SHA in sdk/core/azure_core_amqp/README.md. + # 4. Run this script and then Test-Cleanup.ps1. Make sure that setup + # reports a clean azure-amqp clone. + # + # Set TEST_BROKER_COMMIT to point the broker at a different commit without a + # code change. $repositoryHash = "239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a" + if (-not [string]::IsNullOrWhiteSpace($env:TEST_BROKER_COMMIT)) { + $repositoryHash = $env:TEST_BROKER_COMMIT.Trim() + Write-Host "TEST_BROKER_COMMIT overrides the pinned azure-amqp commit: $repositoryHash" + } + + if ($repositoryHash -notmatch '^[0-9a-fA-F]{40}$') { + LogError "The azure-amqp pin must be a full 40-character commit SHA, but it is '$repositoryHash'." + exit 1 + } + $cloneCommand = "git clone --revision $repositoryHash --depth=1 $repositoryUrl `"$repositoryDir`"" Write-Host "Cloning repository from $repositoryUrl..." Invoke-LoggedCommand $cloneCommand - $repositoryHead = Invoke-LoggedCommand "git -C `"$repositoryDir`" rev-parse HEAD" + # Take the last line only. Invoke-LoggedCommand returns every output line, + # and an array here would turn the comparison below into a filter. + $repositoryHead = "$(Invoke-LoggedCommand "git -C `"$repositoryDir`" rev-parse HEAD" | Select-Object -Last 1)" if ($repositoryHead.Trim() -ne $repositoryHash) { LogError "Expected azure-amqp commit $repositoryHash, but cloned $repositoryHead." exit 1 } - $brokerProject = [System.IO.Path]::Combine($repositoryDir, "test", "TestAmqpBroker", "TestAmqpBroker.csproj") - $nugetConfig = [System.IO.Path]::Combine($repositoryDir, "nuget.cfsclean.config") + # A reachable pin says nothing. Only the other two outcomes write a message. + $pinIsOnMaster = Test-BrokerPinReachable ` + -Repository $repositoryName ` + -CommitHash $repositoryHash + if ($null -eq $pinIsOnMaster) { + # The check did not run. Continue always, even when + # TEST_BROKER_REQUIRE_MERGED is set. A rate limit or a network error is + # not evidence that the pin is bad. + LogWarning "The reachability check for azure-amqp commit $repositoryHash did not run. The pin is unchanged." + } + elseif (-not $pinIsOnMaster) { + $pinMessage = @( + "The azure-amqp commit $repositoryHash is not reachable from master." + "If the source pull request has merged, this is expected: azure-amqp squash-merges, so the head commit of a pull request never lands on master." + "Update the pin to the squash commit on master, which is the merge_commit_sha of the merged pull request. Do not use the merge_commit_sha of an open pull request, because that is a throwaway test-merge commit that disappears." + ) -join "`n" + + if (Test-EnvironmentFlag "TEST_BROKER_REQUIRE_MERGED") { + LogError "$pinMessage`nTEST_BROKER_REQUIRE_MERGED is set, so this is an error." + exit 1 + } + LogWarning $pinMessage + } + + # The dotnet arguments below are relative to the clone root. Keep the + # absolute forms only for the checks. + $brokerProjectRelative = [System.IO.Path]::Combine("test", "TestAmqpBroker", "TestAmqpBroker.csproj") + $nugetConfigRelative = "nuget.cfsclean.config" + $brokerProject = [System.IO.Path]::Combine($repositoryDir, $brokerProjectRelative) + $nugetConfig = [System.IO.Path]::Combine($repositoryDir, $nugetConfigRelative) + if (!(Test-Path $brokerProject)) { + LogError "The pinned azure-amqp commit does not contain $brokerProject." + exit 1 + } if (!(Test-Path $nugetConfig)) { LogError "The pinned azure-amqp commit does not contain $nugetConfig." exit 1 } - Invoke-LoggedCommand ` - "dotnet restore `"$brokerProject`" --configfile `"$nugetConfig`"" ` - -GroupOutput - Invoke-LoggedCommand ` - "dotnet build `"$brokerProject`" --configuration Debug --framework net10.0 --no-restore" ` - -GroupOutput + # Run the restore and the build from the clone root. This Push-Location is + # load-bearing: the dotnet command reads global.json from the current + # directory and not from the project directory, and the arguments above are + # relative to the clone root. Without it, the SDK version in the + # global.json of azure-amqp is never applied. + Push-Location -Path $repositoryDir + try { + Invoke-LoggedCommand ` + "dotnet restore `"$brokerProjectRelative`" --configfile `"$nugetConfigRelative`"" ` + -GroupOutput + Invoke-LoggedCommand ` + "dotnet build `"$brokerProjectRelative`" --configuration Debug --framework net10.0 --no-restore" ` + -GroupOutput + } + finally { + Pop-Location + } Write-Host "Test broker built successfully." @@ -139,12 +296,12 @@ try { Set-Location -Path $brokerOutputDirectory $job = dotnet exec $brokerAssembly ${env:TEST_BROKER_ADDRESS} /headless & + $brokerJobId = $job.Id $env:TEST_BROKER_JOBID = $job.Id Write-Host "Waiting up to 30 seconds for the test broker to accept connections..." if (!(Wait-TestBroker -JobId $job.Id -HostName $brokerHost -Port $brokerPort)) { Write-TestBrokerOutput -JobId $job.Id - Stop-TestBrokerJob -JobId $job.Id LogError "Test broker did not become ready at ${env:TEST_BROKER_ADDRESS}." exit 1 } @@ -158,13 +315,22 @@ try { if ($repositoryStatus.Count -ne 0) { Write-Host "Files changed in the azure-amqp clone:" $repositoryStatus | ForEach-Object { Write-Host $_ } - Stop-TestBrokerJob -JobId $job.Id LogError "Test broker setup changed files in the azure-amqp clone." exit 1 } Write-Host "The azure-amqp clone is clean after setup." + $setupSucceeded = $true } finally { + # Stop the broker on every path that does not finish setup. A broker that + # stays alive holds port 25672 and breaks the next run. + if (-not $setupSucceeded -and $null -ne $brokerJobId) { + Write-Host "Setup did not finish. Stopping the test broker." + Stop-TestBrokerJob -JobId $brokerJobId + $env:TEST_BROKER_JOBID = $null + $env:TEST_BROKER_ADDRESS = $null + } + Pop-Location } diff --git a/sdk/core/azure_core_amqp/src/connection.rs b/sdk/core/azure_core_amqp/src/connection.rs index ba15a6329c..2bdfafdfda 100644 --- a/sdk/core/azure_core_amqp/src/connection.rs +++ b/sdk/core/azure_core_amqp/src/connection.rs @@ -119,6 +119,7 @@ impl AmqpConnection { #[cfg(test)] mod tests { use super::*; + use crate::test_broker::test_broker_address; #[test] fn amqp_connection_options_with_max_frame_size() { @@ -288,75 +289,75 @@ mod tests { #[cfg(not(target_os = "macos"))] #[tokio::test] async fn amqp_connection_open() { - if let Ok(address) = std::env::var("TEST_BROKER_ADDRESS") { - let connection = AmqpConnection::new(); - let url = Url::parse(&address).unwrap(); - connection - .open("test".to_string(), url, None) - .await - .unwrap(); - } else { - println!("TEST_BROKER_ADDRESS is not set. Skipping test."); - } + let Some(address) = test_broker_address() else { + return; + }; + + let connection = AmqpConnection::new(); + let url = Url::parse(&address).unwrap(); + connection + .open("test".to_string(), url, None) + .await + .unwrap(); } #[tokio::test] async fn amqp_connection_open_with_error() { - if std::env::var("TEST_BROKER_ADDRESS").is_ok() { - let connection = AmqpConnection::new(); - let url = Url::parse("amqp://localhost:32767").unwrap(); - assert!(connection - .open("test".to_string(), url, None) - .await - .is_err()); - } else { - println!("TEST_BROKER_ADDRESS is not set. Skipping test."); + if test_broker_address().is_none() { + return; } + + let connection = AmqpConnection::new(); + let url = Url::parse("amqp://localhost:32767").unwrap(); + assert!(connection + .open("test".to_string(), url, None) + .await + .is_err()); } #[cfg(not(target_os = "macos"))] #[tokio::test] async fn amqp_connection_close() { - if let Ok(address) = std::env::var("TEST_BROKER_ADDRESS") { - let connection = AmqpConnection::new(); - let url = Url::parse(&address).unwrap(); - connection - .open("test".to_string(), url, None) - .await - .unwrap(); - connection.close().await.unwrap(); - } else { - println!("TEST_BROKER_ADDRESS is not set. Skipping test."); - } + let Some(address) = test_broker_address() else { + return; + }; + + let connection = AmqpConnection::new(); + let url = Url::parse(&address).unwrap(); + connection + .open("test".to_string(), url, None) + .await + .unwrap(); + connection.close().await.unwrap(); } #[cfg(not(target_os = "macos"))] #[tokio::test] async fn amqp_connection_close_with_error() { tracing_subscriber::fmt::init(); - if let Ok(address) = std::env::var("TEST_BROKER_ADDRESS") { - let connection = AmqpConnection::new(); - let url = Url::parse(&address).unwrap(); - connection - .open("test".to_string(), url, None) - .await - .unwrap(); - let res = connection - .close_with_error( - AmqpSymbol::from("amqp:internal-error"), - Some("Internal error.".to_string()), - None, - ) - .await; - match res { - Ok(_) => {} - Err(err) => { - println!("Error: {:?}", err); - assert!(err.to_string().contains("Internal error.")); - } + let Some(address) = test_broker_address() else { + return; + }; + + let connection = AmqpConnection::new(); + let url = Url::parse(&address).unwrap(); + connection + .open("test".to_string(), url, None) + .await + .unwrap(); + let res = connection + .close_with_error( + AmqpSymbol::from("amqp:internal-error"), + Some("Internal error.".to_string()), + None, + ) + .await; + match res { + Ok(_) => {} + Err(err) => { + println!("Error: {:?}", err); + assert!(err.to_string().contains("Internal error.")); } - } else { - println!("TEST_BROKER_ADDRESS is not set. Skipping test."); } } } diff --git a/sdk/core/azure_core_amqp/src/lib.rs b/sdk/core/azure_core_amqp/src/lib.rs index e691eb96e7..41a0c27ea2 100644 --- a/sdk/core/azure_core_amqp/src/lib.rs +++ b/sdk/core/azure_core_amqp/src/lib.rs @@ -21,6 +21,8 @@ mod receiver; mod sender; mod session; mod simple_value; +#[cfg(test)] +mod test_broker; mod value; pub use cbs::{AmqpClaimsBasedSecurity, AmqpClaimsBasedSecurityApis}; diff --git a/sdk/core/azure_core_amqp/src/test_broker.rs b/sdk/core/azure_core_amqp/src/test_broker.rs new file mode 100644 index 0000000000..84a0ade7f5 --- /dev/null +++ b/sdk/core/azure_core_amqp/src/test_broker.rs @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. All Rights reserved +// Licensed under the MIT license. + +//! Helpers for the tests that need the local AMQP test broker. +//! +//! `Test-Setup.ps1` builds the broker, starts it, and sets +//! `TEST_BROKER_ADDRESS`. A test that needs the broker calls +//! [`test_broker_address`]. +//! +//! Two environment variables control the behavior: +//! +//! * `TEST_BROKER_ADDRESS` holds the broker address, for example +//! `amqp://127.0.0.1:25672`. +//! * `TEST_BROKER_REQUIRED` makes a missing broker an error instead of a skip. +//! The pipeline sets it, so that a broker that stops running makes the build +//! red. A developer who does not set it can still run the other tests. +//! +//! `TEST_BROKER_REQUIRED` is on when it holds a value other than an empty +//! string, `0`, or `false`. + +/// Name of the variable that holds the address of the test broker. +const TEST_BROKER_ADDRESS: &str = "TEST_BROKER_ADDRESS"; + +/// Name of the variable that turns a skipped broker test into a failure. +const TEST_BROKER_REQUIRED: &str = "TEST_BROKER_REQUIRED"; + +/// Returns the address of the test broker, or `None` when the caller must skip +/// the test. +/// +/// # Panics +/// +/// Panics when `TEST_BROKER_REQUIRED` is on and the broker address is absent or +/// empty. +pub(crate) fn test_broker_address() -> Option { + let address = std::env::var(TEST_BROKER_ADDRESS) + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()); + if address.is_some() { + return address; + } + + assert!( + !is_env_flag_set(TEST_BROKER_REQUIRED), + "{TEST_BROKER_REQUIRED} is set, but {TEST_BROKER_ADDRESS} is absent or empty. \ + Start the broker with sdk/core/azure_core_amqp/Test-Setup.ps1 and run the tests in \ + the same shell, or clear {TEST_BROKER_REQUIRED} to skip the broker tests." + ); + + println!("{TEST_BROKER_ADDRESS} is not set. Skipping test."); + None +} + +/// Returns `true` when the variable holds a value other than an empty string, +/// `0`, or `false`. +fn is_env_flag_set(name: &str) -> bool { + match std::env::var(name) { + Ok(value) => { + let value = value.trim(); + !value.is_empty() && value != "0" && !value.eq_ignore_ascii_case("false") + } + Err(_) => false, + } +} + +#[cfg(test)] +mod tests { + use super::is_env_flag_set; + + #[test] + fn env_flag_is_off_when_absent() { + assert!(!is_env_flag_set("AZURE_CORE_AMQP_FLAG_THAT_IS_NEVER_SET")); + } +} From d6621d1775faf225e8f201e63af5e71b88230cbc Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 11:40:31 -0400 Subject: [PATCH 04/14] test(amqp): harden the pin guard against a malformed compare answer A non-numeric ahead_by read as "not reachable" before. A build that sets TEST_BROKER_REQUIRE_MERGED could then fail because of a malformed answer and not because of a bad pin. Count that case as "the check did not run" instead. Coerce after the absence test and never instead of it, because $null -as [int] gives 0. A coercion on its own would turn a missing ahead_by field into 0 and report an unreachable pin as reachable. Correct the comment above the pin. The SHA is now an ancestor of refs/pull/318/head and not the head itself, because that pull request has moved on. --- sdk/core/azure_core_amqp/Test-Setup.ps1 | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index 1607c2a713..c68b25da8b 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -106,7 +106,17 @@ function Test-BrokerPinReachable { return $null } - return [bool]($comparison.ahead_by -eq 0) + # Coerce after the absence test, never instead of it. `$null -as [int]` gives 0, so a + # coercion on its own would turn a missing field into 0 and read as reachable. A + # non-numeric ahead_by counts as "the check did not run", because a build that sets + # TEST_BROKER_REQUIRE_MERGED must not fail on a malformed answer. + $aheadBy = $comparison.ahead_by -as [int] + if ($null -eq $aheadBy) { + LogWarning "The response from $compareUri holds a non-numeric ahead_by field." + return $null + } + + return [bool]($aheadBy -eq 0) } function Stop-TestBrokerJob { @@ -176,9 +186,10 @@ try { # azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer # can move a tag to a different commit without a trace. # - # This SHA comes from refs/pull/318/head of Azure/azure-amqp, the branch - # that adds nuget.cfsclean.config. It is not on master, so the reachability - # check below writes a warning and continues. + # This SHA is the commit of Azure/azure-amqp pull request 318 that adds + # nuget.cfsclean.config. That pull request has moved on since, so the SHA is + # now an ancestor of refs/pull/318/head and not the head itself. It is not on + # master, so the reachability check below writes a warning and continues. # # To update the pin: # 1. Pick an azure-amqp commit that contains nuget.cfsclean.config and From 58705ad34dd684b7554e1c2e0c04cf1c247298c8 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 11:43:36 -0400 Subject: [PATCH 05/14] fix(amqp): repair the rev-parse call that CI showed was broken The previous commit wrapped the rev-parse call in "$( ... )" to take the last output line. That broke the argument. The outer double-quoted string consumes the escaped quotes first, so the command split into three arguments. The second one bound to -ExecutePath, and git ran with a bare -C. CI showed the result: > git -C Push-Location: Cannot find path '.../TestArtifacts/azure-amqp rev-parse HEAD' because it does not exist. Use two statements instead, so no outer string can consume the quotes. The last-line behavior stays the same, and the result is still a string. --- sdk/core/azure_core_amqp/Test-Setup.ps1 | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index c68b25da8b..f711bc0ab7 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -219,9 +219,14 @@ try { Write-Host "Cloning repository from $repositoryUrl..." Invoke-LoggedCommand $cloneCommand - # Take the last line only. Invoke-LoggedCommand returns every output line, - # and an array here would turn the comparison below into a filter. - $repositoryHead = "$(Invoke-LoggedCommand "git -C `"$repositoryDir`" rev-parse HEAD" | Select-Object -Last 1)" + # Take the last line only. Invoke-LoggedCommand returns every output line, and an array + # here would turn the comparison below into a filter. + # + # Keep this as two statements. Wrapping the call in "$( ... )" breaks the argument, because + # the outer double-quoted string consumes the escaped quotes first. The command then splits + # into three arguments, the second one binds to -ExecutePath, and git runs with a bare -C. + $repositoryHeadLines = Invoke-LoggedCommand "git -C `"$repositoryDir`" rev-parse HEAD" + $repositoryHead = [string]($repositoryHeadLines | Select-Object -Last 1) if ($repositoryHead.Trim() -ne $repositoryHash) { LogError "Expected azure-amqp commit $repositoryHash, but cloned $repositoryHead." exit 1 From bece9aba98c570a7fb1a87a884dcc7c63affaac4 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 14:46:18 -0400 Subject: [PATCH 06/14] chore(amqp): add a temporary NuGet source diagnostic REMOVE BEFORE MERGE. Report the package sources that the pipeline agent resolves from inside the broker clone. No local test can answer whether the agent turns off the source named nuget.org, which feeds the agent allows, or whether the explicit config narrows the list on a real agent. The block prints and does nothing else. It restores the error preference around itself, so it cannot fail the run. --- sdk/core/azure_core_amqp/Test-Setup.ps1 | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index f711bc0ab7..244b9a1373 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -271,6 +271,29 @@ try { exit 1 } + # TEMPORARY DIAGNOSTIC. REMOVE BEFORE MERGE. Tracking: Azure/azure-amqp#318. + # + # Report the package sources that this agent resolves from inside the clone. The build does + # not use this information, and this block cannot fail the run. It answers three questions + # that no local test can answer: whether the agent turns off the source named nuget.org, + # which feeds the agent allows, and whether the explicit config narrows the list as expected. + $diagnosticPreference = $ErrorActionPreference + $ErrorActionPreference = "Continue" + try { + Push-Location -Path $repositoryDir + try { + Write-Host "DIAG| ---- sources with no --configfile (agent policy plus root nuget.config) ----" + (& dotnet nuget list source) 2>&1 | ForEach-Object { Write-Host "DIAG| $_" } + Write-Host "DIAG| ---- sources with --configfile nuget.cfsclean.config ----" + (& dotnet nuget list source --configfile ./nuget.cfsclean.config) 2>&1 | ForEach-Object { Write-Host "DIAG| $_" } + Write-Host "DIAG| ---- root nuget.config of the pinned clone ----" + (Get-Content ./nuget.config) | ForEach-Object { Write-Host "DIAG| $_" } + } + finally { Pop-Location } + } + catch { Write-Host "DIAG| the diagnostic did not run: $_" } + $ErrorActionPreference = $diagnosticPreference + # Run the restore and the build from the clone root. This Push-Location is # load-bearing: the dotnet command reads global.json from the current # directory and not from the project directory, and the arguments above are From 2473de2a96e647f957b605c8e989dc0eb8027d4e Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 15:04:47 -0400 Subject: [PATCH 07/14] Revert "chore(amqp): add a temporary NuGet source diagnostic" This reverts commit 00a8c230c75c8a532a630af2d18135531c11558b. --- sdk/core/azure_core_amqp/Test-Setup.ps1 | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index 244b9a1373..f711bc0ab7 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -271,29 +271,6 @@ try { exit 1 } - # TEMPORARY DIAGNOSTIC. REMOVE BEFORE MERGE. Tracking: Azure/azure-amqp#318. - # - # Report the package sources that this agent resolves from inside the clone. The build does - # not use this information, and this block cannot fail the run. It answers three questions - # that no local test can answer: whether the agent turns off the source named nuget.org, - # which feeds the agent allows, and whether the explicit config narrows the list as expected. - $diagnosticPreference = $ErrorActionPreference - $ErrorActionPreference = "Continue" - try { - Push-Location -Path $repositoryDir - try { - Write-Host "DIAG| ---- sources with no --configfile (agent policy plus root nuget.config) ----" - (& dotnet nuget list source) 2>&1 | ForEach-Object { Write-Host "DIAG| $_" } - Write-Host "DIAG| ---- sources with --configfile nuget.cfsclean.config ----" - (& dotnet nuget list source --configfile ./nuget.cfsclean.config) 2>&1 | ForEach-Object { Write-Host "DIAG| $_" } - Write-Host "DIAG| ---- root nuget.config of the pinned clone ----" - (Get-Content ./nuget.config) | ForEach-Object { Write-Host "DIAG| $_" } - } - finally { Pop-Location } - } - catch { Write-Host "DIAG| the diagnostic did not run: $_" } - $ErrorActionPreference = $diagnosticPreference - # Run the restore and the build from the clone root. This Push-Location is # load-bearing: the dotnet command reads global.json from the current # directory and not from the project directory, and the arguments above are From 6208a9ecc9306b6f6c6cbbe1ccbcbeba8c5fc2a9 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 15:52:46 -0400 Subject: [PATCH 08/14] test(amqp): own the broker restore config in this repository The restricted feed policy is this pipeline's requirement, and not a requirement of Azure/azure-amqp. The configuration that satisfies a policy belongs with the code that enforces the policy, so this repository now carries its own nuget.cfsclean.config next to the setup script, and the restore passes that file. Drop the check for a restore config inside the broker clone, and check the copy in this repository instead. The pinned commit no longer has to contain a restore config, so the pin can move to any commit that builds the broker. --- sdk/core/azure_core_amqp/Test-Setup.ps1 | 11 +++++--- .../azure_core_amqp/nuget.cfsclean.config | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 sdk/core/azure_core_amqp/nuget.cfsclean.config diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index f711bc0ab7..2afa09eece 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -259,15 +259,18 @@ try { # The dotnet arguments below are relative to the clone root. Keep the # absolute forms only for the checks. $brokerProjectRelative = [System.IO.Path]::Combine("test", "TestAmqpBroker", "TestAmqpBroker.csproj") - $nugetConfigRelative = "nuget.cfsclean.config" $brokerProject = [System.IO.Path]::Combine($repositoryDir, $brokerProjectRelative) - $nugetConfig = [System.IO.Path]::Combine($repositoryDir, $nugetConfigRelative) if (!(Test-Path $brokerProject)) { LogError "The pinned azure-amqp commit does not contain $brokerProject." exit 1 } + + # The restore config belongs to this repository, and not to the broker clone. The restricted + # feed policy is this pipeline's requirement, so the file that satisfies it sits next to this + # script. Pass an absolute path, because the dotnet calls run from the clone root. + $nugetConfig = [System.IO.Path]::Combine($PSScriptRoot, "nuget.cfsclean.config") if (!(Test-Path $nugetConfig)) { - LogError "The pinned azure-amqp commit does not contain $nugetConfig." + LogError "This repository does not contain $nugetConfig." exit 1 } @@ -279,7 +282,7 @@ try { Push-Location -Path $repositoryDir try { Invoke-LoggedCommand ` - "dotnet restore `"$brokerProjectRelative`" --configfile `"$nugetConfigRelative`"" ` + "dotnet restore `"$brokerProjectRelative`" --configfile `"$nugetConfig`"" ` -GroupOutput Invoke-LoggedCommand ` "dotnet build `"$brokerProjectRelative`" --configuration Debug --framework net10.0 --no-restore" ` diff --git a/sdk/core/azure_core_amqp/nuget.cfsclean.config b/sdk/core/azure_core_amqp/nuget.cfsclean.config new file mode 100644 index 0000000000..e5bea07326 --- /dev/null +++ b/sdk/core/azure_core_amqp/nuget.cfsclean.config @@ -0,0 +1,26 @@ + + + + + + + + + + + From 644aa12e1e1f9b4b6b3dbfb41e48846521a7cca1 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 15:57:32 -0400 Subject: [PATCH 09/14] test(amqp): pin the broker to azure-amqp master Azure/azure-amqp pull request 318 closed without a merge, so its head commit is not on any branch and can disappear. This repository now owns the restore config, so the pin no longer has to reach a commit that carries one, and any commit that builds the broker works. Pin the head of master. The reachability check stays quiet, because that commit is reachable from master. --- sdk/core/azure_core_amqp/README.md | 8 +++----- sdk/core/azure_core_amqp/Test-Setup.ps1 | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index 24e8543060..9a1280bc92 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -32,9 +32,7 @@ Install [PowerShell](https://learn.microsoft.com/powershell/scripting/install/in ./sdk/core/azure_core_amqp/Test-Setup.ps1 ``` -The script clones Azure/azure-amqp at commit [`239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a`](https://github.com/Azure/azure-amqp/commit/239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a), restores through the broker repository's `nuget.cfsclean.config`, builds `TestAmqpBroker` for .NET 10, and launches it in the background. The config was added in [Azure/azure-amqp#318](https://github.com/Azure/azure-amqp/pull/318). - -Run the package tests in the same PowerShell process so `TEST_BROKER_ADDRESS` remains available. +The script clones Azure/azure-amqp at commit [`111de654e170de3ab6cefe150043458c67b6660d`](https://github.com/Azure/azure-amqp/commit/111de654e170de3ab6cefe150043458c67b6660d), restores through the `nuget.cfsclean.config` in this package, builds `TestAmqpBroker` for .NET 10, and launches it in the background. Run the package tests in the same PowerShell process so `TEST_BROKER_ADDRESS` remains available. ```pwsh cargo test --package azure_core_amqp --all-features @@ -58,7 +56,7 @@ $env:TEST_BROKER_COMMIT = '' Setup also asks the GitHub compare API whether the pinned commit is reachable from azure-amqp `master`. A reachable pin says nothing. An unreachable pin writes a warning, and `TEST_BROKER_REQUIRE_MERGED` turns that warning into an error. A check that could not run writes a warning and always continues, because the unauthenticated rate limit is 60 requests each hour for each IP address. -The current pin is the head of the pull request [Azure/azure-amqp#318](https://github.com/Azure/azure-amqp/pull/318), so the warning appears today. It will still appear after that pull request merges. azure-amqp squash-merges, so the head commit of a pull request never lands on `master`. To clear the warning, move the pin to the squash commit on `master`, which is the `merge_commit_sha` of the merged pull request. Do not use the `merge_commit_sha` of an open pull request, because that is a throwaway test-merge commit that disappears. +The current pin is `111de654e170de3ab6cefe150043458c67b6660d`, the head of `master` in Azure/azure-amqp, so no warning appears. Azure/azure-amqp squash-merges its pull requests, so the commit that lands on `master` is the `merge_commit_sha` of a merged pull request and never the head commit of that pull request. ### Manual broker install @@ -66,7 +64,7 @@ Clone the pinned azure-amqp commit to a local directory. ```pwsh cd -git clone https://github.com/Azure/azure-amqp --revision 239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a +git clone https://github.com/Azure/azure-amqp --revision 111de654e170de3ab6cefe150043458c67b6660d ``` Normal external developer builds use the repository's standard NuGet configuration. diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index 2afa09eece..8a551832a4 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -186,10 +186,8 @@ try { # azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer # can move a tag to a different commit without a trace. # - # This SHA is the commit of Azure/azure-amqp pull request 318 that adds - # nuget.cfsclean.config. That pull request has moved on since, so the SHA is - # now an ancestor of refs/pull/318/head and not the head itself. It is not on - # master, so the reachability check below writes a warning and continues. + # This SHA is the head of master in Azure/azure-amqp. The reachability check + # below stays quiet while the pin sits on master. # # To update the pin: # 1. Pick an azure-amqp commit that contains nuget.cfsclean.config and @@ -203,7 +201,7 @@ try { # # Set TEST_BROKER_COMMIT to point the broker at a different commit without a # code change. - $repositoryHash = "239aff0d87b2c19e1fa91636e0fc0f6ee6e9999a" + $repositoryHash = "111de654e170de3ab6cefe150043458c67b6660d" if (-not [string]::IsNullOrWhiteSpace($env:TEST_BROKER_COMMIT)) { $repositoryHash = $env:TEST_BROKER_COMMIT.Trim() Write-Host "TEST_BROKER_COMMIT overrides the pinned azure-amqp commit: $repositoryHash" From 87bd8ead0f0441472305134b44a37d2cf52bf792 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 16:48:53 -0400 Subject: [PATCH 10/14] chore(amqp): reword a comment that the spell check rejected The cspell step failed on nuget.cfsclean.config with "Unknown word (configfile)". Say the same thing in plain words instead of adding a dictionary entry. --- sdk/core/azure_core_amqp/nuget.cfsclean.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure_core_amqp/nuget.cfsclean.config b/sdk/core/azure_core_amqp/nuget.cfsclean.config index e5bea07326..718d62ec0f 100644 --- a/sdk/core/azure_core_amqp/nuget.cfsclean.config +++ b/sdk/core/azure_core_amqp/nuget.cfsclean.config @@ -4,7 +4,7 @@ Test-Setup.ps1 clones Azure/azure-amqp and builds TestAmqpBroker from source. This pipeline must not restore from a direct public endpoint, so the restore - passes this file through the configfile option. + names this file explicitly. The clear entries are the point of this file. Without them NuGet merges the sources it finds in parent directories and in the user and machine From 4421517315b1ff7a0ce92e7a0173a8388ff665ff Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 17:16:11 -0400 Subject: [PATCH 11/14] test(amqp): cover the broker skip rule with real tests The helper had one test, and it covered an absent variable that no rule depends on. The behavior that keeps a silent skip from returning was untested. Move the rule into resolve_broker_address, which reads no environment variable, and test every branch: a present address comes back, an absent address skips, and an absent address panics when the run requires a broker. Cover the documented flag values too. Correct the pin update steps and the README. They said the pinned commit must contain nuget.cfsclean.config. This package owns that file now, so any commit that builds the broker works. --- sdk/core/azure_core_amqp/README.md | 4 +- sdk/core/azure_core_amqp/Test-Setup.ps1 | 5 +- sdk/core/azure_core_amqp/src/test_broker.rs | 79 ++++++++++++++++++--- 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index 9a1280bc92..7d5ce1cafb 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -46,7 +46,7 @@ Stop the broker after the tests finish. #### Updating the broker pin -Update the pin only to an azure-amqp commit that contains `nuget.cfsclean.config` and builds `TestAmqpBroker` for `net10.0`. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, update the same SHA in this file, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. +Update the pin to any azure-amqp commit that builds `TestAmqpBroker` for `net10.0`. The commit does not need to carry a restore configuration, because this package owns that file. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, update the same SHA in this file, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. Set `TEST_BROKER_COMMIT` to try a different commit without a code change. @@ -77,7 +77,7 @@ dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug - CFSClean builds restore from the `azure-sdk-for-net` Azure Artifacts feed. The feed is public and answers anonymous reads, so an external developer needs no credentials to restore a package that the feed has already cached. The CFSClean environment supplies credentials because a cache miss makes the feed fetch the package from upstream, and that fetch needs an authenticated caller. Run this restore and build sequence from the clone root. ```pwsh -dotnet restore .\test\TestAmqpBroker\TestAmqpBroker.csproj --configfile .\nuget.cfsclean.config +dotnet restore .\test\TestAmqpBroker\TestAmqpBroker.csproj --configfile \nuget.cfsclean.config dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug --framework net10.0 --no-restore ``` diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index 8a551832a4..71c3894c44 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -190,8 +190,9 @@ try { # below stays quiet while the pin sits on master. # # To update the pin: - # 1. Pick an azure-amqp commit that contains nuget.cfsclean.config and - # builds TestAmqpBroker for net10.0. + # 1. Pick an azure-amqp commit that builds TestAmqpBroker for net10.0. + # The commit does not need to carry a restore configuration. This + # package owns that file, and the restore names it by absolute path. # 2. Put the full 40-character SHA of that commit below, and update the # comment above with the ref that the SHA comes from. For a merged # pull request, use the merge_commit_sha, not the head SHA. diff --git a/sdk/core/azure_core_amqp/src/test_broker.rs b/sdk/core/azure_core_amqp/src/test_broker.rs index 84a0ade7f5..8f2d5047ce 100644 --- a/sdk/core/azure_core_amqp/src/test_broker.rs +++ b/sdk/core/azure_core_amqp/src/test_broker.rs @@ -32,16 +32,32 @@ const TEST_BROKER_REQUIRED: &str = "TEST_BROKER_REQUIRED"; /// Panics when `TEST_BROKER_REQUIRED` is on and the broker address is absent or /// empty. pub(crate) fn test_broker_address() -> Option { - let address = std::env::var(TEST_BROKER_ADDRESS) + resolve_broker_address(read_broker_address(), is_env_flag_set(TEST_BROKER_REQUIRED)) +} + +/// Reads and normalizes the broker address. An absent or blank value is `None`. +fn read_broker_address() -> Option { + std::env::var(TEST_BROKER_ADDRESS) .ok() .map(|value| value.trim().to_string()) - .filter(|value| !value.is_empty()); + .filter(|value| !value.is_empty()) +} + +/// Decides what a broker test does, given the address and the strictness flag. +/// +/// This holds the whole rule, and it reads no environment variable, so the tests +/// below can cover every branch without a shared global. +/// +/// # Panics +/// +/// Panics when `required` is true and `address` is `None`. +fn resolve_broker_address(address: Option, required: bool) -> Option { if address.is_some() { return address; } assert!( - !is_env_flag_set(TEST_BROKER_REQUIRED), + !required, "{TEST_BROKER_REQUIRED} is set, but {TEST_BROKER_ADDRESS} is absent or empty. \ Start the broker with sdk/core/azure_core_amqp/Test-Setup.ps1 and run the tests in \ the same shell, or clear {TEST_BROKER_REQUIRED} to skip the broker tests." @@ -55,20 +71,65 @@ pub(crate) fn test_broker_address() -> Option { /// `0`, or `false`. fn is_env_flag_set(name: &str) -> bool { match std::env::var(name) { - Ok(value) => { - let value = value.trim(); - !value.is_empty() && value != "0" && !value.eq_ignore_ascii_case("false") - } + Ok(value) => is_flag_value_set(&value), Err(_) => false, } } +/// Reads one flag value. Kept separate from the environment so it can be tested. +fn is_flag_value_set(value: &str) -> bool { + let value = value.trim(); + !value.is_empty() && value != "0" && !value.eq_ignore_ascii_case("false") +} + #[cfg(test)] mod tests { - use super::is_env_flag_set; + use super::{is_env_flag_set, is_flag_value_set, resolve_broker_address}; + + #[test] + fn address_present_is_returned() { + let address = Some("amqp://127.0.0.1:25672".to_string()); + assert_eq!( + resolve_broker_address(address.clone(), false), + address, + "a present address must come back unchanged" + ); + assert_eq!( + resolve_broker_address(address.clone(), true), + address, + "the strictness flag must not change a present address" + ); + } + + #[test] + fn address_absent_and_not_required_skips() { + assert_eq!( + resolve_broker_address(None, false), + None, + "a developer without a broker must still run the other tests" + ); + } + + #[test] + #[should_panic(expected = "is absent or empty")] + fn address_absent_and_required_panics() { + // This is the behavior that keeps a silent skip from returning. A broker that + // stops running must turn the pipeline red. + let _ = resolve_broker_address(None, true); + } + + #[test] + fn flag_values_follow_the_documented_rule() { + for off in ["", " ", "0", "false", "FALSE", "False"] { + assert!(!is_flag_value_set(off), "{off:?} must read as off"); + } + for on in ["1", "true", "TRUE", "yes", "on"] { + assert!(is_flag_value_set(on), "{on:?} must read as on"); + } + } #[test] - fn env_flag_is_off_when_absent() { + fn absent_variable_is_off() { assert!(!is_env_flag_set("AZURE_CORE_AMQP_FLAG_THAT_IS_NEVER_SET")); } } From 90b985c4dc90623c9b6ffe94d66747b857120918 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Mon, 10 Aug 2026 21:31:36 -0400 Subject: [PATCH 12/14] test(amqp): set the broker flag from this package, not a shared template TEST_BROKER_REQUIRED sat in eng/pipelines/templates/jobs/ci.tests.yml, which every package uses for its tests. The flag belongs to this package, so the template no longer carries it and this change leaves that file untouched. Test-Setup.ps1 sets the flag instead, right after the macOS guard. That placement gives the macOS behavior for free, because the script exits before this line on macOS and the flag never gets set there. The script sets the process variable and also emits the pipeline logging command, so the value reaches a later step as well. Move the spelling words into .cspell.json for this package, and drop the inline cspell directives from the README and the two scripts. --- eng/pipelines/templates/jobs/ci.tests.yml | 6 ------ sdk/core/azure_core_amqp/.cspell.json | 7 ++++++- sdk/core/azure_core_amqp/README.md | 1 - sdk/core/azure_core_amqp/Test-Cleanup.ps1 | 1 - sdk/core/azure_core_amqp/Test-Setup.ps1 | 8 +++++++- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index e9bf4d4435..024ed41565 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -78,12 +78,6 @@ jobs: timeoutInMinutes: ${{ parameters.TimeoutInMinutes }} env: CIBW_BUILD_VERBOSITY: 3 - # The azure_core_amqp broker tests skip when TEST_BROKER_ADDRESS is absent. - # In CI they must fail instead, so a broker that stops running turns the - # build red. Test-Setup.ps1 exits early on macOS and never starts the - # broker, so macOS keeps the skip behavior. - ${{ if ne(parameters.OSName, 'macOS') }}: - TEST_BROKER_REQUIRED: 'true' inputs: pwsh: true filePath: $(Build.SourcesDirectory)/eng/scripts/Test-Packages.ps1 diff --git a/sdk/core/azure_core_amqp/.cspell.json b/sdk/core/azure_core_amqp/.cspell.json index eb2c70bd2e..07447291eb 100644 --- a/sdk/core/azure_core_amqp/.cspell.json +++ b/sdk/core/azure_core_amqp/.cspell.json @@ -4,10 +4,15 @@ ], "ignoreWords": [ "amqps", + "cfsclean", + "configfile", + "depsfile", + "JOBID", "mgmt", "sasl", "sastoken", + "setvariable", "smalluint", "smallulong" ] -} \ No newline at end of file +} diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index 7d5ce1cafb..c73efa871f 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -1,4 +1,3 @@ - # Azure AMQP library for Rust diff --git a/sdk/core/azure_core_amqp/Test-Cleanup.ps1 b/sdk/core/azure_core_amqp/Test-Cleanup.ps1 index 811aec6c58..249adb8763 100644 --- a/sdk/core/azure_core_amqp/Test-Cleanup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Cleanup.ps1 @@ -1,6 +1,5 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -# cspell: ignore JOBID . "$PSScriptRoot\..\..\..\eng\common\scripts\common.ps1" diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index 71c3894c44..ae0fa4ad0c 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -1,6 +1,5 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -# cspell: ignore JOBID cfsclean configfile depsfile # Load common ES scripts @@ -148,6 +147,13 @@ if ($IsMacOS) { exit 0 } +# This script starts a broker, so a broker test that finds no address is a fault and not a +# reason to skip. Set the flag here, and not in a pipeline template, because the flag belongs +# to this package and a template serves every package. macOS never reaches this line, so macOS +# keeps the skip behavior. +$env:TEST_BROKER_REQUIRED = "true" +Write-Host "##vso[task.setvariable variable=TEST_BROKER_REQUIRED]true" + # Create the test binary *outside* the repo root to avoid polluting the repo. $WorkingDirectory = [System.IO.Path]::Combine($RepoRoot, "../TestArtifacts") From 44ce2b27e80874eadf268262adf4e29345a204df Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Tue, 11 Aug 2026 09:14:33 -0400 Subject: [PATCH 13/14] test(amqp): restore through the existing repository NuGet template The package carried its own nuget.cfsclean.config, and that file was a copy of eng/templates/NuGet.config.template, which the pipeline already uses through eng/pipelines/templates/steps/nuget-config.yml. Two files with the same contents drift apart. Delete the copy and name the template on the restore. The restore still has to name a configuration. The broker clone carries its own nuget.config that adds NuGet.org, and a directory level file wins over the user level one that the pipeline writes. Keep the pinned commit in Test-Setup.ps1 alone. README.md repeated the SHA, so an update needed two edits and could leave the two out of step. The README now points at the script. --- sdk/core/azure_core_amqp/.cspell.json | 1 - sdk/core/azure_core_amqp/README.md | 10 +++---- sdk/core/azure_core_amqp/Test-Setup.ps1 | 15 ++++++----- .../azure_core_amqp/nuget.cfsclean.config | 26 ------------------- 4 files changed, 14 insertions(+), 38 deletions(-) delete mode 100644 sdk/core/azure_core_amqp/nuget.cfsclean.config diff --git a/sdk/core/azure_core_amqp/.cspell.json b/sdk/core/azure_core_amqp/.cspell.json index 07447291eb..6368491656 100644 --- a/sdk/core/azure_core_amqp/.cspell.json +++ b/sdk/core/azure_core_amqp/.cspell.json @@ -6,7 +6,6 @@ "amqps", "cfsclean", "configfile", - "depsfile", "JOBID", "mgmt", "sasl", diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index c73efa871f..55b0c24f57 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -31,7 +31,7 @@ Install [PowerShell](https://learn.microsoft.com/powershell/scripting/install/in ./sdk/core/azure_core_amqp/Test-Setup.ps1 ``` -The script clones Azure/azure-amqp at commit [`111de654e170de3ab6cefe150043458c67b6660d`](https://github.com/Azure/azure-amqp/commit/111de654e170de3ab6cefe150043458c67b6660d), restores through the `nuget.cfsclean.config` in this package, builds `TestAmqpBroker` for .NET 10, and launches it in the background. Run the package tests in the same PowerShell process so `TEST_BROKER_ADDRESS` remains available. +The script clones Azure/azure-amqp at the commit that `Test-Setup.ps1` pins, restores through `eng/templates/NuGet.config.template`, builds `TestAmqpBroker` for .NET 10, and launches it in the background. Run the package tests in the same PowerShell process so `TEST_BROKER_ADDRESS` remains available. ```pwsh cargo test --package azure_core_amqp --all-features @@ -45,7 +45,7 @@ Stop the broker after the tests finish. #### Updating the broker pin -Update the pin to any azure-amqp commit that builds `TestAmqpBroker` for `net10.0`. The commit does not need to carry a restore configuration, because this package owns that file. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, update the same SHA in this file, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. +Update the pin to any azure-amqp commit that builds `TestAmqpBroker` for `net10.0`. The commit does not need to carry a restore configuration. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, update the same SHA in this file, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. Set `TEST_BROKER_COMMIT` to try a different commit without a code change. @@ -55,7 +55,7 @@ $env:TEST_BROKER_COMMIT = '' Setup also asks the GitHub compare API whether the pinned commit is reachable from azure-amqp `master`. A reachable pin says nothing. An unreachable pin writes a warning, and `TEST_BROKER_REQUIRE_MERGED` turns that warning into an error. A check that could not run writes a warning and always continues, because the unauthenticated rate limit is 60 requests each hour for each IP address. -The current pin is `111de654e170de3ab6cefe150043458c67b6660d`, the head of `master` in Azure/azure-amqp, so no warning appears. Azure/azure-amqp squash-merges its pull requests, so the commit that lands on `master` is the `merge_commit_sha` of a merged pull request and never the head commit of that pull request. +`Test-Setup.ps1` holds the pin, and it is the only place that needs an update. The pin sits on `master` in Azure/azure-amqp today, so no warning appears. Azure/azure-amqp squash-merges its pull requests, so the commit that lands on `master` is the `merge_commit_sha` of a merged pull request and never the head commit of that pull request. ### Manual broker install @@ -63,7 +63,7 @@ Clone the pinned azure-amqp commit to a local directory. ```pwsh cd -git clone https://github.com/Azure/azure-amqp --revision 111de654e170de3ab6cefe150043458c67b6660d +git clone https://github.com/Azure/azure-amqp --revision ``` Normal external developer builds use the repository's standard NuGet configuration. @@ -76,7 +76,7 @@ dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug - CFSClean builds restore from the `azure-sdk-for-net` Azure Artifacts feed. The feed is public and answers anonymous reads, so an external developer needs no credentials to restore a package that the feed has already cached. The CFSClean environment supplies credentials because a cache miss makes the feed fetch the package from upstream, and that fetch needs an authenticated caller. Run this restore and build sequence from the clone root. ```pwsh -dotnet restore .\test\TestAmqpBroker\TestAmqpBroker.csproj --configfile \nuget.cfsclean.config +dotnet restore .\test\TestAmqpBroker\TestAmqpBroker.csproj --configfile \eng\templates\NuGet.config.template dotnet build .\test\TestAmqpBroker\TestAmqpBroker.csproj --configuration Debug --framework net10.0 --no-restore ``` diff --git a/sdk/core/azure_core_amqp/Test-Setup.ps1 b/sdk/core/azure_core_amqp/Test-Setup.ps1 index ae0fa4ad0c..848798e9f8 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -202,10 +202,11 @@ try { # 2. Put the full 40-character SHA of that commit below, and update the # comment above with the ref that the SHA comes from. For a merged # pull request, use the merge_commit_sha, not the head SHA. - # 3. Update the same SHA in sdk/core/azure_core_amqp/README.md. - # 4. Run this script and then Test-Cleanup.ps1. Make sure that setup + # 3. Run this script and then Test-Cleanup.ps1. Make sure that setup # reports a clean azure-amqp clone. # + # This line is the only place that holds the pin. README.md points here. + # # Set TEST_BROKER_COMMIT to point the broker at a different commit without a # code change. $repositoryHash = "111de654e170de3ab6cefe150043458c67b6660d" @@ -270,10 +271,12 @@ try { exit 1 } - # The restore config belongs to this repository, and not to the broker clone. The restricted - # feed policy is this pipeline's requirement, so the file that satisfies it sits next to this - # script. Pass an absolute path, because the dotnet calls run from the clone root. - $nugetConfig = [System.IO.Path]::Combine($PSScriptRoot, "nuget.cfsclean.config") + # Restore through the same feed configuration that the pipeline uses, at + # eng/templates/NuGet.config.template. The broker clone carries its own nuget.config that + # adds NuGet.org, and a directory level file wins over the user level one, so the restore + # has to name a configuration explicitly. Pass an absolute path, because the dotnet calls + # run from the clone root. + $nugetConfig = [System.IO.Path]::Combine($RepoRoot, "eng", "templates", "NuGet.config.template") if (!(Test-Path $nugetConfig)) { LogError "This repository does not contain $nugetConfig." exit 1 diff --git a/sdk/core/azure_core_amqp/nuget.cfsclean.config b/sdk/core/azure_core_amqp/nuget.cfsclean.config deleted file mode 100644 index 718d62ec0f..0000000000 --- a/sdk/core/azure_core_amqp/nuget.cfsclean.config +++ /dev/null @@ -1,26 +0,0 @@ - - - - - - - - - - - From c28ddf975933bb944690872bf3eaba0121b79510 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Tue, 11 Aug 2026 10:26:23 -0400 Subject: [PATCH 14/14] docs(amqp): drop the stale second home for the broker pin The pin update steps still told the reader to write the SHA into README.md. Test-Setup.ps1 is the only place that holds the pin now, and the paragraph below the steps already says so. The two statements disagreed. --- sdk/core/azure_core_amqp/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure_core_amqp/README.md b/sdk/core/azure_core_amqp/README.md index 55b0c24f57..faa37a43e8 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -45,7 +45,7 @@ Stop the broker after the tests finish. #### Updating the broker pin -Update the pin to any azure-amqp commit that builds `TestAmqpBroker` for `net10.0`. The commit does not need to carry a restore configuration. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, update the same SHA in this file, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. +Update the pin to any azure-amqp commit that builds `TestAmqpBroker` for `net10.0`. The commit does not need to carry a restore configuration. Change `$repositoryHash` in `Test-Setup.ps1` to the full 40-character SHA, run the setup and cleanup scripts, and make sure that setup reports a clean azure-amqp clone. The pin stays a bare SHA. A tag is not safe here, because azure-amqp uses lightweight tags and has no tag ruleset, so a maintainer can move a tag to a different commit without a trace. Set `TEST_BROKER_COMMIT` to try a different commit without a code change.