Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AzureInstanceMetadata: replace tests #2357

Closed
TimothyMothra opened this issue Aug 10, 2021 · 0 comments · Fixed by #2360
Closed

AzureInstanceMetadata: replace tests #2357

TimothyMothra opened this issue Aug 10, 2021 · 0 comments · Fixed by #2360
Labels
Milestone

Comments

@TimothyMothra
Copy link
Member

TimothyMothra commented Aug 10, 2021

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 response
    • AzureImsResponseTooLargeStopsCollection
      unable to force a failure here. HttpClient successfully handles the same json response object
    • AzureImsResponseExcludesMalformedValues
      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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant