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

How to unit test with ServiceClient #413

Open
SEU2131 opened this issue Nov 14, 2023 · 10 comments
Open

How to unit test with ServiceClient #413

SEU2131 opened this issue Nov 14, 2023 · 10 comments
Labels
enhancement New feature or request General Discussion General Conversation about a feature or feature area

Comments

@SEU2131
Copy link

SEU2131 commented Nov 14, 2023

Hi, we are using Dataverse SDK to connect to SMP. We dependency inject ServiceClient to our class. But when we unit test our code, ServiceClient can't be mocked because when it is initialized, it will connect to dataverse to verify the connection.
So my question is, how can I unit test the build of ServiceClient? Besides, I find there is an internal method used for unit test, do you have plan to public it or add a new method in testHelper to provide a way to unit test? Thanks!

@jordimontana82
Copy link

@SEU2131 ServiceClient implements IOrganizationService and IOrganizationServiceAsync* interfaces. You can inject them and create mocks for them as opposed to the ServiceClient class, unless you really need methods of the ServiceClient directly. This also applies to the CrmServiceClient. Or you can go and find an already existing open source solution that does all that heavy lifting for you.

@FarmerenHE
Copy link

You should look into https://dynamicsvalue.github.io/fake-xrm-easy-docs/quickstart/ its a (license) based tool made to mock the IOrganizationService with support for lots of features

@MattB-msft
Copy link
Member

@SEU2131

We have recently been discussing this internally. General we do not mock ServiceClient, we test ServiceClient independently and Mock only IOrganziation* if we need to simulate a response.

There has been some discussion going on about creating an Interface for the totality of Service Client, inclusive of the IOrganization* interfaces.

However, this carries its own issues and negatives, We are trying to decide if the risk/reward is sufficient to expose an interface for just the service client (without the extensions), given that it will logically be picked up immediately for use in DI and other injection type scenarios.

@MattB-msft MattB-msft added the General Discussion General Conversation about a feature or feature area label Dec 28, 2023
@SEU2131
Copy link
Author

SEU2131 commented Jan 3, 2024

Hi @MattB-msft ,
Thanks for your response. We use interface instead of Service Client when sending request Dataverse. And yes, in DI scenarios, we want to unit test the creation of Service Client, but when New ServiceClient(), it will connect Dataverse and fail. If you could provide testHelper including mock service client, it would really help.

@FarmerenHE
Copy link

Hi @MattB-msft , Thanks for your response. We use interface instead of Service Client when sending request Dataverse. And yes, in DI scenarios, we want to unit test the creation of Service Client, but when New ServiceClient(), it will connect Dataverse and fail. If you could provide testHelper including mock service client, it would really help.

Consider wrapping the ServiceClient in a class that extends the IOrganizationServiceAsync2 interface and then keep the ServiceClient local in that class, when you need to test you can then mock the entire class and that way avoid the issue with ServiceClient not having an interface.

It also adds the benefit of being able to extend the methods that are avaliable
I.e. instead of calling Retrive(EntityLogicalName, EntityId, Columnset).ToEntity() we have an extension that is just "T Retrieve(EntityReference, Columnset)" saving us some work every time we need to fetch an entity.

@SEU2131
Copy link
Author

SEU2131 commented Jan 9, 2024

Hi @FarmerenHE ,
Yes, it's similar with the way we use. We choose to DI the class which creates service clients first (as we need to connect multiple Dataverse), and then DI the class which returns IOrganizationServiceAsync. The second class can get service clients from DI, which is just like your local Service Client.

@MattB-msft
Copy link
Member

@SEU2131 providing a test helper to setup the service client is an option we are considering, effectively we would ship this method / module as its own nuget

@jordimontana82
Copy link

jordimontana82 commented Feb 1, 2024

@MattB-msft just following up on this thread, I think it would A LOT useful if this constructor (or a similar one that takes an IOrganizationService) was made public. It allow us to pass an IOrganizationService interface that mocks all the serivce calls without adding extensions for each interface (corrrect me if I am wrong, but it seems it would allow to use our own mock service for every DataverseServiceClient call that is made internally in the client .... )

internal ServiceClient(IOrganizationService orgSvc, HttpClient httpClient, string baseConnectUrl, Version targetVersion = null, ILogger logger = null)
{
_testOrgSvcInterface = orgSvc;
_logEntry = new DataverseTraceLogger(logger)
{

Calls to the DataverseService property would then return the mocked service:

internal IOrganizationService DataverseService
{
get
{
// Added to support testing of ServiceClient direct code.
if (_testOrgSvcInterface != null)
return _testOrgSvcInterface;

@MattB-msft
Copy link
Member

thanks for the feedback @jordimontana82,

That method was created sole for internal testing of the core client operations itself.,

It was not made public because it would create a perception that you could use it with any IOrganizationService (say inside a plugin) and we did not create rails to protect the client from that. It also does not work for the Async Variations and several of the extension features, also there is not a way to go from an IOrg to an IOrgAsync implementation.

Thats why I pointed to the test helper above, should we ship that, that helper can call that constructor to set up a mock and its very clear its a testing support system.

@MattB-msft MattB-msft added the enhancement New feature or request label Feb 2, 2024
@jordimontana82
Copy link

jordimontana82 commented Feb 2, 2024

Thanks for your quick reply @MattB-msft !

I understand what the goal was when it was declared as internal, it makes sense. If adding a public constructor would cause confusion, maybe adding a factory method instead that would return a ServiceClient instance from our own IOrgService* interfaces? So that is a bit more obvious that is for "testing"?

I've seen the unit test helpers classes in your example above, they are a bit opinionated in that they use Moq and come with other rather simple mocks for some requests. I was just suggesting, instead, a much simpler approach where the devs can decide to inject whatever interface they want as opposed to having to mock all the individual specific extension methods of the client that we don't have on the interfaces.

I see that it wouldn't work for async code, and, at the same time, all the extensions in the client appear to be just synchronous right now https://github.com/microsoft/PowerPlatform-DataverseServiceClient/blob/master/src/GeneralTools/DataverseClient/Client/Extensions/MetadataExtensions.cs#L116

So I think adding that factory method (or a similar solution) would be a little time investment with a great return (i.e. being able to mock all the extension methods fairly easily (i.e. without having to mock each one, wrappers, etc...)).

Maybe all the extension methods will be moved to the IOrganizationServiceAsync* interfaces in future so that they could be be made async?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request General Discussion General Conversation about a feature or feature area
Projects
None yet
Development

No branches or pull requests

4 participants