github · grokys · Aug 9, 2016 · Aug 8, 2016 · Aug 8, 2016 · Aug 8, 2016
diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs
@@ -4,12 +4,20 @@

 namespace GitHub.Services
 {
+    [NullGuard.NullGuard(NullGuard.ValidationFlags.None)]
    [Export(typeof(IPullRequestService))]
    [PartCreationPolicy(CreationPolicy.Shared)]
    public class PullRequestService : IPullRequestService
    {
        public IObservable<IPullRequestModel> CreatePullRequest(IRepositoryHost host, ISimpleRepositoryModel repository, string title, string body, IBranch source, IBranch target)
        {
+            Extensions.Guard.ArgumentNotNull(host, nameof(host));
+            Extensions.Guard.ArgumentNotNull(repository, nameof(repository));
+            Extensions.Guard.ArgumentNotNull(title, nameof(title));
+            Extensions.Guard.ArgumentNotNull(body, nameof(body));
+            Extensions.Guard.ArgumentNotNull(source, nameof(source));
+            Extensions.Guard.ArgumentNotNull(target, nameof(target));
+
            return host.ModelService.CreatePullRequest(repository, title, body, source, target);
        }
    }

diff --git a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs
@@ -17,6 +17,7 @@
 using System.Reactive;
 using System.Diagnostics.CodeAnalysis;
 using Octokit;
+using NLog;

 namespace GitHub.ViewModels
 {
@@ -25,6 +26,8 @@ namespace GitHub.ViewModels
    [SuppressMessage("Microsoft.Design", "CA1001:TypesThatOwnDisposableFieldsShouldBeDisposable")]
    public class PullRequestCreationViewModel : BaseViewModel, IPullRequestCreationViewModel
    {
+        static readonly Logger log = LogManager.GetCurrentClassLogger();
+
        readonly IRepositoryHost repositoryHost;
        readonly ISimpleRepositoryModel activeRepo;
        readonly Subject<Unit> initializationComplete = new Subject<Unit>();
@@ -36,7 +39,7 @@ public class PullRequestCreationViewModel : BaseViewModel, IPullRequestCreationV
             IPullRequestService service, INotificationService notifications)
             : this(connectionRepositoryHostMap.CurrentRepositoryHost, teservice.ActiveRepo, service, notifications)
         {}
-        
+
        public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ISimpleRepositoryModel activeRepo,
            IPullRequestService service, INotificationService notifications)
        {
@@ -78,18 +81,18 @@ public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ISimpleRepos
                .Subscribe(x => notifications.ShowError(BranchValidator.ValidationResult.Message));

            createPullRequest = ReactiveCommand.CreateAsyncObservable(whenAnyValidationResultChanges,
-                _ => service.CreatePullRequest(repositoryHost, activeRepo, PRTitle, Description, SourceBranch, TargetBranch)
-            );
-            createPullRequest.ThrownExceptions.Subscribe(ex =>
-            {
-                if (!ex.IsCriticalException())
-                {
-                    //TODO:Will need a uniform solution to HTTP exception message handling
-                    var apiException = ex as ApiValidationException;
-                    var error = apiException?.ApiError?.Errors?.FirstOrDefault();
-                    notifications.ShowError(error?.Message ?? ex.Message);
-                }
-            });
+                _ => service
+                    .CreatePullRequest(repositoryHost, activeRepo, PRTitle, Description ?? String.Empty, SourceBranch, TargetBranch)
+                    .Catch<IPullRequestModel, Exception>(ex =>
+                    {
+                        log.Error(ex);
+
+                        //TODO:Will need a uniform solution to HTTP exception message handling
+                        var apiException = ex as ApiValidationException;
+                        var error = apiException?.ApiError?.Errors?.FirstOrDefault();
+                        notifications.ShowError(error?.Message ?? ex.Message);
+                        return Observable.Empty<IPullRequestModel>();
+                    }));
        }

        public override void Initialize([AllowNull] ViewWithData data)

