diff --git a/sdk/core/azure_core_amqp/.cspell.json b/sdk/core/azure_core_amqp/.cspell.json index eb2c70bd2e..6368491656 100644 --- a/sdk/core/azure_core_amqp/.cspell.json +++ b/sdk/core/azure_core_amqp/.cspell.json @@ -4,10 +4,14 @@ ], "ignoreWords": [ "amqps", + "cfsclean", + "configfile", + "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 9133de3575..faa37a43e8 100644 --- a/sdk/core/azure_core_amqp/README.md +++ b/sdk/core/azure_core_amqp/README.md @@ -1,3 +1,4 @@ + # Azure AMQP library for Rust Azure AMQP crate for consumption of AMQP based packages in the Azure SDK for Rust and C++. @@ -14,61 +15,76 @@ 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. + +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. -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), 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 ``` -This will download the TestAmqpBroker, build it and launch the executable in the background. +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. -Note that this requires that you have the [.NET SDK](https://dot.net/download) installed on your machine. - -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 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. -For Manual testing, first clone the azure-amqp repository to a local directory: +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. + +`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 + +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 ``` -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 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 -$env:TEST_BROKER_ADDRESS = 'amqp://127.0.0.1:25672' +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 ``` -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..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" @@ -9,12 +8,13 @@ if ($IsMacOS) { exit 0 } -if ($true) { - Write-Host "AMQP Test Broker tests disabled until test broker is updated." +# 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 fa793eddd2..848798e9f8 100644 --- a/sdk/core/azure_core_amqp/Test-Setup.ps1 +++ b/sdk/core/azure_core_amqp/Test-Setup.ps1 @@ -1,20 +1,158 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -# cspell: ignore JOBID depsfile # Load common ES scripts . "$PSScriptRoot\..\..\..\eng\common\scripts\common.ps1" +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 + } + + # 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 { + $connectTask = $client.ConnectAsync($HostName, $Port) + if ($connectTask.Wait($remainingMilliseconds) -and $client.Connected) { + return $true + } + } + catch { + # 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 + } + + # 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 { + 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 ($IsMacOS) { Write-Host "AMQP tests are not supported on macOS. Skipping test setup." exit 0 } -if ($true) { - Write-Host "AMQP Test Broker tests disabled until test broker is updated." - 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") @@ -30,6 +168,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 { @@ -39,25 +184,127 @@ try { Remove-Item $repositoryDir -Force -Recurse | Out-Null } - $repositoryUrl = "https://github.com/Azure/azure-amqp.git" - $repositoryHash = "d82a86455c3459c5628bc95b25511f6e8a065598" - $cloneCommand = "git clone $repositoryUrl --revision $repositoryHash --depth=1" + $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 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 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. + # 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" + 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 - Set-Location -Path "./azure-amqp/test/TestAmqpBroker" + # 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 + } + + # 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") + $brokerProject = [System.IO.Path]::Combine($repositoryDir, $brokerProjectRelative) + if (!(Test-Path $brokerProject)) { + LogError "The pinned azure-amqp commit does not contain $brokerProject." + exit 1 + } - Invoke-LoggedCommand "dotnet build --framework net8.0" - if (-not $?) { - Write-Error "Failed to build TestAmqpBroker." + # 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 } + # 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 `"$nugetConfig`"" ` + -GroupOutput + Invoke-LoggedCommand ` + "dotnet build `"$brokerProjectRelative`" --configuration Debug --framework net10.0 --no-restore" ` + -GroupOutput + } + finally { + Pop-Location + } + 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,23 +312,52 @@ 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 & + $brokerJobId = $job.Id $env:TEST_BROKER_JOBID = $job.Id - Write-Host "Waiting for test broker to start..." - Start-Sleep -Seconds 3 + 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 + LogError "Test broker did not become ready at ${env:TEST_BROKER_ADDRESS}." + exit 1 + } - Write-Host "Job Output after wait:" - Receive-Job $job.Id + Write-TestBrokerOutput -JobId $job.Id + Write-Host "Test broker is ready." - $job = Get-Job -Id $env:TEST_BROKER_JOBID - if ($job.State -ne "Running") { - Write-Host "Test broker failed to start." + $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 $_ } + 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..8f2d5047ce --- /dev/null +++ b/sdk/core/azure_core_amqp/src/test_broker.rs @@ -0,0 +1,135 @@ +// 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 { + 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()) +} + +/// 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!( + !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) => 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, 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 absent_variable_is_off() { + assert!(!is_env_flag_set("AZURE_CORE_AMQP_FLAG_THAT_IS_NEVER_SET")); + } +}