there should be base interfaces for mocking on classes with business logic

[MovGP0]

classes with business logic like CloudBlobContainer, CloudStorageAccount, CloudBlobClient, CloudFileClient, CloudQueueClient, and CloudTableClient should have a base interface that allows to replace those classes easily with mocks in unit tests.

[MovGP0]

refers to Microsoft.WindowsAzure.Storage, Version 8.6.0.0

[Izzmo]

Yeah.. this also needs to be done on the new V12 stuff too... like smh.

[amnguye]

HI @Izzmo, if this needs to be done for the v12 stuff please make an issue on the V12 repo so we can address this accordingly :)

[PureKrome]

Link please @amnguye to the v12 repo.

[seanmcc-msft]

https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/storage

All v12 client's public APIs are virtual, allowing you mock them using mocking frameworks like MOQ. Return types can be constructed using the models factors, such as Azure.Storage.Blobs.Models.BlobModelFactory.

[Izzmo]

@seanmcc-msft ah, very true. I didn't catch that. What are the advantages to making them all virtual vs. using an interface?

[seanmcc-msft]

Making public API methods as virtual conforms to our SDK guidelines.

DO make service methods virtual.

[stewartadam]

@seanmcc-msft can you elaborate apart from just the internal guidance to do it? Is it because changes (even pure additions) to interfaces would break all implementing classes in customer code?

Also for anyone else searching the repo and finding no results -- the class needs an 's', BlobsModelFactory.

[seanmcc-msft]

I'm not sure why we chose to use virtual methods instead of interfaces, the One Azure SDK review board made this decision for all new .NET Azure SDKs.

[MovGP0]

Sealing everything and using interfaces for mocking on service classes would be the recommended way. A class should therefore either be abstract or sealed.

For modifying behaviour, one should use the 'composition over inheritance' principle instead.

Opening the class for inheritance and making service methods virtual (open-closed-principle) should be used additionally where it provides a benefit over dependency inversion and composition.

[stewartadam]

To circle back on this, given the policy of:

DO make service methods virtual.

Then I don't expect there to be base classes/interfaces implemented that would provide a clear path forward consumers of the SDK to create unit tests. If the SDK expects users to spawn (mocked) instances using its own factory classes, I'd recommend the SDK providing examples of those factory classes built-in to make the usage clear (at minimum, link to any relevant factory classes required for unit testing in the README.md).

As for what examples should be provided -- I think the callout in the original issue description is a good start: a simple test for CloudBlobContainer, CloudStorageAccount, CloudBlobClient, CloudFileClient, CloudQueueClient, and CloudTableClient.

[PureKrome]

DO make service methods virtual.

and then

don't expect there to be base classes/interfaces implemented that would provide a clear path forward consumers of the SDK to create unit tests.

and

As for what examples should be provided ... CloudQueueClient

Okay. So if the methods are virtual, then you can mock them in unit tests. You don't need interfaces, for mocking ... at least with the Moq mocking framework (which could be fair to say is the most popular .net mocking framework on NuGet).

Example:

• I wish to add a message to an Azure Queue

(all pseudo code. Just quickly wrote something up. missing all null checks, etc. etc. etc).

REF: Azure.Storages.Queue.QueueClient.

public class SomeService
{
    private readonly QueueClient _queueClient; // This is the Azure Queue Client from Azure.Storages.Queue.

    // Constructor.
    // The QueueClient is 'injected' into this class usually via DI/IoC (out of scope of this example code).
    public void SomeService(QueueClient queueClient) => _queueClient = queueClient;

    // We wish to add a message to a queue.
    public async Task SomeMethodAsync(string message, CancellationToken)
    {
        var response = await _queueClient.SendMessageAsync(message, cancellationToken);
    }
}

and now lets unit test this bad boy with XUnit, and Moq...

public class SomeMethodAsyncTests
{
    public async Task GivenSomeMessage_SomeMethodAsync_ShouldSendTheMessageToTheQueue()
    {
        // Arrange.
        const string message = "I love turtles!";

        // Lets create a mocked QueueClient and fake a response when someone sends a message to the queue.
        var response = new Mock<Response<SendReceipt>>();
        var queueClient = new Mock<QueueClient>();
        queueClient
            .Setup(x => x.SendMessageAsync(message, It.IsAny<CancellationToken>())) // Push the message onto the queue.
            .ReturnsAsync(response.Object); // Not sure if you really care about the response object here.

        // SUT: Service Under Test.
        var service = new SomeService(queueClient.Object);

        // Act.
        var result = await service.SomeMethodAsync(message, CancellationToken.None);

        // Assert.
        queueClient.VerifyAll(); // The SendMessageAsync should have been called at least once, because we defined a SETUP condition.
    }
}

Okay .. so that's a quick a dirty example of

• adding a message to a queue
• unit testing that a specific method on the queue class (provided by Microsoft via their NuGet package) was called.

