Skip to content

Commit d5aa9a6

Browse files
authored
Improve live test playback running (#2333)
* Improve live test playback running * Add LRO helper method for playback * Fix linting * Add comment and fix cancellation behavior
1 parent a2472dd commit d5aa9a6

30 files changed

Lines changed: 480 additions & 380 deletions

File tree

core/Azure.Mcp.Core/src/Services/Azure/BaseAzureResourceService.cs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,9 @@ protected async Task<ArmClient> CreateArmClientWithApiVersionAsync(string resour
196196
/// <param name="cancellationToken">Cancellation token.</param>
197197
/// <returns>The <see cref="GenericResource"/> instance for the requested resource.</returns>
198198
/// <exception cref="ArgumentNullException">Thrown when a required parameter is null.</exception>
199-
protected async Task<GenericResource> GetGenericResourceAsync(ArmClient armClient, ResourceIdentifier resourceIdentifier, CancellationToken cancellationToken = default)
199+
protected static async Task<GenericResource> GetGenericResourceAsync(ArmClient armClient, ResourceIdentifier resourceIdentifier, CancellationToken cancellationToken = default)
200200
{
201-
if (armClient == null)
202-
throw new ArgumentNullException(nameof(armClient));
201+
ArgumentNullException.ThrowIfNull(armClient);
203202

204203
var genericResources = armClient.GetGenericResources();
205204
var response = await genericResources.GetAsync(resourceIdentifier, cancellationToken).ConfigureAwait(false);
@@ -224,10 +223,15 @@ protected async Task<GenericResource> GetGenericResourceAsync(ArmClient armClien
224223
/// <returns>The <see cref="GenericResource"/> instance for the requested resource.</returns>
225224
/// <exception cref="ArgumentNullException">Thrown when a required parameter is null.</exception>
226225
/// <exception cref="InvalidOperationException">Thrown when the content is invalid.</exception>
227-
protected async Task<GenericResource> CreateOrUpdateGenericResourceAsync<T>(ArmClient armClient, ResourceIdentifier resourceIdentifier, AzureLocation azureLocation, T content, JsonTypeInfo<T> jsonTypeInfo, CancellationToken cancellationToken)
226+
protected static async Task<GenericResource> CreateOrUpdateGenericResourceAsync<T>(
227+
ArmClient armClient,
228+
ResourceIdentifier resourceIdentifier,
229+
AzureLocation azureLocation,
230+
T content,
231+
JsonTypeInfo<T> jsonTypeInfo,
232+
CancellationToken cancellationToken)
228233
{
229-
if (armClient == null)
230-
throw new ArgumentNullException(nameof(armClient));
234+
ArgumentNullException.ThrowIfNull(armClient);
231235

232236
// Convert from T to GenericResourceData
233237
byte[] jsonBytes = JsonSerializer.SerializeToUtf8Bytes(content, jsonTypeInfo);
@@ -236,7 +240,8 @@ protected async Task<GenericResource> CreateOrUpdateGenericResourceAsync<T>(ArmC
236240
GenericResourceData data = dataModel.Create(ref reader, new ModelReaderWriterOptions("W"))
237241
?? throw new InvalidOperationException("Failed to create deployment data");
238242
// Create the resource
239-
var result = await armClient.GetGenericResources().CreateOrUpdateAsync(WaitUntil.Completed, resourceIdentifier, data, cancellationToken);
243+
var result = await armClient.GetGenericResources().CreateOrUpdateAsync(WaitUntil.Started, resourceIdentifier, data, cancellationToken);
244+
await WaitForLroCompletionAsync(result, cancellationToken);
240245
return result.Value;
241246
}
242247
}

core/Azure.Mcp.Core/src/Services/Azure/BaseAzureService.cs

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Azure.Core.Pipeline;
88
using Azure.Mcp.Core.Services.Azure.Tenant;
99
using Azure.ResourceManager;
10+
using Microsoft.Mcp.Core.Helpers;
1011
using Microsoft.Mcp.Core.Options;
1112
using Microsoft.Mcp.Core.Services.Azure;
1213

@@ -30,6 +31,7 @@ public abstract class BaseAzureService
3031
private static readonly string s_framework;
3132
private static readonly string s_platform;
3233
private static readonly string s_defaultUserAgent;
34+
private static readonly TimeSpan? s_defaultPollInterval;
3335

3436
static BaseAzureService()
3537
{
@@ -41,6 +43,13 @@ static BaseAzureService()
4143
// Initialize the default user agent policy without transport type
4244
s_defaultUserAgent = $"azmcp/{s_version} ({s_framework}; {s_platform})";
4345
s_sharedUserAgentPolicy = new UserAgentPolicy(s_defaultUserAgent);
46+
47+
#if DEBUG
48+
if (EnvironmentHelpers.IsPlaybackTesting())
49+
{
50+
s_defaultPollInterval = TimeSpan.FromMilliseconds(1);
51+
}
52+
#endif
4453
}
4554

4655
/// <summary>
@@ -76,20 +85,12 @@ public static void InitializeUserAgentPolicy(string transportType)
7685
/// Disables upper bounds enforcement on retry policy values (delays, timeouts, max retries).
7786
/// This method should be called once during application startup when the --dangerously-disable-retry-limits flag is set.
7887
/// </summary>
79-
public static void DisableRetryLimits()
80-
{
81-
s_retryLimitsDisabled = true;
82-
}
88+
public static void DisableRetryLimits() => s_retryLimitsDisabled = true;
8389

8490
/// <summary>
8591
/// Resets the retry limits flag. For testing only.
8692
/// </summary>
87-
internal static void ResetRetryLimits()
88-
{
89-
s_retryLimitsDisabled = false;
90-
}
91-
92-
93+
internal static void ResetRetryLimits() => s_retryLimitsDisabled = false;
9394

9495
/// <summary>
9596
/// Initializes a new instance of the <see cref="BaseAzureService"/> class.
@@ -175,7 +176,7 @@ protected async Task<TokenCredential> GetCredential(string? tenant, Cancellation
175176

176177
try
177178
{
178-
return await TenantService!.GetTokenCredentialAsync(tenantId, cancellationToken);
179+
return await TenantService.GetTokenCredentialAsync(tenantId, cancellationToken);
179180
}
180181
catch (Exception ex)
181182
{
@@ -295,4 +296,45 @@ protected static void ValidateRequiredParameters(params (string name, string? va
295296
$"Required parameter{(missingParams.Length > 1 ? "s are" : " is")} null or empty: {string.Join(", ", missingParams)}");
296297
}
297298
}
299+
300+
/// <summary>
301+
/// Waits for the completion of a long-running operation, periodically polling the operation status until it completes.
302+
/// </summary>
303+
/// <typeparam name="T">The return type.</typeparam>
304+
/// <param name="operation">The long-running operation.</param>
305+
/// <param name="cancellationToken">The cancellation token that can cancel the request.</param>
306+
/// <returns>The response once the long-running operation completes.</returns>
307+
protected static async Task WaitForLroCompletionAsync<T>(Operation<T> operation, CancellationToken cancellationToken = default) where T : notnull
308+
{
309+
ArgumentNullException.ThrowIfNull(operation);
310+
311+
if (s_defaultPollInterval.HasValue)
312+
{
313+
await operation.WaitForCompletionAsync(s_defaultPollInterval.Value, cancellationToken);
314+
}
315+
else
316+
{
317+
await operation.WaitForCompletionAsync(cancellationToken);
318+
}
319+
}
320+
321+
/// <summary>
322+
/// Waits for the completion of a long-running operation, periodically polling the operation status until it completes.
323+
/// </summary>
324+
/// <param name="operation">The long-running operation.</param>
325+
/// <param name="cancellationToken">The cancellation token that can cancel the request.</param>
326+
/// <returns>The response once the long-running operation completes.</returns>
327+
protected static async Task WaitForLroCompletionAsync(Operation operation, CancellationToken cancellationToken = default)
328+
{
329+
ArgumentNullException.ThrowIfNull(operation);
330+
331+
if (s_defaultPollInterval.HasValue)
332+
{
333+
await operation.WaitForCompletionResponseAsync(s_defaultPollInterval.Value, cancellationToken);
334+
}
335+
else
336+
{
337+
await operation.WaitForCompletionResponseAsync(cancellationToken);
338+
}
339+
}
298340
}

core/Azure.Mcp.Core/tests/Azure.Mcp.Core.LiveTests/RecordingFramework/RecordedCommandTestHarness.cs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ namespace Azure.Mcp.Core.LiveTests.RecordingFramework;
1313
/// </summary>
1414
/// <param name="output"></param>
1515
/// <param name="fixture"></param>
16-
internal sealed class RecordedCommandTestHarness(ITestOutputHelper output, TestProxyFixture fixture, LiveServerFixture liveServerFixture) : RecordedCommandTestsBase(output, fixture, liveServerFixture)
16+
internal sealed class RecordedCommandTestHarness(ITestOutputHelper output, TestProxyFixture fixture, LiveServerFixture liveServerFixture)
17+
: RecordedCommandTestsBase(output, fixture, liveServerFixture)
1718
{
1819
public TestMode DesiredMode { get; set; } = TestMode.Record;
1920

@@ -37,13 +38,7 @@ protected override ValueTask LoadSettingsAsync()
3738
return ValueTask.CompletedTask;
3839
}
3940

40-
public void ResetVariables()
41-
{
42-
TestVariables.Clear();
43-
}
41+
public void ResetVariables() => TestVariables.Clear();
4442

45-
public string GetRecordingId()
46-
{
47-
return RecordingId;
48-
}
43+
public string GetRecordingId() => RecordingId;
4944
}

core/Azure.Mcp.Core/tests/Azure.Mcp.Core.LiveTests/RecordingFramework/RecordedCommandTestsBaseTests.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
using System;
5-
using System.IO;
64
using System.Reflection;
75
using System.Text;
86
using System.Text.Json;

core/Microsoft.Mcp.Core/src/Helpers/EnvironmentVariableHelpers.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,16 @@ public static void SetAzureSubscriptionId(string? subscriptionId)
3737
{
3838
Environment.SetEnvironmentVariable(AzureSubscriptionIdEnvironmentVariable, subscriptionId);
3939
}
40+
41+
public static bool IsPlaybackTesting()
42+
{
43+
#if DEBUG
44+
// In debug builds, check for the presence of an environment variable to determine if we're in playback testing mode.
45+
var testModeEnv = Environment.GetEnvironmentVariable("TEST_MODE");
46+
return string.Equals(testModeEnv, "Playback", StringComparison.OrdinalIgnoreCase);
47+
#else
48+
// In non-debug builds, never consider ourselves to be in playback testing mode.
49+
return false;
50+
#endif
51+
}
4052
}

core/Microsoft.Mcp.Core/src/Services/Azure/Authentication/CustomChainedCredential.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,8 @@ private static TokenCredential CreateCredential(string? tenantId, ILogger<Custom
130130
bool hasExplicitCredentialSetting = !string.IsNullOrEmpty(tokenCredentials);
131131

132132
#if DEBUG
133-
bool isPlaybackMode = string.Equals(tokenCredentials, "PlaybackTokenCredential", StringComparison.OrdinalIgnoreCase);
134133
// Short-circuit for playback to avoid any real auth & interactive prompts.
135-
if (isPlaybackMode)
134+
if (EnvironmentHelpers.IsPlaybackTesting())
136135
{
137136
logger?.LogDebug("Playback mode detected: using PlaybackTokenCredential.");
138137
return new PlaybackTokenCredential();

core/Microsoft.Mcp.Core/src/Services/Http/RecordingRedirectHandler.cs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System.Net.Http.Headers;
5+
using Microsoft.Mcp.Core.Helpers;
6+
47
namespace Microsoft.Mcp.Core.Services.Http;
58

69
/// <summary>
@@ -10,26 +13,23 @@ namespace Microsoft.Mcp.Core.Services.Http;
1013
/// This handler is intended to be injected as the LAST delegating handler (closest to the transport) so
1114
/// that it rewrites the final outgoing wire request.
1215
/// </summary>
13-
internal sealed class RecordingRedirectHandler : DelegatingHandler
16+
/// <param name="proxyUri">The URI of the recording/replace proxy to redirect requests to.</param>
17+
internal sealed class RecordingRedirectHandler(Uri proxyUri) : DelegatingHandler
1418
{
1519
private const string CosmosSerializationHeader = "x-ms-cosmos-supported-serialization-formats";
16-
private readonly Uri _proxyUri;
17-
18-
public RecordingRedirectHandler(Uri proxyUri)
19-
{
20-
_proxyUri = proxyUri ?? throw new ArgumentNullException(nameof(proxyUri));
21-
}
20+
private readonly Uri _proxyUri = proxyUri ?? throw new ArgumentNullException(nameof(proxyUri));
21+
private readonly bool _playbackTesting = EnvironmentHelpers.IsPlaybackTesting();
2222

2323
protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
2424
{
2525
Redirect(request);
26-
return base.Send(request, cancellationToken)!;
26+
return StripRetryAfter(base.Send(request, cancellationToken));
2727
}
2828

2929
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
3030
{
3131
Redirect(request);
32-
return await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
32+
return StripRetryAfter(await base.SendAsync(request, cancellationToken).ConfigureAwait(false));
3333
}
3434

3535
private void Redirect(HttpRequestMessage message)
@@ -60,4 +60,24 @@ private void Redirect(HttpRequestMessage message)
6060

6161
message.RequestUri = builder.Uri;
6262
}
63+
64+
private HttpResponseMessage StripRetryAfter(HttpResponseMessage response)
65+
{
66+
if (_playbackTesting)
67+
{
68+
if (response.Headers.Remove("Retry-After"))
69+
{
70+
response.Headers.RetryAfter = new RetryConditionHeaderValue(TimeSpan.Zero);
71+
}
72+
if (response.Headers.Remove("x-ms-retry-after-ms"))
73+
{
74+
response.Headers.Add("x-ms-retry-after-ms", "0");
75+
}
76+
if (response.Headers.Remove("retry-after-ms"))
77+
{
78+
response.Headers.Add("retry-after-ms", "0");
79+
}
80+
}
81+
return response;
82+
}
6383
}

core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212

1313
namespace Microsoft.Mcp.Tests.Client;
1414

15-
public abstract class CommandTestsBase(ITestOutputHelper output, LiveServerFixture liveServerFixture) : IAsyncLifetime, IDisposable, IClassFixture<LiveServerFixture>
15+
public abstract class CommandTestsBase(ITestOutputHelper output, LiveServerFixture liveServerFixture)
16+
: IAsyncLifetime, IDisposable, IClassFixture<LiveServerFixture>
1617
{
1718
protected const string TenantNameReason = "Service principals cannot use TenantName for lookup";
1819

@@ -65,7 +66,7 @@ protected virtual async ValueTask LoadSettingsAsync()
6566
TestMode = Settings.TestMode;
6667
}
6768

68-
private async Task<LiveTestSettings?> TryLoadLiveSettingsAsync()
69+
private static async Task<LiveTestSettings?> TryLoadLiveSettingsAsync()
6970
{
7071
try
7172
{
@@ -90,22 +91,25 @@ protected virtual async ValueTask LoadSettingsAsync()
9091
// { "AZURE_SUBSCRIPTION_ID", Settings.SubscriptionId }
9192
];
9293

93-
if (proxy != null && proxy.Proxy != null)
94+
// Add any custom environment variables from settings
95+
if (Settings?.EnvironmentVariables != null)
9496
{
95-
envVarDictionary.Add("TEST_PROXY_URL", proxy.Proxy.BaseUri);
96-
97-
if (TestMode is TestMode.Playback)
97+
foreach (var kvp in Settings.EnvironmentVariables)
9898
{
99-
envVarDictionary.Add("AZURE_TOKEN_CREDENTIALS", "PlaybackTokenCredential");
99+
envVarDictionary[kvp.Key] = kvp.Value;
100100
}
101101
}
102102

103-
// Add any custom environment variables from settings
104-
if (Settings?.EnvironmentVariables != null)
103+
if (proxy != null && proxy.Proxy != null)
105104
{
106-
foreach (var kvp in Settings.EnvironmentVariables)
105+
envVarDictionary["TEST_PROXY_URL"] = proxy.Proxy.BaseUri;
106+
107+
if (TestMode is TestMode.Playback)
107108
{
108-
envVarDictionary[kvp.Key] = kvp.Value;
109+
// AZURE_TOKEN_CREDENTIALS=PlaybackTokenCredential tells the server to use a special credential that
110+
// returns fake tokens in playback mode, which prevents any accidental live calls if a test is misconfigured
111+
envVarDictionary["AZURE_TOKEN_CREDENTIALS"] = "PlaybackTokenCredential";
112+
envVarDictionary["TEST_MODE"] = "Playback";
109113
}
110114
}
111115

@@ -262,8 +266,5 @@ protected virtual void Dispose(bool disposing)
262266

263267
// subclasses should override this method to dispose async resources
264268
// overrides should still call base.DisposeAsyncCore()
265-
protected virtual ValueTask DisposeAsyncCore()
266-
{
267-
return ValueTask.CompletedTask;
268-
}
269+
protected virtual ValueTask DisposeAsyncCore() => ValueTask.CompletedTask;
269270
}

core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/Helpers/LiveTestSettings.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ public class LiveTestSettings
1212
{
1313
public const string TestSettingsFileName = ".testsettings.json";
1414

15+
private static readonly JsonSerializerOptions s_jsonOptions = new()
16+
{
17+
PropertyNameCaseInsensitive = true,
18+
Converters = { new JsonStringEnumConverter() }
19+
};
20+
1521
public string PrincipalName { get; set; } = string.Empty;
1622
public bool IsServicePrincipal { get; set; }
1723
public string TenantId { get; set; } = string.Empty;
@@ -53,11 +59,7 @@ public static bool TryLoadTestSettings([NotNullWhen(true)] out LiveTestSettings?
5359
{
5460
var json = File.ReadAllText(path);
5561

56-
settings = JsonSerializer.Deserialize<LiveTestSettings>(json, new JsonSerializerOptions()
57-
{
58-
PropertyNameCaseInsensitive = true,
59-
Converters = { new JsonStringEnumConverter() }
60-
});
62+
settings = JsonSerializer.Deserialize<LiveTestSettings>(json, s_jsonOptions);
6163

6264
if (settings != null)
6365
{

0 commit comments

Comments
 (0)