How do I test the things

[M-Zuber]

When writing unit tests, you have to write something like the following:

var connection = Substitute.For<IApiConnection>();
var client = new RepositoryPagesClient(connection);
client.Get("fake", "repo");
connection.Received().Get<Page>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/pages"), null);

Without the null on the last line the test will fail on an Ensure check that the uri is null, but it is not clear why it is needed or what it does.

I am mostly confident that the problem is not really with octokit but where can i go to better understand the tools being used?

[haacked]

Hi @M-Zuber, we're using NSubstitute.

But I'm confused by what you wrote. Are you saying that if you pass something other than null you get a test failure? Or if you omit that last argument?

Also, connection.Get returns a Task so you need to set up that return value.

[shiftkey]

@M-Zuber

So NSubstitute takes a different tact to other mocking frameworks by making the proxy object transparent (no Mock<T> anywhere), and using extension methods on the object to verify behaviour. This is the purpose of the .Received() extension method, but it's a bit obscure because it still returns your IApiConnection.

So from the value returned from .Receieved() you then declare the method you expected to be invoked - using the Arg helper class to do expression-based checks of the arguments provided during the test.

Without the null on the last line the test will fail on an Ensure check that the uri is null, but it is not clear why it is needed or what it does.

I gather this is occurring due to the overloads on Get but it's hard to say specifically without a bit more context. So it sounds like the Get<T>(Uri uri) code was failing the test, but Get<T>(Uri uri, IDictionary<string, string> parameters) was making the test pass - does that sound right to you?

Also, if an Ensure check isn't occurring I suspect some actual code is being invoked. Do you recall where that was?

Happy to run up the tests myself in #1061 if there's a specific way to trigger what you're seeing.

[M-Zuber]

But I'm confused by what you wrote. Are you saying that if you pass something other than null you get a test failure? Or if you omit that last argument?

If I omit the last argument.

Also, connection.Get returns a Task so you need to set up that return value.

I was just following most other tests, where all that is being checked is the fact the correct url was touched.

Also, if an Ensure check isn't occurring I suspect some actual code is being invoked. Do you recall where that was?

It is being invoked and therefore failing as it sees the URI as null.

I gather this is occurring due to the overloads on Get but it's hard to say specifically without a bit more context. So it sounds like the Get(Uri uri) code was failing the test, but Get(Uri uri, IDictionary<string, string> parameters) was making the test pass - does that sound right to you?

This sounds exactly right to me, a simple repro is too take the test here and remove the null. (this way you could use code theoretically already on your machine)

What is more confusing for me is that this line: connection.Received().GetAll<RepositoryHook>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/hooks")); does not need the null to pass... It seems to be related to whether the call returns a single object or a List

[shiftkey]

Well, this is interesting:

By dropping the null it now uses an extension method - which explains why you're seeing real code run at that point. However it doesn't quite fully explain why the uri is null, but I'm sure they're related...

[shiftkey]

So we have a few of these extension methods but I'm not sure whether we should:

a) move them back into IApiConnection as they aren't really complicated and will clean up the tests
b) properly investigate why the mock object created isn't handling the received Uri correctly - perhaps it's never worked...

Favouring a) right now (I tested this with Get<T> and was able to remove a few tests which were doing this null anti-pattern to make the tests pass) but I'll let this sit for a bit.

[shiftkey]

Opened #1063 to address this specific problem. Feedback/suggestions welcome.