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

Make ModelledResponse<T> testable #94

Open
compdesigner-nz opened this issue May 8, 2024 · 2 comments
Open

Make ModelledResponse<T> testable #94

compdesigner-nz opened this issue May 8, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@compdesigner-nz
Copy link

compdesigner-nz commented May 8, 2024

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 compdesigner-nz added the enhancement New feature or request label May 8, 2024
@compdesigner-nz
Copy link
Author

compdesigner-nz commented May 8, 2024

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
Copy link
Member

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

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

No branches or pull requests

2 participants