search

supabase-community / postgrest-csharp

A C# Client library for Postgrest
https://supabase-community.github.io/postgrest-csharp/api/Postgrest.html
MIT License
114 stars 22 forks source link

Make `ModelledResponse<T>` testable #94

Open compdesigner-nz opened 1 month ago

compdesigner-nz commented 1 month ago

Feature request

Is your feature request related to a problem? Please describe.

Hi. I am trying to unit test the this library and I am coming across the issue of ModeledResponse<T> not being testable.

ModeledResponse<T> does not have a parameterless constructor, meaning that a framework like NSubstitute or Moq cannot create a proxy object easily.

ModeledResponse<T>.Trip/ModeledResponse<T>.Trips are both inaccessible properties making them unusable using a Mock wrapper like what Moq does (see below).

_myMock.Setup(obj => obj.Trip = fakeTripInstance)

Describe the solution you'd like

I would like for either ModeledResponse<T> to have a parameterless constructor and for the JSON deserialization process to happen outside the constructor, or for the behaviour to be contained within some kind of class outside of ModeledResponse<T>.

An alternative is to have ModeledResponse<T> inherit from a new abstraction IModeledResponse<T> where T : BaseModel.

This is probably preferred as it doesn't require a reconsideration of how conversions are handled. This would also mean that IPostgrestTable<T> and Table<T> would need their response types to be updated respectively.

Describe alternatives you've considered

I have tried numerous mocking frameworks with these classes and how their are configured means they are very sealed and unmockable.

Happy to make a PR if need be 😃

compdesigner-nz commented 1 month ago

If the interface route is taken, then potentially there can be two response implementations: ModeledResponse<T> where is the BaseModel, and ModeledResponseCollection<T> where T is the BaseModel. The collection would have an immutable collection of T instances, and the single response would just have the single T instance.

The value add would be: mocking would be easier as you can mock a collection of T or a single T and assign to the mocked instance. Secondly, it means that response values just have the single object store (ICollection<T> or T) which would simplify things a bit. It also would make the interface clearer to the end user of the SDK as to what the endpoint returns.

acupofjose commented 1 month ago

The interface option seems to be the better of the two - Would love a PR if you’re interested in doing one!