Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

shana
Copy link
Contributor

@shana shana commented Sep 18, 2015

This PR is based on #79, so that one should be merged first before looking at this.

Add repository model tests and support for testing things that rely on the GetUri and GetRepo extension methods that use libgit2#.

Testing RepositoryModel creation via the path constructor implies hitting libgit2sharp codepaths, and libgit2sharp doesn't expose nice interfaces for testing (why??). So, move things around and add a
GitService that replaces the extension methods that depend on libgit2sharp, so we can at least mock at a higher level.

The things that require this GitService are not created via mef, so expose it via the Services static class.

Testing RepositoryModel creation via the path constructor implies
hitting libgit2sharp codepaths, and libgit2sharp doesn't expose nice
interfaces for testing (why??). So, move things around and add a
GitService that replaces the extension methods that depend on
libgit2sharp, so we can at least mock at a higher level.

The things that require this GitService are not created via mef, so
expose it via the Services static class.
@shana shana changed the title Shana/repository tests Add repository model tests Sep 18, 2015
@shana shana mentioned this pull request Sep 18, 2015
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This got moved up to VSExtensions 'cause it makes sense there.

The prior code always created a `IGitService` instance, but we only need
to create them if there are any active repositories.

Also changed to use `.Any()` because it is more expressive.
Also we should prefer to use `var` unless the type's really not clear at
all from the right hand side. Also, I prefer all the usings on top and
not within namespaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

@shana: So according to R#, there is no type in the solution that inherits from both EnvDTE.DTE and EnvDTE80.DTE2. That leads me to believe that the Dte2 property is always null. Also, I can't find any usages of the Dte2 property. Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, don't believe everything R# says. These are COM interfaces, the cast is

It's going to be used in the future, so I keep it around, along with some other handy magical vs types (that are all probably used now, but weren't in the beginning).

@haacked
Copy link
Contributor

haacked commented Sep 18, 2015

🍍

@haacked
Copy link
Contributor

haacked commented Sep 18, 2015

@shana: If you're ok with my changes, this is good to merge. The only outstanding question is the Dte2 property. I have a concern about touching the file system in the tests, but let's not worry about that right now.

@shana
Copy link
Contributor Author

shana commented Sep 18, 2015

@haacked Changes look 👍!

haacked added a commit that referenced this pull request Sep 18, 2015
@haacked haacked merged commit 067be61 into master Sep 18, 2015
@haacked haacked deleted the shana/repository-tests branch September 18, 2015 21:48
@haacked
Copy link
Contributor

haacked commented Sep 18, 2015

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants