Skip to content

Commit 8b4d0a7

Browse files
m-reddingCopilot
m-redding
and
Copilot
authored
[Microsoft.ClientModel.TestFramework] Copy improvements from core test framework, add better integration tests, fix some tooling issues (#54020)
* pass 1 * updates & test stubs * update * updates * updates * comment out tests for now * export API * bringing over some integration tests from Azure.Core.TestFramework * bring over improvements from core test framework * fix issue with restore arg * debugging * debugging again * more debugging * try removing global install * more testing * add dotnet tool restore * add some debugging * fix build * update recordings and tests * test diff WD * remove debugging * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * copilot fb * Include dotnet tool restore in CI tests Add dotnet tool restore step before running tests. --------- Co-authored-by: Copilot <[email protected]>
1 parent a43d5cd commit 8b4d0a7

29 files changed

+1188
-966
lines changed

‎.config/dotnet-tools.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
"commands": [
1414
"reportgenerator"
1515
]
16+
},
17+
"Azure.Sdk.Tools.TestProxy": {
18+
"version": "1.0.0-dev.20251022.1",
19+
"commands": [
20+
"test-proxy"
21+
]
1622
}
1723
}
1824
}

‎eng/pipelines/templates/jobs/ci.tests.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ jobs:
157157
parameters:
158158
LogFilePath: $(Build.ArtifactStagingDirectory)/test.binlog
159159

160+
- pwsh: |
161+
dotnet tool restore
162+
160163
- pwsh: >
161164
./eng/scripts/runwithdevopslogging.ps1
162165
dotnet test eng/service.proj

‎sdk/core/Microsoft.ClientModel.TestFramework/api/Microsoft.ClientModel.TestFramework.net462.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ protected TestEnvironment() { }
380380
public static bool IsWindows { get { throw null; } }
381381
public Microsoft.ClientModel.TestFramework.RecordedTestMode? Mode { get { throw null; } set { } }
382382
public string? PathToTestResourceBootstrappingScript { get { throw null; } set { } }
383-
public static string? RepositoryRoot { get { throw null; } }
383+
public static string? RepositoryRoot { get { throw null; } protected set { } }
384384
public void BootStrapTestResources() { }
385385
protected string? GetOptionalVariable(string name) { throw null; }
386386
protected string? GetRecordedOptionalVariable(string name) { throw null; }

‎sdk/core/Microsoft.ClientModel.TestFramework/api/Microsoft.ClientModel.TestFramework.net8.0.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ protected TestEnvironment() { }
380380
public static bool IsWindows { get { throw null; } }
381381
public Microsoft.ClientModel.TestFramework.RecordedTestMode? Mode { get { throw null; } set { } }
382382
public string? PathToTestResourceBootstrappingScript { get { throw null; } set { } }
383-
public static string? RepositoryRoot { get { throw null; } }
383+
public static string? RepositoryRoot { get { throw null; } protected set { } }
384384
public void BootStrapTestResources() { }
385385
protected string? GetOptionalVariable(string name) { throw null; }
386386
protected string? GetRecordedOptionalVariable(string name) { throw null; }

‎sdk/core/Microsoft.ClientModel.TestFramework/api/Microsoft.ClientModel.TestFramework.net9.0.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ protected TestEnvironment() { }
380380
public static bool IsWindows { get { throw null; } }
381381
public Microsoft.ClientModel.TestFramework.RecordedTestMode? Mode { get { throw null; } set { } }
382382
public string? PathToTestResourceBootstrappingScript { get { throw null; } set { } }
383-
public static string? RepositoryRoot { get { throw null; } }
383+
public static string? RepositoryRoot { get { throw null; } protected set { } }
384384
public void BootStrapTestResources() { }
385385
protected string? GetOptionalVariable(string name) { throw null; }
386386
protected string? GetRecordedOptionalVariable(string name) { throw null; }

‎sdk/core/Microsoft.ClientModel.TestFramework/assets.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"AssetsFileName": "assets.json",
3+
"AssetsRepo": "Azure/azure-sdk-assets",
4+
"Tag": "net/core/Microsoft.ClientModel.TestFramework_d3209afd89",
5+
"AssetsRepoPrefixPath": "net",
6+
"TagPrefix": "net/core/Microsoft.ClientModel.TestFramework",
7+
"AssetsRepoLocation": {},
8+
"AssetRepoShortHash": "UfGeNmhxxt"
9+
}

