diff --git a/core/Azure.Mcp.Core/src/Services/Azure/BaseAzureService.cs b/core/Azure.Mcp.Core/src/Services/Azure/BaseAzureService.cs index 6e93b0c22d..7687afbf3e 100644 --- a/core/Azure.Mcp.Core/src/Services/Azure/BaseAzureService.cs +++ b/core/Azure.Mcp.Core/src/Services/Azure/BaseAzureService.cs @@ -47,7 +47,7 @@ static BaseAzureService() #if DEBUG if (EnvironmentHelpers.IsPlaybackTesting()) { - s_defaultPollInterval = TimeSpan.FromMilliseconds(1); + s_defaultPollInterval = TimeSpan.Zero; } #endif } @@ -310,7 +310,7 @@ protected static async Task WaitForLroCompletionAsync(Operation operation, if (s_defaultPollInterval.HasValue) { - await operation.WaitForCompletionAsync(s_defaultPollInterval.Value, cancellationToken); + await WaitForLroCompletionInternalAsync(operation, cancellationToken).ConfigureAwait(false); } else { @@ -330,11 +330,25 @@ protected static async Task WaitForLroCompletionAsync(Operation operation, Cance if (s_defaultPollInterval.HasValue) { - await operation.WaitForCompletionResponseAsync(s_defaultPollInterval.Value, cancellationToken); + await WaitForLroCompletionInternalAsync(operation, cancellationToken).ConfigureAwait(false); } else { await operation.WaitForCompletionResponseAsync(cancellationToken); } } + + private static async Task WaitForLroCompletionInternalAsync(Operation operation, CancellationToken cancellationToken) + { + while (true) + { + Response response = await operation.UpdateStatusAsync(cancellationToken); + if (operation.HasCompleted) + { + return operation.GetRawResponse(); + } + + await Task.Delay(s_defaultPollInterval!.Value, cancellationToken).ConfigureAwait(false); + } + } } diff --git a/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs b/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs index 138ab36c02..c81d049f02 100644 --- a/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs +++ b/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs @@ -1,9 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Reflection; using System.Text; using System.Text.Json; using System.Text.Json.Nodes; +using Microsoft.Mcp.Tests.Attributes; using Microsoft.Mcp.Tests.Client.Helpers; using Microsoft.Mcp.Tests.Helpers; using ModelContextProtocol.Client; @@ -121,6 +123,9 @@ protected virtual async ValueTask InitializeAsyncInternal(TestProxyFixture? prox { await LoadSettingsAsync(); + // Check that the test is allowed to run in the current mode + CheckLiveOnly(); + // Use custom arguments if provided, otherwise use standard mode (debug can be enabled via environment variable) var debugEnvVar = Environment.GetEnvironmentVariable("AZURE_MCP_TEST_DEBUG"); var enableDebug = string.Equals(debugEnvVar, "true", StringComparison.OrdinalIgnoreCase) || Settings.DebugOutput; @@ -233,6 +238,18 @@ protected virtual async ValueTask InitializeAsyncInternal(TestProxyFixture? prox return resultProcessor.Invoke(root); } + internal void CheckLiveOnly() + { + // resolve the current test method once for all attribute checks + var methodInfo = TestMethodResolver.TryResolveCurrentMethodInfo(); + + // skip tests marked [LiveTestOnly] when not in Live mode + if (TestMode != TestMode.Live && methodInfo?.GetCustomAttribute() != null) + { + Assert.Skip("Test is marked [LiveTestOnly] and cannot run in Playback or Record mode."); + } + } + public void Dispose() { Dispose(disposing: true); diff --git a/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/RecordedCommandTestsBase.cs b/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/RecordedCommandTestsBase.cs index a0a2d4ee95..76526978a1 100644 --- a/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/RecordedCommandTestsBase.cs +++ b/core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/RecordedCommandTestsBase.cs @@ -172,14 +172,8 @@ public override async ValueTask InitializeAsync() // load settings first to determine test mode await LoadSettingsAsync(); - // resolve the current test method once for all attribute checks - var methodInfo = TestMethodResolver.TryResolveCurrentMethodInfo(); - - // skip tests marked [LiveTestOnly] when not in Live mode - if (TestMode != TestMode.Live && HasLiveTestOnlyAttribute(methodInfo)) - { - Assert.Skip("Test is marked [LiveTestOnly] and cannot run in Playback or Record mode."); - } + // check if the test is allowed to run in the current mode + CheckLiveOnly(); if (fixture.Proxy == null) { @@ -194,7 +188,7 @@ public override async ValueTask InitializeAsync() await StartRecordOrPlayback(); // apply custom matcher if test has attribute - await ApplyAttributeMatcherSettings(methodInfo); + await ApplyAttributeMatcherSettings(TestMethodResolver.TryResolveCurrentMethodInfo()); SetRecordingOptions(RecordingOptions); } @@ -231,11 +225,6 @@ private async Task ApplyAttributeMatcherSettings(MethodInfo? methodInfo) await SetMatcher(matcher, RecordingId); } - private static bool HasLiveTestOnlyAttribute(MethodInfo? methodInfo) - { - return methodInfo?.GetCustomAttribute() != null; - } - private async Task SetMatcher(CustomDefaultMatcher matcher, string? recordingId = null) { if (Proxy == null) diff --git a/eng/pipelines/templates/jobs/build.yml b/eng/pipelines/templates/jobs/build.yml index 31bb40899e..88479b339a 100644 --- a/eng/pipelines/templates/jobs/build.yml +++ b/eng/pipelines/templates/jobs/build.yml @@ -73,29 +73,16 @@ jobs: AZURE_MCP_COLLECT_TELEMETRY: 'false' - task: Powershell@2 - displayName: "Run unit tests" + displayName: "Run tests" condition: and(succeeded(), eq(variables['RunUnitTests'], 'true')) timeoutInMinutes: ${{ parameters.TestTimeoutInMinutes }} inputs: pwsh: true filePath: $(Build.SourcesDirectory)/eng/scripts/Test-Code.ps1 arguments: > + -TestType 'All' -CollectCoverage:$${{ eq(parameters.OSName, 'linux') }} -TestResultsPath '$(Build.ArtifactStagingDirectory)/testResults' - workingDirectory: $(Build.SourcesDirectory) - env: - AZURE_MCP_COLLECT_TELEMETRY: 'false' - - - task: Powershell@2 - displayName: "Run recorded tests" - condition: and(succeeded(), eq(variables['RunRecordedTests'], 'true')) - timeoutInMinutes: ${{ parameters.TestTimeoutInMinutes }} - inputs: - pwsh: true - filePath: $(Build.SourcesDirectory)/eng/scripts/Test-Code.ps1 - arguments: > - -TestType 'Recorded' - -TestResultsPath '$(Build.ArtifactStagingDirectory)/recordedTestResults' -ScopingBuildInfoPath '$(Pipeline.Workspace)/build_info/build_info.json' workingDirectory: $(Build.SourcesDirectory) env: @@ -110,15 +97,6 @@ jobs: testResultsFormat: "VSTest" mergeTestResults: true - - task: PublishTestResults@2 - displayName: "Publish Recorded Test Results" - condition: and(succeededOrFailed(), eq(variables['RunRecordedTests'], 'true')) - inputs: - testResultsFiles: "$(Build.ArtifactStagingDirectory)/recordedTestResults/*.trx" - testRunTitle: "recorded_$(System.JobName)" - testResultsFormat: "VSTest" - mergeTestResults: true - - task: PublishCodeCoverageResults@2 displayName: "Publish Code Coverage Reports" condition: and(succeededOrFailed(), eq(variables['RunUnitTests'], 'true'), eq(variables['PublishCoverage'], 'true'), eq('${{ parameters.OSName }}', 'linux')) diff --git a/eng/scripts/New-BuildInfo.ps1 b/eng/scripts/New-BuildInfo.ps1 index f6dd7c3b92..0c6d3f4053 100644 --- a/eng/scripts/New-BuildInfo.ps1 +++ b/eng/scripts/New-BuildInfo.ps1 @@ -201,13 +201,6 @@ function Split-PropertyGroup { function Get-PathsToTest { Write-Host "Getting paths to test" - # When "core" is modified, include storage and keyVault as the canary service tools. - # TODO: These should be sourced from csproj files - $canaryPaths = @{ - "core/Azure.Mcp.Core"= @('tools/Azure.Mcp.Tools.Storage', 'tools/Azure.Mcp.Tools.KeyVault') - "core/Microsoft.Mcp.Core"= @('tools/Azure.Mcp.Tools.Storage', 'tools/Azure.Mcp.Tools.KeyVault') - } - # While there is a "core" directory at the repo root, we consider the "core" path to be all of the repo outside of the # "tools" directory. # This lets us make simple statements like: @@ -241,7 +234,6 @@ function Get-PathsToTest { # tools/Azure.Mcp.Tools.Storage # core/Fabric.Mcp.Core # servers/Azure.Mcp.Server - $projectDirectoryPattern = '^(tools|servers|core)/[^/]+' $normalizedPaths = $paths @@ -266,59 +258,62 @@ function Get-PathsToTest { # If we're in a pull request, use the set of changed files to narrow down the set of paths to test. $changedFiles = Get-ChangedFiles - # Assuming $changedFiles = [ - # tools/Azure.Mcp.Tools.Storage/src/someFile.cs <- "Azure.Mcp.Tools.Storage" - # tools/Azure.Mcp.Tools.Monitoring/README.md <- "Azure.Mcp.Tools.Monitoring" - # core/src/commonClass.cs <- "Core" - # eng/scripts/SomeScript.ps1 <- "Core" - # ] - Write-Host '' - - # Currently, we don't exclude non-code files from the changed files list. - # For example, updating a markdown file in a service path will still trigger tests for that path. - # Updating a file outside of the defined paths will be seen as a change to the core path. - $changedPaths = @($changedFiles - | Where-Object { $skipFiles -notcontains (Split-Path $_ -Leaf) } - | ForEach-Object { $_ -match $projectDirectoryPattern -and $normalizedPaths -contains $Matches[0] ? $Matches[0] : 'core/Microsoft.Mcp.Core' } - | Sort-Object -Unique) - - <# This makes $changedPaths = @( - 'tools/Azure.Mcp.Tools.Storage', - 'tools/Azure.Mcp.Tools.Monitoring', - 'core/Microsoft.Mcp.Core' - ) #> - - if($changedPaths.Count -eq 0) { - Write-Host "No changed, testable paths detected. Defaulting to core." -ForegroundColor Yellow - $changedPaths = @('core/Microsoft.Mcp.Core') + # Track whether engineering, the Core libraries, or shared build changed. If so, build everything. + $coreChanged = ($changedFiles | Where-Object { $_ -match '^core/(Azure|Fabric|Microsoft).Mcp.Core/src/' }).Count -gt 0 + $engChanged = ($changedFiles | Where-Object { $_ -match '^eng/' }).Count -gt 0 + $sharedBuildChanged = ($changedFiles | Where-Object { $_ -match '^Directory.(Build|Packages).props' }).Count -gt 0 + if ($coreChanged -or $engChanged -or $sharedBuildChanged) { + Write-Host "Core, engineering, or shared build changes detected. Building everything." -ForegroundColor Yellow + $pathsToTest = @() } else { - Write-Host "Changed paths detected: $($changedPaths -join ', ')" - } + # Assuming $changedFiles = [ + # tools/Azure.Mcp.Tools.Storage/src/someFile.cs <- "Azure.Mcp.Tools.Storage" + # tools/Azure.Mcp.Tools.Monitoring/README.md <- "Azure.Mcp.Tools.Monitoring" + # core/src/commonClass.cs <- "Core" + # eng/scripts/SomeScript.ps1 <- "Core" + # ] + Write-Host '' + + # Currently, we don't exclude non-code files from the changed files list. + # For example, updating a markdown file in a service path will still trigger tests for that path. + # Updating a file outside of the defined paths will be seen as a change to the core path. + $changedPaths = @($changedFiles + | Where-Object { $skipFiles -notcontains (Split-Path $_ -Leaf) } + | ForEach-Object { $_ -match $projectDirectoryPattern -and $normalizedPaths -contains $Matches[0] ? $Matches[0] : 'core/Microsoft.Mcp.Core' } + | Sort-Object -Unique) + + <# This makes $changedPaths = @( + 'tools/Azure.Mcp.Tools.Storage', + 'tools/Azure.Mcp.Tools.Monitoring', + 'core/Microsoft.Mcp.Core' + ) #> + + if($changedPaths.Count -eq 0) { + Write-Host "No changed, testable paths detected. Defaulting to core." -ForegroundColor Yellow + $changedPaths = @('core/Microsoft.Mcp.Core') + } else { + Write-Host "Changed paths detected: $($changedPaths -join ', ')" + } - $pathsToTest = $changedPaths - # If any affected path has "canaries", add them to the paths to test - foreach ($canaryKey in $canaryPaths.Keys) { - if($changedPaths -contains $canaryKey) { - $canaries = $canaryPaths[$canaryKey] - Write-Host "$canaryKey changes detected. Including canary paths: $($canaries -join ', ')" -ForegroundColor Cyan - $pathsToTest += $canaries + if ($pathsToTest -notcontains 'core/Microsoft.Mcp.Core') { + $pathsToTest = $changedPaths } - } - # Always include Azure.Mcp.Server to run ConsolidatedModeTests.cs in all PRs - if ($pathsToTest -notcontains 'servers/Azure.Mcp.Server') { - Write-Host "Adding servers/Azure.Mcp.Server to test paths for PR validation" -ForegroundColor Cyan - $pathsToTest += 'servers/Azure.Mcp.Server' - } + # Always include Azure.Mcp.Server to run ConsolidatedModeTests.cs in all PRs + if ($pathsToTest -notcontains 'servers/Azure.Mcp.Server') { + Write-Host "Adding servers/Azure.Mcp.Server to test paths for PR validation" -ForegroundColor Cyan + $pathsToTest += 'servers/Azure.Mcp.Server' + } - $normalizedPaths = @($pathsToTest | Sort-Object -Unique) + $normalizedPaths = @($pathsToTest | Sort-Object -Unique) - <# Making $paths = @( - 'tools/Azure.Mcp.Tools.Storage', - 'tools/Azure.Mcp.Tools.Monitoring', - 'core/Microsoft.Mcp.Core', - 'tools/Azure.Mcp.Tools.KeyVault' <-- from Microsoft.Mcp.Core's server canary list - ) #> + <# Making $paths = @( + 'tools/Azure.Mcp.Tools.Storage', + 'tools/Azure.Mcp.Tools.Monitoring', + 'core/Microsoft.Mcp.Core', + 'tools/Azure.Mcp.Tools.KeyVault' <-- from Microsoft.Mcp.Core's server canary list + ) #> + } } $pathsToTest = $normalizedPaths | ForEach-Object -ThrottleLimit 5 -Parallel { @@ -661,13 +656,12 @@ function Get-BuildMatrices { # we do not currently have a method to get an arm64 mac or windows agent at this time, so we will have to skip $runUnitTests for those platforms # if a set of unit tests exists, we should run them - $runUnitTests = !!($pathsToTest | Where-Object { $_.hasUnitTests }) + $runUnitTests = !!($pathsToTest | Where-Object { $_.hasUnitTests -or $_.hasRecordedTests }) # except for certain platforms if ($platform.native -or $platform.specialPurpose -or ($arch -like '*arm64*' -and $os -ne 'linux')) { $runUnitTests = $false } - $runRecordedTests = $runUnitTests -and ($pathsToTest | Where-Object { $_.hasRecordedTests } | Measure-Object | Select-Object -ExpandProperty Count) -gt 0 $publishCoverage = $runUnitTests -and -not ($arch -like '*arm64*') $hostArchitecture = if ($needsArm64Hardware) { 'Arm64' } else { '' } @@ -683,7 +677,6 @@ function Get-BuildMatrices { OSVmImage = $vmImage HostArchitecture = $hostArchitecture RunUnitTests = $runUnitTests - RunRecordedTests = $runRecordedTests PublishCoverage = $publishCoverage } diff --git a/eng/scripts/Test-Code.ps1 b/eng/scripts/Test-Code.ps1 index 4ce2e8e3bb..be55471271 100755 --- a/eng/scripts/Test-Code.ps1 +++ b/eng/scripts/Test-Code.ps1 @@ -54,6 +54,16 @@ function FilterTestProjects { Relative = (Resolve-Path -Path $_.FullName -Relative -RelativeBasePath $RepoRoot).Replace('\', '/').TrimStart('./') }} + # if provided a buildinfo, further scope the tests to only those impacted by changes + if ($BuildInfo){ + $changedPaths = $BuildInfo.pathsToTest | ForEach-Object { $_.path } + + $testProjects = $testProjects | Where-Object { + $testProjectPath = $_.Relative + ($changedPaths | Where-Object { $testProjectPath.StartsWith($_) }).Count -gt 0 + } + } + if ($testType -eq 'Recorded') { # until all LiveTest projects are migrated to recorded tests, we _must_ complete # an additional filter such that we'll only invoke those csprojs where playback is possible @@ -61,16 +71,6 @@ function FilterTestProjects { $projectDirectory = Split-Path -Path $_.FullName -Parent Test-Path -Path (Join-Path -Path $projectDirectory -ChildPath 'assets.json') } - - # if provided a buildinfo, further scope the recorded tests to only those impacted by changes - if ($BuildInfo){ - $changedPaths = $BuildInfo.pathsToTest | ForEach-Object { $_.path } - - $testProjects = $testProjects | Where-Object { - $testProjectPath = $_.Relative - ($changedPaths | Where-Object { $testProjectPath.StartsWith($_) }).Count -gt 0 - } - } } $normalizedPathFilters = $Paths ? ($Paths | ForEach-Object { "*$($_.Replace('\', '/'))*" }) : @()