Unit tests for the Requester

[BenFradet]

Unit tests are required for the Requester class.

[BenFradet]

Additionally, mark the Requester class as internal explicitly.

[shrayasr]

I'm new to writing proper unit tests. Could you guide me through what tests would be required here? I was thinking something like this:

• Make a request to fetch a champion. Check that the request returns a json
• Do the same, but in async
• Make a request to a dummy url, check that it throws an exception
• Do the same, but in async

Is this right?

[BenFradet]

That's the gist of it, yes.

Basically, for each method in Requester, there should be unit tests.

As an example, if we take GetResult there should be two tests:

• GetResult_ShouldSendBackResult_Test() where you check that a succeeding get request sends back the correct result
• GetResult_ShouldSendBackStatusCode_Test() where you check that a failing get request returns the expected status code

[shrayasr]

Great. I've something up at #239. Please do give your comments. I'm really bad at writing Unit tests.

[KittieCrash]

I'll take a look at writing the rest of the unit tests for Requester and start this evening. :smile:

[KittieCrash]

Is there a particular reason you haven't been mocking dependencies out of your unit tests?

For example, in RiotApiTest.GetSummoner_ById_Test, we instantiate the RiotApi class, and by proxy instantiate and use a concrete RateLimitedRequester.

So, if this test fails, we don't have any way of knowing whether or not the error we've introduced is in RiotApi.GetSummoner or RateLimitedRequester.CreateGetRequest. In particular this is troubling if we break one of our very low-level dependencies, as the failing tests can bubble up through the entire project leaving us with the impression that "everything broke".

I can continue to write the Requester tests in the fashion of the rest of the project, but it should be with the understanding that these are really integration tests in disguise if we're not mocking out dependent classes.

Alternatively, we could look at introducing a mocking framework like Moq to our tests, but this is not going to be a trivial debt to pay down.

[BenFradet]

Is there a particular reason you haven't been mocking dependencies out of your unit tests?

That's a fallacy I'm aware of but haven't had the time to fix.

Ideally, as you suggested, we'd introduce a mocking framework but the quantity of work that'd introduce is fairly large and I personally don't have that time to put into this project.

However, feel free to work on that if you feel up to it. I'll create an issue to keep track of it.