‎sdk/core/Microsoft.ClientModel.TestFramework/src/RecordedTests/RecordedTestAttribute.cs‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Linq;
7-
using System.Threading;
87
using NUnit.Framework;
98
using NUnit.Framework.Interfaces;
109
using NUnit.Framework.Internal;
@@ -117,32 +116,50 @@ public override TestResult Execute(TestExecutionContext context)
117116

118117
if (resultMessage?.Contains(typeof(TestTimeoutException).FullName!) == true)
119118
{
120-
// retry once
121-
context.CurrentResult = context.CurrentTest.MakeTestResult();
122-
context.CurrentResult = innerCommand.Execute(context);
119+
HandleTestTimeout(context, originalResult);
120+
}
121+
}
123122

124-
if (IsTestFailed(context))
125-
{
126-
context.CurrentResult.SetResult(
127-
ResultState.Error,
128-
"The test timed out twice:" + Environment.NewLine +
129-
$"First attempt: {originalResult.Message}" + Environment.NewLine +
130-
$"Second attempt: {context.CurrentResult.Message}");
131-
}
132-
else
133-
{
134-
context.CurrentResult.SetResult(
135-
context.CurrentResult.ResultState,
136-
"Test timed out in initial run, but was retried successfully.");
137-
}
123+
return context.CurrentResult;
124+
}
138125

126+
private TestResult HandleTestTimeout(TestExecutionContext context, TestResult originalResult)
127+
{
128+
var results = new List<TestResult> { originalResult };
129+
130+
for (int retryCount = 0; retryCount < 2; retryCount++)
131+
{
132+
// Create a new TestResult instance
133+
context.CurrentResult = context.CurrentTest.MakeTestResult();
134+
// Run the test again
135+
context.CurrentResult = innerCommand.Execute(context);
136+
results.Add(context.CurrentResult);
137+
138+
if (!IsTestFailed(context))
139+
{
140+
context.CurrentResult.SetResult(
141+
ResultState.Success,
142+
ConstructRetryMessage("Test timed out initially, but passed on retry", results));
139143
return context.CurrentResult;
140144
}
141145
}
142146

147+
context.CurrentResult.SetResult(
148+
ResultState.Error,
149+
ConstructRetryMessage("The test timed out on all attempts", results));
150+
143151
return context.CurrentResult;
144152
}
145153

154+
private static string ConstructRetryMessage(string header, IEnumerable<TestResult> results)
155+
{
156+
var attemptDetails = string.Join(Environment.NewLine,
157+
results.Select((r, i) =>
158+
$"Attempt {i + 1}: {r.Message ?? "Passed"}"));
159+
160+
return header + ":" + Environment.NewLine + attemptDetails + Environment.NewLine;
161+
}
162+
146163
/// <summary>
147164
/// Sets the recording mode for the specified recorded test fixture.
148165
/// </summary>

‎sdk/core/Microsoft.ClientModel.TestFramework/src/RecordedTests/RecordedTestBase.cs‎

Lines changed: 3 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ public abstract class RecordedTestBase : ClientTestBase
4545
(char)21, (char)22, (char)23, (char)24, (char)25, (char)26, (char)27, (char)28, (char)29, (char)30,
4646
(char)31, ':', '*', '?', '\\', '/'
4747
});
48-
private static string EmptyGuid = Guid.Empty.ToString();
49-
private static readonly object s_syncLock = new();
50-
private static bool s_ranTestProxyValidation;
48+
private static readonly string s_emptyGuid = Guid.Empty.ToString();
5149
private TestProxyProcess? _proxy;
5250
private DateTime _testStartTime;
5351

@@ -134,8 +132,8 @@ public abstract class RecordedTestBase : ClientTestBase
134132
/// </summary>
135133
public virtual List<UriRegexSanitizer> UriRegexSanitizers { get; } = new()
136134
{
137-
UriRegexSanitizer.CreateWithQueryParameter("skoid", EmptyGuid),
138-
UriRegexSanitizer.CreateWithQueryParameter("sktid", EmptyGuid),
135+
UriRegexSanitizer.CreateWithQueryParameter("skoid", s_emptyGuid),
136+
UriRegexSanitizer.CreateWithQueryParameter("sktid", s_emptyGuid),
139137
};
140138

