Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Make it possible to abstract LibGit2Sharp away in testing context #138

Closed
nulltoken opened this Issue · 23 comments

5 participants

@nulltoken
Owner

As rightfully stated, some work could be done in order to make LibGit2Sharp a more test-friendly component.

/cc @anglicangeek @Haacked @tclem

@nulltoken
Owner

@dahlbyk @spraints If you have some requirements as well, now's the time to voice them :)

@dahlbyk
Collaborator

For now I'll assume my requirements will be covered by others'.

@Haacked
Owner

The BlueLine branch has some work I did in this regard. Simply adding interfaces for the "interesting" types and public constructors for the simpler types went a long way to enabling me to abstract LibGit2Sharp and test my code. I really wanted to avoid wrappers for everything. :)

@nulltoken
Owner

That's certainly a good starting point. ..And I'm confident we should be able to deliver a set of granular and cohesive commits. Along with tests helping the reader understand how to properly leverage those changes ... for testing purposes. ;)

@spraints

What @Haacked said.

For git-tfs, I'm currently focused on high-level, integration-type tests, and I'm planning to leave git in the loop.

For lower-level tests, I would want to mock out libgit2sharp classes. I'm not sure of the current state of .NET mocking libraries, but if enough of them can mock a concrete class, then maybe marking public methods virtual would be adequate?

@Haacked
Owner

Mocking concrete classes are only possible if you can call their constructors. Unfortunately, many of the types in libgit2sharp have internal constructors which makes mocking the concrete classes impossible without resorting to Profiler intercetpion tricks, which I think most TDD practitioners are going to want to avoid just to test their own code.

@nulltoken
Owner

@Haacked Thanks for #170. However, I'd prefer to actually start from the requirements.

Let's select one standard scenario, and, actually, write the client code (the testing code).
Then we'll decide what changes would be required to make the test pass against a "simulated" version of LibGit2Sharp.

@Haacked
Owner

Most of these changes were cherry picked from BlueLine. The client code is GitHub for Windows so it represents real scenarios.

@nulltoken
Owner

The client code is GitHub for Windows so it represents real scenarios.

I have no doubt about this :) However, as written above, I'd like to have in LibGit2Sharp.Tests some tests helping the API consumer understand how to properly isolate LibGit2Sharp.

@Haacked
Owner
@nulltoken
Owner

Are there existing tests of proper isolation for those classes I can look at as examples of what you mean?

No, there aren't any yet. This is currently the whole point of this issue. Providing the users with an easy and supported way to isolate LibGit2Sharp.

@Haacked
Owner

Yeah, but I just see it as the product of good design, not as a separate feature that requires tests itself. In the same way that abstract base classes were chosen in this code base without tests to prove that they provide the right level of isolation. They were chosen as part of a design process and the only tests are those that test implementation.

I think writing a contrived client, and then tests for that client, is not going to produce useful tests and just creates a ton of busy work for me.

Now, if I had infinite time, I think the "proper" way to test this would be to re-write all the existing tests so they isolate the class being tested and mock out the other classes that might be needed. However, that's also a huge amount of work for a change that feels "obvious" to me. Also, the existing tests are great integration tests so we wouldn't want to lose them either.

A couple of ideas come to mind.

What we did with GitHub for Windows was create 2 test suites. One for unit tests and one for integration. If you want to go that route, we could do that here. And perhaps convert a sample of existing tests to isolated unit tests as an example for future tests.

Otherwise, maybe I could rewrite a single test suite as an example of isolation. I'd just like to get these changes in promptly since we have to maintain a separate branch with our interface changes for GHfW.

@nulltoken
Owner

maybe I could rewrite a single test suite as an example of isolation

Ok. Let's do this.

@Haacked
Owner

Sent a new pull request: #182

This was referenced
@nulltoken
Owner

@half-ogre, @spraints, @dahlbyk, @Haacked Could you peek at #185?

@nulltoken
Owner

Fixed by #185 and #187

@nulltoken nulltoken closed this
@Haacked
Owner

Unfortunately MoQ has a bug when working with classes that it doesn't have when working with interfaces so I'm dead in the water at the moment. :( :( :(

@dahlbyk
Collaborator

That's easy...fix the bug.

@Haacked
Owner

Too busy fixing other bugs. :)

@nulltoken
Owner

It looks like @yorah is already working on this... see Moq/moq4#9 , Moq/moq4#14 and Moq/moq4#19

@kzu

Merged @yorah pull request. Should be out to nuget next week with a bunch of other fixes.

Thanks!

@nulltoken
Owner

@danielkzu Awesome! Thanks for letting us know!

@nulltoken
Owner

@Haacked It looks like a new version of MoQ has been released on September, 11th as a NuGet package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.