Skip to content

Add support for unit tests with an example.#5

Open
mattjhosking wants to merge 1 commit into
Zastai:mainfrom
mattjhosking:unit-test-support
Open

Add support for unit tests with an example.#5
mattjhosking wants to merge 1 commit into
Zastai:mainfrom
mattjhosking:unit-test-support

Conversation

@mattjhosking

Copy link
Copy Markdown

I've created a strategy for mocking the responses to the WebClient class and added a sample unit test on the browse releases. Should be relatively easy to build out more tests from here. Let me know what you think.

@Zastai

Zastai commented May 21, 2020

Copy link
Copy Markdown
Owner

Going to hold off on this for a bit.
I'm working on switching to HttpClient instead of WebClient (initially in MetaBrainz.ListenBrainz because of the smaller API surface). That would certainly be impactful to the internals (HttpClient only has async methods, so essentially everything inside Query would become async, with the non-async methods being simple wrappers), and it would also require changes to the mocking.

Plus, I'm not quite sure yet which unit testing framework I'm going to use (although xUnit was what I was leaning towards).

@Zastai

Zastai commented May 21, 2020

Copy link
Copy Markdown
Owner

Once I decide on the unit test framework, I may just start adding tests using Query.Deserialize(); the bulk of the real code is in the converters anyway.

Tests for the rest of the bits (including potentially mocking the client) can then come later.

Comment thread .gitignore
*.*proj.user
*.DotSettings.user
*.userprefs
/_ReSharper.Caches

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd, I've never seen R# create that folder. But then I'm mostly using Rider, so this might be something only the VS plugin does.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I have my ReSharper configured that way so it gets cleaned up regularly.

public class BrowseTests : MusicBrainzTest {

private const string ReleaseTestData = @"{
""releases"":[

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be more inclined to use a .json file stored as embedded resource; the overhead of loading that is small enough, and it makes the test data easier to work with.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - I was thinking this would be a better path to go down but kept the POC simple.

@mattjhosking

Copy link
Copy Markdown
Author

Once I decide on the unit test framework, I may just start adding tests using Query.Deserialize(); the bulk of the real code is in the converters anyway.

Tests for the rest of the bits (including potentially mocking the client) can then come later.

Yeah, that would be ideal for unit testing - break it down into the components and test each phase. There may also be merit in testing that it generates the same serialized JSON requests and URLs between builds with unit tests as well.

@mattjhosking

Copy link
Copy Markdown
Author

Going to hold off on this for a bit.
I'm working on switching to HttpClient instead of WebClient (initially in MetaBrainz.ListenBrainz because of the smaller API surface). That would certainly be impactful to the internals (HttpClient only has async methods, so essentially everything inside Query would become async, with the non-async methods being simple wrappers), and it would also require changes to the mocking.

Plus, I'm not quite sure yet which unit testing framework I'm going to use (although xUnit was what I was leaning towards).

HttpClient would be great. Just watch for the usual HttpClient pit-falls where it doesn't clean up well. Simplest is to provide a constructor for your Query with HttpClient exposes and then they can simply configure .NET Core DI (or similar) to use something like the standard IServiceCollection.AddHttpClient which will automatically inject an appropriate HttpClient instance. https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-3.1

@Zastai

Zastai commented May 21, 2020

Copy link
Copy Markdown
Owner

Yes, that's in part why I suggested avoiding using many short-lived Query objects :)
That way, even if there turn out to be reasons to keep one HttpClient as owned by the Query, the socket exhaustion is unlikely to occur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants