cloudinary / CloudinaryDotNet

Cloudinary DotNet library
MIT License
102 stars 69 forks source link

Interfaces for easy testing support? #317

Closed PureKrome closed 11 months ago

PureKrome commented 1 year ago

Feature request for Cloudinary .NET SDK

Provide interfaces for the main CloudinaryDotNet classes.

Explain your use case

Has anyone on the team considered adding any interfaces to the CloudinaryDotNet SDK, which would make it way easier for testing scenario's / projects?

If we wish to test some custom service which uses the CloudinaryDotNet SDK/nuget package, then we need to create instances of this class which means it will try and hit the real resources/endpoints.

Versus, if there are some interfaces, then we can just provide that (vid DI/IoC) to any of our own code and when it comes to testings, it's soo much easier.

Currently, we need to create our own custom "wrapper" Cloudinary class + interface, which just has an internal instance of this cloudinary SDK inside it. Works? Sure.. a bit of a pain? yes :(

Describe the problem you’re trying to solve

The main problem we have is that when we write our tests, we don't want to be hitting the real Cloudinary endpoints. This is bad. We just want to pretend we are hitting them and get back a good or bad pretend result and then see how our code deals with that.

Looking at the CloudinaryDotNet TESTS in this repo, I can see that there is a MockedCloudinary class which you folks have created to sorta deal with this. Basically, you're trying to hijack the low level HttpClient response and then let the rest of the code see how it handles. This is GREAT for testing your CloudinaryDotNet SDK because you need to test both the happy path and the bad paths, when bad things happen and to make sure the CloudinaryDotNet SDK can handles all the various scenario's. Great! But when it comes to consumers of your library, then should NOT KNOW of these low level details. I shouldn't be having to deal with faking the response of the HTTPClient. I should even KNOW what this SDK is doing behind the scenes!

We should just be dealing with the public methods and their results (good and bad).

Interfaces enable this -> easily!

Do you have a proposed solution?

Sure do!

Add interfaces to the public classes :)

For example, with the class public class Cloudinary, generate an interface for all of these public properties and methods.

Then we can have some nice code that looks like this:

public interface ICloudinary
{
    Task<ImageUploadResult> UploadAsync(ImageUploadParams parameters, CancellationToken? cancellationToken = null);
}

public class Cloudinary: ICloudinary
{
    // Ctor, all the same methods, etc.

    public Task<ImageUploadResult> UploadAsync(ImageUploadParams parameters, CancellationToken? cancellationToken = null)
    { ... }
}

public class MyService : IMyService
{
    private readonly ICloudinary _cloudinary;

    public MyService(ICloudinary cloudinary)
    {
        _cloudinary = ICloudinary;
    }

    public async Task DoSomethingAsync()
    {
        // some stuff ...

        await result = _cloudinary.UploadAsync(someImageUploadParams, cancellationToken);

        // some more stuff ...
    }
}

public class DoSomethingAsyncTests
{
    [Fact]
    public async Task GivenSomeImage_DoSomethingAsync_ShouldUploadTheImage()
    {
        // Arrange.
        var imageUploadResult = new ImageUploadResult(); // Whatever result data goes in here.
        var cloudinary = new Mock<ICloudinary>();
        cloudinary
            .Setup(x => x.UploadAsync(It.Is<ImageUploadParams>(iup => ....), It.IsAny<CancelltionToken>())
            .ReturnsAsync(imageUploadResult);

        var service = new MyService(cloudinary.Object);

       // Act.
       await service.DoSomethingAsync();

       // Assert.
       cloudinary.VerifyAll(); // Makes sure that our code did end up calling Cloudinary with the _specific_ 
    }
}
wissam-khalili commented 1 year ago

Hi @PureKrome,

Thank you for this idea. I will forward your suggestion to our team and will keep you posted.

Thanks, Wissam

PureKrome commented 11 months ago

Thank you!!!!

const-cloudinary commented 11 months ago

@PureKrome, thank you for raising the issue and providing such a great description!