Attempting to switch to FrameworkReferences (#2355), and I found a problem with some tests for AzureInstanceMetadada.
What
unnecessary local server
First issue, the tests for AzureInstanceMetadata run a local server to intercept HttpRequests.
There were breaking changes to WebHostBuilder in the change from netcore2.x -> netcore3.x.
|
ResponseHandlerMock.OnRequestDictionary.Add(testName, onRequest); |
|
|
|
this.cts = new CancellationTokenSource(); |
|
this.host = new WebHostBuilder() |
|
.UseKestrel() |
|
.UseStartup<ResponseHandlerMock>() |
|
.UseUrls(baseUrl + "?testName=" + testName) |
|
.Build(); |
|
|
|
Task.Run(() => this.host.Run(this.cts.Token)); |
We do not need a local server to test AzureInstanceMetadata, we only need to mock the HTTP Response.
This is a good candidate to switch to use HttpClient mocking.
https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.-ctor?view=net-5.0#System_Net_Http_HttpClient__ctor_System_Net_Http_HttpMessageHandler_
invalid tests
Second issue, tests were replacing the HTTP call with a Func<> that returns a parsed object.
The existing tests seem to verify the behavior of this mock class, and not the HttpClient.
While rewriting tests, I've found that the HttpClient successfully handles the test condition.
Affected product code:
AzureMetadataRequestor ctor accepts a Func<>.
|
internal AzureMetadataRequestor(Func<string, Task<AzureInstanceComputeMetadata>> makeAzureIMSRequestor = null) |
|
{ |
|
this.azureIMSRequestor = makeAzureIMSRequestor; |
|
} |
When making a request, it can switch to use the Func instead.
This is used in unit tests to invoke a local server.
|
private async Task<AzureInstanceComputeMetadata> MakeAzureMetadataRequestAsync(string metadataRequestUrl) |
|
{ |
|
AzureInstanceComputeMetadata requestResult = null; |
|
|
|
SdkInternalOperationsMonitor.Enter(); |
|
try |
|
{ |
|
if (this.azureIMSRequestor != null) |
|
{ |
|
requestResult = await this.azureIMSRequestor(metadataRequestUrl).ConfigureAwait(false); |
|
} |
|
else |
|
{ |
|
requestResult = await this.MakeWebRequestAsync(metadataRequestUrl).ConfigureAwait(false); |
|
} |
|
} |
|
catch (Exception ex) |
|
{ |
|
WindowsServerEventSource.Log.AzureInstanceMetadataRequestFailure( |
|
metadataRequestUrl, ex.Message, ex.InnerException != null ? ex.InnerException.Message : string.Empty); |
|
} |
|
finally |
|
{ |
|
SdkInternalOperationsMonitor.Exit(); |
|
} |
|
|
|
return requestResult; |
|
} |
Affected Unit Tests:
Some of these tests are testing a condition that is only reproducible using the mock code.
I'm summarizing the affected tests here and will replace the tests where appropriate.
AzureInstanceMetadataEndToEndTests
SpoofedResponseFromAzureIMSDoesntCrash
tests that a request sucessfully parses the json responseAzureImsResponseTooLargeStopsCollection
unable to force a failure here. HttpClient successfully handles the same json response objectAzureImsResponseExcludesMalformedValues
this test invalid values within the json response object. these invalid values are correctly ignored.AzureImsResponseTimesOut
Unable to force a timeout using HttpClient. The HttpClient is sucessfully parsing the json response after timeout.
Reading the docs, HttpClient will throw an exception when a timeout is experienced.
While Testing, the AzureMetadataRequestor correctly handles exceptions.AzureImsResponseUnsuccessful
test crrectly verifies errant HttpStatusCodes.
AzureInstanceMetadataTests
AzureIMSGetFailsWithException
this test verifies that the AzureMetadataRequestor handles HttpExceptions.
Attempting to switch to FrameworkReferences (#2355), and I found a problem with some tests for AzureInstanceMetadada.
What
unnecessary local server
First issue, the tests for AzureInstanceMetadata run a local server to intercept HttpRequests.
There were breaking changes to
WebHostBuilderin the change from netcore2.x -> netcore3.x.ApplicationInsights-dotnet/WEB/Src/WindowsServer/WindowsServer.Tests/Mock/AzureInstanceMetadataServiceMock.cs
Lines 27 to 36 in a90eb8f
We do not need a local server to test AzureInstanceMetadata, we only need to mock the HTTP Response.
This is a good candidate to switch to use HttpClient mocking.
https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.-ctor?view=net-5.0#System_Net_Http_HttpClient__ctor_System_Net_Http_HttpMessageHandler_
invalid tests
Second issue, tests were replacing the HTTP call with a Func<> that returns a parsed object.
The existing tests seem to verify the behavior of this mock class, and not the HttpClient.
While rewriting tests, I've found that the HttpClient successfully handles the test condition.
Affected product code:
AzureMetadataRequestorctor accepts a Func<>.ApplicationInsights-dotnet/WEB/Src/WindowsServer/WindowsServer/Implementation/AzureMetadataRequestor.cs
Lines 34 to 37 in a90eb8f
When making a request, it can switch to use the Func instead.
This is used in unit tests to invoke a local server.
ApplicationInsights-dotnet/WEB/Src/WindowsServer/WindowsServer/Implementation/AzureMetadataRequestor.cs
Lines 59 to 86 in a90eb8f
Affected Unit Tests:
Some of these tests are testing a condition that is only reproducible using the mock code.
I'm summarizing the affected tests here and will replace the tests where appropriate.
AzureInstanceMetadataEndToEndTestsSpoofedResponseFromAzureIMSDoesntCrashtests that a request sucessfully parses the json response
AzureImsResponseTooLargeStopsCollectionunable to force a failure here. HttpClient successfully handles the same json response object
AzureImsResponseExcludesMalformedValuesthis test invalid values within the json response object. these invalid values are correctly ignored.
AzureImsResponseTimesOutUnable to force a timeout using HttpClient. The HttpClient is sucessfully parsing the json response after timeout.
Reading the docs, HttpClient will throw an exception when a timeout is experienced.
While Testing, the AzureMetadataRequestor correctly handles exceptions.
AzureImsResponseUnsuccessfultest crrectly verifies errant HttpStatusCodes.
AzureInstanceMetadataTestsAzureIMSGetFailsWithExceptionthis test verifies that the AzureMetadataRequestor handles HttpExceptions.