diff --git a/src/UnitTests/GitHub.App/Models/RepositoryModelTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryModelTests.cs
@@ -60,7 +60,6 @@ public class PathConstructorTests : TempFileBaseClass
        public void NoRemoteUrl()
        {
            var provider = Substitutes.ServiceProvider;
-            Services.PackageServiceProvider = provider;
            var gitservice = provider.GetGitService();
            var repo = Substitute.For<IRepository>();
            var path = Directory.CreateSubdirectory("repo-name");
@@ -73,7 +72,6 @@ public void NoRemoteUrl()
        public void WithRemoteUrl()
        {
            var provider = Substitutes.ServiceProvider;
-            Services.PackageServiceProvider = provider;
            var gitservice = provider.GetGitService();
            var repo = Substitute.For<IRepository>();
            var path = Directory.CreateSubdirectory("repo-name");

diff --git a/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs b/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs
@@ -0,0 +1,48 @@
+using System.Reactive.Linq;
+using System.Threading.Tasks;
+using NSubstitute;
+using Xunit;
+using UnitTests;
+using GitHub.Models;
+using System;
+using GitHub.Services;
+
+public class PullRequestServiceTests : TestBaseClass
+{
+    [Fact]
+    public async Task CreatePullRequestAllArgsMandatory()
+    {
+        var serviceProvider = Substitutes.ServiceProvider;
+        var service = new PullRequestService();
+
+        IRepositoryHost host = null;
+        ISimpleRepositoryModel repository = null;
+        string title = null;
+        string body = null;
+        IBranch source = null;
+        IBranch target = null;
+
+        Assert.Throws<ArgumentNullException>(() => service.CreatePullRequest(host, repository, title, body, source, target));
+
+        host = serviceProvider.GetRepositoryHosts().GitHubHost;
+        Assert.Throws<ArgumentNullException>(() => service.CreatePullRequest(host, repository, title, body, source, target));
+
+        repository = new SimpleRepositoryModel("name", new GitHub.Primitives.UriString("http://github.com/github/stuff"));
+        Assert.Throws<ArgumentNullException>(() => service.CreatePullRequest(host, repository, title, body, source, target));
+
+        title = "a title";
+        Assert.Throws<ArgumentNullException>(() => service.CreatePullRequest(host, repository, title, body, source, target));
+
+        body = "a body";
+        Assert.Throws<ArgumentNullException>(() => service.CreatePullRequest(host, repository, title, body, source, target));
+
+        source = new BranchModel() { Name = "source" };
+        Assert.Throws<ArgumentNullException>(() => service.CreatePullRequest(host, repository, title, body, source, target));
+
+        target = new BranchModel() { Name = "target" };
+        var pr = await service.CreatePullRequest(host, repository, title, body, source, target);
+
+        Assert.NotNull(pr);
+    }
+
+}
diff --git a/src/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs
@@ -0,0 +1,36 @@
+using System.Reactive.Linq;
+using System.Threading.Tasks;
+using NSubstitute;
+using Xunit;
+using UnitTests;
+using GitHub.Models;
+using System;
+using GitHub.Services;
+using GitHub.ViewModels;
+
+public class PullRequestCreationViewModelTests : TempFileBaseClass
+{
+    [Fact]
+    public async Task NullDescriptionBecomesEmptyBody()
+    {
+        var serviceProvider = Substitutes.ServiceProvider;
+        var service = new PullRequestService();
+        var notifications = Substitute.For<INotificationService>();
+
+        var host = serviceProvider.GetRepositoryHosts().GitHubHost;
+        var ms = Substitute.For<IModelService>();
+        host.ModelService.Returns(ms);
+
+        var repository = new SimpleRepositoryModel("name", new GitHub.Primitives.UriString("http://github.com/github/stuff"));
+        var title = "a title";
+
+        var vm = new PullRequestCreationViewModel(host, repository, service, notifications);
+        vm.SourceBranch = new BranchModel() { Name = "source" };
+        vm.TargetBranch = new BranchModel() { Name = "target" };
+        vm.PRTitle = title;
+
+        await vm.CreatePullRequest.ExecuteAsync();
+        var unused = ms.Received().CreatePullRequest(repository, vm.PRTitle, String.Empty, vm.SourceBranch, vm.TargetBranch);
+    }
+
+}
diff --git a/src/UnitTests/GitHub.Exports/SimpleRepositoryModelTests.cs b/src/UnitTests/GitHub.Exports/SimpleRepositoryModelTests.cs
@@ -13,7 +13,6 @@ public class SimpleRepositoryModelTests : TempFileBaseClass
    void SetupRepository(string sha)
    {
        var provider = Substitutes.ServiceProvider;
-        Services.PackageServiceProvider = provider;
        var gitservice = provider.GetGitService();
        var repo = Substitute.For<IRepository>();
        gitservice.GetRepo(Args.String).Returns(repo);

diff --git a/src/UnitTests/Properties/AssemblyInfo.cs b/src/UnitTests/Properties/AssemblyInfo.cs
@@ -1,6 +1,7 @@
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using Xunit;

 // General Information about an assembly is controlled through the following 
 // set of attributes. Change these attribute values to modify the information
@@ -34,3 +35,4 @@
 // [assembly: AssemblyVersion("1.0.*")]
 [assembly: AssemblyVersion("1.0.0.0")]
 [assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: CollectionBehavior(CollectionBehavior.CollectionPerAssembly, DisableTestParallelization = true)]
diff --git a/src/UnitTests/Substitutes.cs b/src/UnitTests/Substitutes.cs
@@ -2,6 +2,7 @@
 using GitHub.Authentication;
 using GitHub.Models;
 using GitHub.Services;
+using GitHub.VisualStudio;
 using Microsoft.VisualStudio.ComponentModelHost;
 using NSubstitute;
 using Rothko;
@@ -65,6 +66,7 @@ public static IOperatingSystem OperatingSystem
        public static IConnectionManager ConnectionManager { get { return Substitute.For<IConnectionManager>(); } }
        public static ITwoFactorChallengeHandler TwoFactorChallengeHandler { get { return Substitute.For<ITwoFactorChallengeHandler>(); } }
        public static IGistPublishService GistPublishService { get { return Substitute.For<IGistPublishService>(); } }
+        public static IPullRequestService PullRequestService { get { return Substitute.For<IPullRequestService>(); } }

        /// <summary>
        /// This returns a service provider with everything mocked except for 
@@ -106,6 +108,7 @@ public static IServiceProvider GetServiceProvider(
            cc.ComposeExportedValue(gitservice);
            ((IComponentModel)cm).DefaultExportProvider.Returns(cc);
            ret.GetService(typeof(SComponentModel)).Returns(cm);
+            Services.PackageServiceProvider = ret;

            var os = OperatingSystem;
            var vs = IVSServices;
@@ -126,6 +129,7 @@ public static IServiceProvider GetServiceProvider(
            ret.GetService(typeof(IAvatarProvider)).Returns(avatarProvider);
            ret.GetService(typeof(ITwoFactorChallengeHandler)).Returns(TwoFactorChallengeHandler);
            ret.GetService(typeof(IGistPublishService)).Returns(GistPublishService);
+            ret.GetService(typeof(IPullRequestService)).Returns(PullRequestService);
            return ret;
        }

@@ -197,5 +201,10 @@ public static IGistPublishService GetGistPublishService(this IServiceProvider pr
        {
            return provider.GetService(typeof(IGistPublishService)) as IGistPublishService;
        }
+
+        public static IPullRequestService GetPullRequestsService(this IServiceProvider provider)
+        {
+            return provider.GetService(typeof(IPullRequestService)) as IPullRequestService;
+        }
    }
 }
diff --git a/src/UnitTests/UnitTests.csproj b/src/UnitTests/UnitTests.csproj
@@ -166,11 +166,13 @@
    <Compile Include="GitHub.App\Models\RepositoryModelTests.cs" />
    <Compile Include="GitHub.App\Services\AvatarProviderTests.cs" />
    <Compile Include="GitHub.App\Services\GitClientTests.cs" />
+    <Compile Include="GitHub.App\Services\PullRequestServiceTests.cs" />
    <Compile Include="GitHub.App\Services\RepositoryCloneServiceTests.cs" />
    <Compile Include="GitHub.App\Services\RepositoryCreationServiceTests.cs" />
    <Compile Include="GitHub.App\ViewModels\GistCreationViewModelTests.cs" />
    <Compile Include="GitHub.App\ViewModels\LoginControlViewModelTests.cs" />
    <Compile Include="GitHub.App\ViewModels\LoginToGitHubViewModelTests.cs" />
+    <Compile Include="GitHub.App\ViewModels\PullRequestCreationViewModelTests.cs" />
    <Compile Include="GitHub.App\ViewModels\PullRequestListViewModelTests.cs" />
    <Compile Include="GitHub.App\ViewModels\RepositoryCloneViewModelTests.cs" />
    <Compile Include="GitHub.App\ViewModels\RepositoryCreationViewModelTests.cs" />