141139
/// <summary>
@@ -667,11 +665,6 @@ public virtual async Task StopTestRecordingAsync()
667665
if (Recording != null)
668666
{
669667
await Recording.DisposeAsync(save).ConfigureAwait(false);
670-
671-
if (Mode == RecordedTestMode.Record && save)
672-
{
673-
AssertTestProxyToolIsInstalled();
674-
}
675668
}
676669

677670
if (_proxy != null)
@@ -761,72 +754,6 @@ public static Task Delay(RecordedTestMode mode, int milliseconds = 1000, int? pl
761754
return Task.CompletedTask;
762755
}
763756

764-
private void AssertTestProxyToolIsInstalled()
765-
{
766-
if (s_ranTestProxyValidation ||
767-
!TestEnvironment.IsWindows ||
768-
AssetsJsonPath == null)
769-
{
770-
return;
771-
}
772-
773-
lock (s_syncLock)
774-
{
775-
if (s_ranTestProxyValidation)
776-
{
777-
return;
778-
}
779-
780-
s_ranTestProxyValidation = true;
781-
782-
try
783-
{
784-
if (IsTestProxyToolInstalled())
785-
{
786-
return;
787-
}
788-
789-
string path = Path.Combine(
790-
TestEnvironment.RepositoryRoot ?? throw new InvalidOperationException("TestEnvironment.RepositoryRoot is null"),
791-
"eng",
792-
"scripts",
793-
"Install-TestProxyTool.ps1");
794-
795-
var processInfo = new ProcessStartInfo("pwsh.exe", path)
796-
{
797-
UseShellExecute = true
798-
};
799-
800-
var process = Process.Start(processInfo);
801-
802-
if (process != null)
803-
{
804-
process.WaitForExit();
805-
}
806-
}
807-
catch (Exception)
808-
{
809-
// Ignore
810-
}
811-
}
812-
}
813-
814-
private bool IsTestProxyToolInstalled()
815-
{
816-
var processInfo = new ProcessStartInfo("dotnet.exe", "tool list --global")
817-
{
818-
RedirectStandardOutput = true,
819-
UseShellExecute = false
820-
};
821-
822-
var process = Process.Start(processInfo);
823-
var output = process?.StandardOutput.ReadToEnd();
824-
825-
process?.WaitForExit();
826-
827-
return output != null && output.Contains("azure.sdk.tools.testproxy");
828-
}
829-
830757
/// <summary>
831758
/// Provides a helper for implementing retry logic in tests that accounts for the current recording mode.
832759
/// The retry behavior adapts based on whether the test is running in Live, Record, or Playback mode.

‎sdk/core/Microsoft.ClientModel.TestFramework/src/RecordedTests/TestProxy/ProxyTransport.cs‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,26 @@ public ProxyTransport(
4444
bool useFiddler = TestEnvironment.EnableFiddler;
4545
string certIssuer = useFiddler ? FiddlerCertIssuer : DevCertIssuer;
4646
_proxyHost = useFiddler ? "ipv4.fiddler" : TestProxyProcess.IpAddress;
47-
var handler = new HttpClientHandler
47+
48+
if (innerTransport is HttpClientPipelineTransport _)
4849
{
49-
ServerCertificateCustomValidationCallback = (_, certificate, _, _) => certificate?.Issuer == certIssuer,
50-
AllowAutoRedirect = false,
51-
UseCookies = false
52-
};
53-
var httpClient = new HttpClient(handler)
50+
var handler = new HttpClientHandler
51+
{
52+
ServerCertificateCustomValidationCallback = (_, certificate, _, _) => certificate?.Issuer == certIssuer,
53+
AllowAutoRedirect = false,
54+
UseCookies = false
55+
};
56+
var httpClient = new HttpClient(handler)
57+
{
58+
// Timeouts are handled by the pipeline
59+
Timeout = Timeout.InfiniteTimeSpan
60+
};
61+
_innerTransport = new HttpClientPipelineTransport(httpClient);
62+
}
63+
else // Use provided custom transport as-is when it's not an HttpClientPipelineTransport
5464
{
55-
// Timeouts are handled by the pipeline
56-
Timeout = Timeout.InfiniteTimeSpan
57-
};
58-
_innerTransport = new HttpClientPipelineTransport(httpClient);
65+
_innerTransport = innerTransport;
66+
}
5967
}
6068

6169
/// <inheritdoc/>

0 commit comments

Comments
 (0)