IPnPContextFactory returns PnPContext instead of IPnPContext - makes mocking difficult

[MartinHatchUK]

So this is in direct reference to "Mocking PnPContext using Moq" #948

In that discussion thread we can see that an IPnPContext interface was added specifically to allow PnPContext to be implemented as a Mock using frameworks like Moq - this was to allow unit testing to work (as PnPContext doesn't have a public constructor).

This can be seen in the latest version where the code summary on the IPnPContext interface event states that is the only reason why this interface exists.

However - I am building a project using Dependency Injection and make reference to IPnPContextFactory

public class SPObservationBlueprintService : IObservationBlueprintService
{
    private readonly SharePointOptions _options;
    private readonly IPnPContextFactory _contextFactory;
    private readonly ILogger _logger;

    public SPObservationBlueprintService(IOptions<SharePointOptions> options, 
                                IPnPContextFactory contextFactory, 
                                ILogger<SPObservationBlueprintService> logger)
    {
        _logger = logger;
        _options = options.Value;
        _contextFactory = contextFactory;
    }

One of the ideas behind this approach is that I can create a mocked IPnPContextFactory object which is passed into the constructor and use that to generate further mocked objects for unit tests.

When I went to start writing my unit tests I was dismayed to find that IPnPContextFactory doesn't return an IPnPContext when creating a new context - and continues to use PnPContext instead (which as we know from the issue referenced above - cannot be mocked)

This means I've had to add an extra constructor to my service PURELY to allow me to use IPnPContext explicitly when writing unit tests.

....

So in summary .. PLEASE can you modify IPnPContextFactory so that it returns IPnPContext objects from it's Create methods, and not explicit PnPContext class objects!

Thank you

Martin

[jansenbe]

Thanks for the feedback @MartinHatchL365 . Returning an IPnPContext instead of a PnPContext makes sense, but I would like to only do that when we're building a new major version (the 2.X series). Seems you're unblocked for the moment.

[MartinHatchUK]

I am technically unblocked - although the solution is undesirable.
(adding constructors that I don't really want my services to have - purely so that I can unit test them)

Appreciate the comments and I look forward to this being implemented.

[jansenbe]

@MartinHatchL365 : labelled to inspect this one when building v2, for now I'm gonna close it to keep the issue list clean.