Bonus reamarks: The code above can be improved with error handling and even wrapping the call to the SendMessageAsync inside a Polly 'retry' policy.

HTH.

Final note: current SDK is soooooooooooooooo much better than the original days of the initial/earlier SDK's. We can mock stuff, today. 🎉

[danneberg]

When you write this in your business code: public void SomeService(QueueClient queueClient) => _queueClient = queueClient;

You have to write this in your business code: using Azure.Storages.Queue.QueueClient;

Do you really ever want to reference a 3rd party library from your business code. Bad Idea. You have to recompile your code when you: Want to write to an Azure topic or a Service Bus Queue / Wan to write into an AWS Queue.

Solution: Write your own IQueueRepository interface in Business, Implement it in a Repository as QueueRepository and use the Classes from the external library to implement the repository. Unit testing the Repository makes no sense. Unit testing the Business is simple. Mocking IQueueRepository.

Why are the methods virtual? So you can use dependency injection to create exception handling / retrying / logging just by interception.

Hope this gives you some background why the rule: "There is no interface" make sense here.

[PureKrome]

Do you really ever want to reference a 3rd party library from your business code.

Yes! TOTALLY!

Bad Idea.

Sigh...

I know exactly what the problem requires to solve it ... It requires me to throw a message on an AZURE Queue. Could I have gone to AWS? Sure. MB? sure! hell, i could have used channels or even a text file. So many ways to solve a problem ....

Bad Idea. You have to recompile your code when you: Want to write to an Azure topic or a Service Bus Queue / Wan to write into an AWS Queue.

Okay ... so now you're going to suggest that I also abstract my database because ... you know .. we all switch from MS-SQL -> Postgres -> MySql -> NoSql DB's -> text files -> back to MS-SQL.

You can abstract anything to the N'th degree. Go nuts. But at some point, you'll need to actually end up calling something. A good architect has to make a decision about what infrastructure they will end up using and hopefully stick to it. If you're going to play the "we might keep moving to different infrastructure" cards, then you have different problem(s). For years and decades heaps of people have cried out loud about abstracting everything away to buggery. Yay! lets abstract ORM's cause an ORM isn't already an abstraction. Seriously. The number of times people swap infrastructure is pretty damn low. How many times have you spent months working against Sql-Server .. to now get told you're moving to Postgres ... and then 5 months later again to MySql? Same applies across the board. And if you can't make a decision, then enjoy the flip flopping.

Same goes for your 'Storage' infra.

Solution: Write your own IQueueRepository interface in Business, Implement it in a Repository as QueueRepository

HA! I called it, above!

Why are the methods virtual? So you can use dependency injection to create exception handling / retrying / logging just by interception.

Okay - again, I'm not going to get into opinions of what is good vs bad. It's all tabs-vs-spaces. An echo chamber of "i'm right, you're wrong". If you don't like DI/IoC - fine. If you like abstracting everything away because there might be a chance in some future that you might switch providers/infra .. then fine. Good luck.

My example was clean and simple and (most importantly) about how to unit test against the given SDK. If you have some ideological war against virtual methods vs interfaces then go for it. But please don't make false assertions that we cannot mock against the given SDK, given I just showed you a really simple example of it.

Useless Info: I'm personally a fan of interfaces and use em all over the place. I just am more old school and prefer interfaces to virtual methods. I'm not right or wrong. But I do know how to use virtual methods in unit tests and for that, I'm not going to hate at all.

Hope this gives you some background why the rule: "There is no interface" make sense here.

[danneberg]

I understand your point. And I understand the point of Microsoft. And thinking it 3 times I prefer not to have an Interface.

[stewartadam]

var response = new Mock<Response>();

This actually is another caveat testing with the SDK, as since Azure.Response<T> is abstract it can't be instantiated for purposes of returning a Response<BlobFoo> from a unit test. Either the user has to mock the entire Response-wrapped BlobFoo class (as in your example) and then it setup appropriately to return the right values, or the user has to write their own little response wrapper class to wrap the instances produced by the BlobsModelFactory in the unit test. Also good note for the docs.

.ReturnsAsync(response.Object); // Not sure if you really care about the response object here.

Getting the proper response type for testing is how I landed on this GitHub issue, as even if you isolate/abstract dealing with blob storage to a specific class in your app, that logic still needs to get tested. At some point or another, the user's going to want to return the proper Storage SDK domain objects to test that logic.

It's great that the client methods are easily mockable now, but IMO folks consuming the SDK shouldn't have to delve into its source code (or what will eventually be a closed GitHub issue) to figure out the particular factory class that is needed to generate proper SDK return types for their unit tests. It would be great for this note from @seanmcc-msft above to be up-front in the docs:

All v12 client's public APIs are virtual, allowing you mock them using mocking frameworks like MOQ. Return types can be constructed using the models factors, such as Azure.Storage.Blobs.Models.BlobModelFactory.