diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index f1df35a486..9a27bf7dcd 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -224,6 +224,7 @@ public async Task CloneRepository( catch (Exception ex) { log.Error(ex, "Could not clone {CloneUrl} to {Path}", cloneUrl, repositoryPath); + operatingSystem.Directory.DeleteDirectory(repositoryPath); throw; } } diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index d3941f8798..28ca979afe 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -1,24 +1,24 @@ -using System.Reactive.Linq; +using System; +using System.Linq.Expressions; +using System.Reactive.Linq; using System.Threading.Tasks; +using GitHub.Api; +using GitHub.Models; +using GitHub.Services; using NSubstitute; using NUnit.Framework; -using UnitTests; -using GitHub.Services; -using System.Linq.Expressions; -using System; -using GitHub.Models; +using Rothko; public class RepositoryCloneServiceTests { - public class TheCloneRepositoryMethod : TestBaseClass + public class TheCloneRepositoryMethod { [Test] public async Task ClonesToRepositoryPathAsync() { - var serviceProvider = Substitutes.GetServiceProvider(); - var operatingSystem = serviceProvider.GetOperatingSystem(); - var vsGitServices = serviceProvider.GetVSGitServices(); - var cloneService = CreateRepositoryCloneService(serviceProvider); + var operatingSystem = Substitute.For(); + var vsGitServices = Substitute.For(); + var cloneService = CreateRepositoryCloneService(operatingSystem, vsGitServices); await cloneService.CloneRepository("https://github.com/foo/bar", @"c:\dev\bar"); @@ -34,9 +34,9 @@ public async Task ClonesToRepositoryPathAsync() [TestCase("https://enterprise.com/foo/bar", 0, nameof(UsageModel.MeasuresModel.NumberOfGitHubClones))] public async Task UpdatesMetricsWhenRepositoryClonedAsync(string cloneUrl, int numberOfCalls, string counterName) { - var serviceProvider = Substitutes.GetServiceProvider(); - var usageTracker = serviceProvider.GetUsageTracker(); - var cloneService = CreateRepositoryCloneService(serviceProvider); + var vsGitServices = Substitute.For(); + var usageTracker = Substitute.For(); + var cloneService = CreateRepositoryCloneService(vsGitServices: vsGitServices, usageTracker: usageTracker); await cloneService.CloneRepository(cloneUrl, @"c:\dev\bar"); var model = UsageModel.Create(Guid.NewGuid()); @@ -58,8 +58,9 @@ public async Task Skip_OpenRepository_When_Already_Open(string repositoryPath, s { var repositoryUrl = "https://github.com/owner/repo"; var cloneDialogResult = new CloneDialogResult(repositoryPath, repositoryUrl); - var serviceProvider = Substitutes.GetServiceProvider(); - var operatingSystem = serviceProvider.GetOperatingSystem(); + var operatingSystem = Substitute.For(); + var serviceProvider = Substitute.For(); + var teamExplorerServices = Substitute.For(); operatingSystem.Directory.DirectoryExists(repositoryPath).Returns(true); var dte = Substitute.For(); serviceProvider.GetService().Returns(dte); @@ -68,11 +69,11 @@ public async Task Skip_OpenRepository_When_Already_Open(string repositoryPath, s { operatingSystem.Directory.DirectoryExists(solutionPath).Returns(true); } - var cloneService = CreateRepositoryCloneService(serviceProvider); + var cloneService = CreateRepositoryCloneService(operatingSystem: operatingSystem, + teamExplorerServices: teamExplorerServices, serviceProvider: serviceProvider); await cloneService.CloneOrOpenRepository(cloneDialogResult); - var teamExplorerServices = serviceProvider.GetTeamExplorerServices(); teamExplorerServices.Received(openRepository).OpenRepository(repositoryPath); } @@ -91,11 +92,10 @@ public async Task UpdatesMetricsWhenCloneOrOpenRepositoryAsync(string cloneUrl, { var repositoryPath = @"c:\dev\bar"; var cloneDialogResult = new CloneDialogResult(repositoryPath, cloneUrl); - var serviceProvider = Substitutes.GetServiceProvider(); - var operatingSystem = serviceProvider.GetOperatingSystem(); + var operatingSystem = Substitute.For(); + var usageTracker = Substitute.For(); operatingSystem.Directory.DirectoryExists(repositoryPath).Returns(dirExists); - var usageTracker = serviceProvider.GetUsageTracker(); - var cloneService = CreateRepositoryCloneService(serviceProvider); + var cloneService = CreateRepositoryCloneService(operatingSystem: operatingSystem, usageTracker: usageTracker); await cloneService.CloneOrOpenRepository(cloneDialogResult); @@ -108,11 +108,10 @@ await usageTracker.Received(numberOfCalls).IncrementCounter( [TestCase(@"c:\not_default\repo", @"c:\default", 0, nameof(UsageModel.MeasuresModel.NumberOfClonesToDefaultClonePath))] public async Task UpdatesMetricsWhenDefaultClonePath(string targetPath, string defaultPath, int numberOfCalls, string counterName) { - var serviceProvider = Substitutes.GetServiceProvider(); - var vsGitServices = serviceProvider.GetVSGitServices(); + var vsGitServices = Substitute.For(); vsGitServices.GetLocalClonePathFromGitProvider().Returns(defaultPath); - var usageTracker = serviceProvider.GetUsageTracker(); - var cloneService = CreateRepositoryCloneService(serviceProvider); + var usageTracker = Substitute.For(); + var cloneService = CreateRepositoryCloneService(usageTracker: usageTracker, vsGitServices: vsGitServices); await cloneService.CloneRepository("https://github.com/foo/bar", targetPath); var model = UsageModel.Create(Guid.NewGuid()); @@ -122,10 +121,36 @@ await usageTracker.Received(numberOfCalls).IncrementCounter( ((MemberExpression)x.Body).Member.Name == counterName)); } - static RepositoryCloneService CreateRepositoryCloneService(IGitHubServiceProvider sp) + [TestCase("https://github.com/failing/url", @"c:\dev\bar")] + public async Task CleansDirectoryOnCloneFailed(string cloneUrl, string clonePath) { - return new RepositoryCloneService(sp.GetOperatingSystem(), sp.GetVSGitServices(), sp.GetTeamExplorerServices(), - sp.GetGraphQLClientFactory(), sp.GetGitHubContextService(), sp.GetUsageTracker(), sp); + var operatingSystem = Substitute.For(); + var vsGitServices = Substitute.For(); + vsGitServices.Clone(cloneUrl, clonePath, true).Returns(x => { throw new Exception(); }); + var cloneService = CreateRepositoryCloneService(operatingSystem: operatingSystem, vsGitServices: vsGitServices); + + Assert.ThrowsAsync(() => cloneService.CloneRepository(cloneUrl, clonePath)); + + operatingSystem.Directory.Received().CreateDirectory(clonePath); + operatingSystem.Directory.Received().DeleteDirectory(clonePath); + await vsGitServices.Received().Clone(cloneUrl, clonePath, true); + } + + static RepositoryCloneService CreateRepositoryCloneService(IOperatingSystem operatingSystem = null, + IVSGitServices vsGitServices = null, IUsageTracker usageTracker = null, + ITeamExplorerServices teamExplorerServices = null, IGitHubServiceProvider serviceProvider = null) + { + operatingSystem = operatingSystem ?? Substitute.For(); + vsGitServices = vsGitServices ?? Substitute.For(); + usageTracker = usageTracker ?? Substitute.For(); + teamExplorerServices = teamExplorerServices ?? Substitute.For(); + serviceProvider = serviceProvider ?? Substitute.For(); + + operatingSystem.Environment.ExpandEnvironmentVariables(Args.String).Returns(x => x[0]); + + return new RepositoryCloneService(operatingSystem, vsGitServices, teamExplorerServices, + Substitute.For(), Substitute.For(), + usageTracker, serviceProvider); } } } diff --git a/test/GitHub.App.UnitTests/Substitutes.cs b/test/GitHub.App.UnitTests/Substitutes.cs index ac707407f8..a3b0255e74 100644 --- a/test/GitHub.App.UnitTests/Substitutes.cs +++ b/test/GitHub.App.UnitTests/Substitutes.cs @@ -30,32 +30,8 @@ public static T1 For(params object[] constructorArguments) }, constructorArguments); } - - // public static IGitRepositoriesExt IGitRepositoriesExt { get { return Substitute.For(); } } public static IGitService IGitService { get { return Substitute.For(); } } - public static IVSGitServices IVSGitServices - { - get - { - var ret = Substitute.For(); - ret.GetLocalClonePathFromGitProvider().Returns(@"c:\foo\bar"); - return ret; - } - } - - public static IOperatingSystem OperatingSystem - { - get - { - var ret = Substitute.For(); - // this expansion happens when the GetLocalClonePathFromGitProvider call is setup by default - // see IVSServices property above - ret.Environment.ExpandEnvironmentVariables(Args.String).Returns(x => x[0]); - return ret; - } - } - public static IViewViewModelFactory ViewViewModelFactory { get { return Substitute.For(); } } public static IRepositoryCreationService RepositoryCreationService { get { return Substitute.For(); } } @@ -109,9 +85,8 @@ public static IGitHubServiceProvider GetServiceProvider( ret.GetService(typeof(SComponentModel)).Returns(cm); Services.UnitTestServiceProvider = ret; - var os = OperatingSystem; - var vsgit = IVSGitServices; - var clone = cloneService ?? new RepositoryCloneService(os, vsgit, Substitute.For(), + var clone = cloneService ?? new RepositoryCloneService(Substitute.For(), + Substitute.For(), Substitute.For(), Substitute.For(), Substitute.For(), Substitute.For(), ret); var create = creationService ?? new RepositoryCreationService(clone); @@ -123,8 +98,8 @@ public static IGitHubServiceProvider GetServiceProvider( ret.GetService(typeof(IGitHubContextService)).Returns(Substitute.For()); ret.GetService(typeof(IVSGitExt)).Returns(Substitute.For()); ret.GetService(typeof(IUsageTracker)).Returns(Substitute.For()); - ret.GetService(typeof(IVSGitServices)).Returns(vsgit); - ret.GetService(typeof(IOperatingSystem)).Returns(os); + ret.GetService(typeof(IVSGitServices)).Returns(Substitute.For()); + ret.GetService(typeof(IOperatingSystem)).Returns(Substitute.For()); ret.GetService(typeof(IRepositoryCloneService)).Returns(clone); ret.GetService(typeof(IRepositoryCreationService)).Returns(create); ret.GetService(typeof(IViewViewModelFactory)).Returns(ViewViewModelFactory);