microsoft / microsoft/mcp

Update way we initialise *AzureServices to be more testable.

Open
#172 1 comment 0 reactions 1 assignee Claimed by @conniey View on GitHub
Engineering Excellence engineering item server-Azure.Mcp tools-AppConfig
Dominant language
C#
Stars
3.7k
Forks
624
Avg merge
2d 20h
Merged PRs (30d)
220

Description

Related: https://github.com/Azure/azure-mcp-pr/issues/268

### 2. Abstract the creation of any `*Clients`

`BaseAzureService` creates a real instance of `ArmClient`. This requires us to use real credentials, set-up real data, etc, in order to test the required scenario. We aren't testing the Azure services and should assume that if we call whatever function on ArmClient, it returns the correct thing based on its method signature. `ArmClient` is mockable and its methods are `virtual`.

```cs
public partial class AzureClientService
{
public virtual ArmClient GetArmClient(string tenantId, RetryPolicyOptions options) { }
}

// Update BaseAzureService to include this dependency in its constructor
public BaseAzureService(AzureClientService service, ITenantService service)
{
protected async Task GetArmClientAsync(string, options) { /* Uses that service underneath*/ }
}

#### Example for people using `GetArmClientAsync`

```cs
public class DatadogService
{
public async Task> ListMonitoredResources(string resourceGroup, string subscription, string datadogResource)
{
try
{
var tenantId = await ResolveTenantIdAsync(null);
var armClient = await CreateArmClientAsync(tenant: tenantId, retryPolicy: null);

var resourceId = $"/subscriptions/{subscription}/resourceGroups/{resourceGroup}/providers/Microsoft.Datadog/monitors/{datadogResource}";

ResourceIdentifier id = new ResourceIdentifier(resourceId);

// This is where we would have to set-up real data.
var datadogMonitorResource = armClient.GetDatadogMonitorResource(id);
var monitoredResources = datadogMonitorResource.GetMonitoredResources();

var resourceList = new List();
foreach (var resource in monitoredResources)
{
var resourceIdSegments = resource.Id.ToString().Split('/');
var lastSegment = resourceIdSegments[^1];
resourceList.Add(lastSegment);
}

return resourceList;
}
catch (Exception ex)
{
throw new Exception($"Error listing monitored resources: {ex.Message}", ex);
}
}
}
```
and its test:
```cs
public class DatadogServiceTest
{
[Fact]
public async Task ListMonitoredResourceTest()
{
// Arrange
var mockedArmClient = Substitute.For();
var azureClientService = Substitute.For();
var datadogResource = Substitute.For();
var monitoredResource = Substitute.For();

// Generally I want to be specific about my arg matchers so that we know we are constructing this resourceId correctly.
azureClientService.GetArmClient(Args.Any(), Args.Any()).Returns(mockedArmClient);

// I can do interesting things now based on the ArmClient's GetDatadogMonitorResource API docs.
// For example, the API says it'll return a Response with 404 if the id is not found,
// throw an ArgumentException in other scenarios, or fake what an empty monitor resource would look like, etc.
mockedArmClient.GetDatadogMonitorResource(Arg.Any()).Returns(datadogResource);

var datadogService = new DatadogService(azureClientService, tenantService);

// Act
var expectedResources = datadogService.ListMonitoredResources()

// Assert
// There is some string manipulation here I can verify.
}
}
```

#### Example for services constructing a client

If there are services that construct their own clients, add to that partial class. For example, `MonitorService` creates a `LogQueryClient`.

Then add a file in their service folder, `AzureClientService.Log.cs`... and use that in their tests.
```cs
public partial class AzureClientService
{
public virtual LogsQueryClient GetQueryClient(TokenCredential tokenCredential, LogsQueryClientOptions options)
{
return new LogsQueryClient(tokenCredential, options);
}
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.