Moving logic to validate a new branch name to a testable location; Adding unit tests to check boundary conditions

Author: StanleyGoldman

src/GitHub.Api/GitHub.Api.csproj

@@ -104,6 +104,7 @@
     <Compile Include="Application\ApplicationManagerBase.cs" />
     <Compile Include="Helpers\Constants.cs" />
     <Compile Include="Cache\IBranchCache.cs" />
+    <Compile Include="Helpers\RegularExpressions.cs" />
     <Compile Include="Platform\DefaultEnvironment.cs" />
     <Compile Include="Extensions\EnvironmentExtensions.cs" />
     <Compile Include="Extensions\FileEventExtensions.cs" />

src/GitHub.Api/Helpers/RegularExpressions.cs

@@ -0,0 +1,20 @@
+using System.Text.RegularExpressions;
+
+namespace GitHub.Unity.Helpers
+{
+    public static class RegularExpressions
+    {
+        public static readonly Regex BranchNameRegex = new Regex(@"^(?<name>[\w\d\/\-_]+)$");
+    }
+
+    public static class BranchNameValidator
+    {
+        public static bool IsBranchNameValid(string branchName)
+        {
+            return !string.IsNullOrEmpty(branchName)
+                && !branchName.StartsWith("/")
+                && !branchName.EndsWith("/")
+                && RegularExpressions.BranchNameRegex.IsMatch(branchName);
+        }
+    }
+}

src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Utility.cs

@@ -28,7 +28,6 @@ class Utility : ScriptableObject
         public static readonly Regex StatusAheadBehindRegex =
                                          new Regex(
                                              @"\[ahead (?<ahead>\d+), behind (?<behind>\d+)\]|\[ahead (?<ahead>\d+)\]|\[behind (?<behind>\d+>)\]");
-        public static readonly Regex BranchNameRegex = new Regex(@"^(?<name>[\w\d\/\-\_]+)$");
 
         private static bool ready;
         private static Action onReady;

src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using GitHub.Unity.Helpers;
 using UnityEditor;
 using UnityEngine;
 using Debug = System.Diagnostics.Debug;
@@ -453,10 +454,7 @@ private void OnCreateGUI()
                     var cancelCreate = false;
                     var cannotCreate = selectedNode == null ||
                                        selectedNode.Type == NodeType.Folder ||
-                                       newBranchName == null ||
-                                       newBranchName.StartsWith("/") ||
-                                       newBranchName.EndsWith("/") ||
-                                       !Utility.BranchNameRegex.IsMatch(newBranchName);
+                                       !BranchNameValidator.IsBranchNameValid(newBranchName);
 
                     // Create on return/enter or cancel on escape
                     var offsetID = GUIUtility.GetControlID(FocusType.Passive);

src/tests/UnitTests/Git/BranchNameValidationTests.cs

@@ -0,0 +1,53 @@
+using System;
+using System.Linq;
+using FluentAssertions;
+using GitHub.Unity.Helpers;
+using NUnit.Framework;
+
+namespace UnitTests
+{
+    [TestFixture]
+    public class BranchNameValidatorTests
+    {
+        [TestCase(true, "feature1", TestName = "Branch name is valid")]
+        [TestCase(true, "feature-1", TestName = "Branch name with hyphen is valid")]
+        [TestCase(true, "feature.1", TestName = "Branch name can contain dots")]
+        [TestCase(true, "feature..1", TestName = "Branch name cannot contain consecutive dots")]
+        [TestCase(false, "feature 1", TestName = "Branch name cannot contain a space")]
+        [TestCase(false, "feature~1", TestName = "Branch name cannot contain a ~")]
+        [TestCase(false, "feature^1", TestName = "Branch name cannot contain a ^")]
+        [TestCase(false, "feature:1", TestName = "Branch name cannot contain a :")]
+        [TestCase(false, "feature?1", TestName = "Branch name cannot contain a ?")]
+        [TestCase(false, "feature*1", TestName = "Branch name cannot contain a *")]
+        [TestCase(false, "feature[1", TestName = "Branch name cannot contain a [")]
+        [TestCase(false, "/feature1", TestName = "Branch name cannot begin with a slash")]
+        [TestCase(false, "feature1/", TestName = "Branch name cannot end with a slash")]
+        [TestCase(false, "feature1.", TestName = "Branch name cannot end with a dot")]
+        [TestCase(false, "feature1.lock", TestName = "Branch name cannot end with .lock")]
+        [TestCase(true, "a", TestName = "Single character is valid")]
+        [TestCase(false, "@", TestName = "Single character cannot be @")]
+        [TestCase(false, ".", TestName = "Single character cannot be [period]")]
+        [TestCase(true, "features/feature-1", TestName = "Folder and branch name is valid")]
+        [TestCase(true, "features\\feature-1", TestName = "Backslash is not a valid character")]
+        [TestCase(true, ".hidden", TestName = "Branch name is valid when starting with [period]")]
+        [TestCase(false, ".features/feature-1", TestName = "Folder and branch name is not valid when starting with [period]")]
+        [TestCase(false, "features//feature-1", TestName = "Multiple consecutive slashes are not valid")]
+        [TestCase(false, null, TestName = "null string is not valid")]
+        [TestCase(false, "", TestName = "Empty string is not valid")]
+        [TestCase(false, "/", TestName = "Single slash is not valid")]
+        [TestCase(false, "asdf@{", TestName = "Sequence @{ is not valid")]
+        public void TestFeatureString(bool isValid, string branch)
+        {
+            BranchNameValidator.IsBranchNameValid(branch).Should().Be(isValid);
+        }
+
+        [TestCase(true, 45, 45, 45, TestName = "Can test with ascii values")]
+        [TestCase(false, 45, 45, 39, TestName = "No individual ASCII value should be < octal(40)")]
+        [TestCase(false, 45, 45, 177, TestName = "No individual ASCII value should = octal(177)")]
+        public void TestFeatureStringFromAsciiArray(bool isValid, params int[] asciiValues)
+        {
+            var branch = new string(asciiValues.Select(Convert.ToChar).ToArray());
+            BranchNameValidator.IsBranchNameValid(branch).Should().Be(isValid);
+        }
+    }
+}

src/tests/UnitTests/UnitTests.csproj

@@ -81,6 +81,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="Authentication\KeychainTests.cs" />
+    <Compile Include="Git\BranchNameValidationTests.cs" />
     <Compile Include="Git\GitConfigTests.cs" />
     <Compile Include="IO\BranchListOutputProcessorTests.cs" />
     <Compile Include="Extensions\EnvironmentExtensionTests.cs" />