Fixed: merging config vs. CLI globbing (#403)

close #402

Author: Aaronontheweb

src/Incrementalist.Cmd/Config/ConfigMerger.cs

@@ -5,6 +5,7 @@
 // -----------------------------------------------------------------------
 
 using System;
+using System.Linq;
 
 namespace Incrementalist.Cmd.Config
 {
@@ -42,8 +43,8 @@ public static SlnOptions Merge(SlnOptions options, IncrementalistConfig? config)
             merged.ContinueOnError = config.ContinueOnError.GetValueOrDefault(true);
             merged.RunInParallel = config.RunInParallel.GetValueOrDefault(false);
             merged.FailOnNoProjects = config.FailOnNoProjects.GetValueOrDefault(false);
-            merged.SkipGlobs = options.SkipGlobs ?? config.SkipGlob;
-            merged.TargetGlobs = options.TargetGlobs ?? config.TargetGlob;
+            merged.SkipGlobs = MergeGlobs(options.SkipGlobs?.ToArray(), config.SkipGlob);
+            merged.TargetGlobs = MergeGlobs(options.TargetGlobs?.ToArray(), config.TargetGlob);
 
             // Merge int properties (CLI takes precedence)
             merged.TimeoutMinutes = config.TimeoutMinutes.GetValueOrDefault(2);
@@ -62,6 +63,15 @@ public static SlnOptions Merge(SlnOptions options, IncrementalistConfig? config)
             return merged;
         }
 
+        private static string[] MergeGlobs(string[]? primary, string[]? secondary)
+        {
+            if(primary is { Length: > 0 })
+                return primary;
+            if (secondary is { Length: > 0 })
+                return secondary;
+            return [];
+        }
+
         /// <summary>
         /// Apply default values to any null properties.
         /// </summary>

src/Incrementalist.Tests/Config/ConfigFileTests.cs

@@ -98,7 +98,9 @@ public void Merge_Config_With_CommandLine_Options()
                 TimeoutMinutes = 5,
                 Verbose = true,
                 RunInParallel = true,
-                OutputFile = "output.txt"
+                OutputFile = "output.txt",
+                SkipGlob = ["**/obj/**", "**/bin/**"],
+                TargetGlob = ["src/**/*.csproj", "tests/**/*.csproj"]
             };
 
             var options = new RunOptions()
@@ -123,6 +125,8 @@ public void Merge_Config_With_CommandLine_Options()
             Assert.Equal("output.txt", merged.OutputFile); // From config
             Assert.True(merged.Verbose); // From config
             Assert.True(merged.RunInParallel); // From config
+            Assert.Equivalent(config.SkipGlob, merged.SkipGlobs); // From config
+            Assert.Equivalent(config.TargetGlob, merged.TargetGlobs); // From config
 
             // Command-specific options should be preserved
             Assert.Equal(3, merged.DotNetArgs.Length);

src/Incrementalist.Tests/Config/SlnOptionsParsingSpecs.cs

@@ -6,6 +6,7 @@
 
 using System.Linq;
 using Incrementalist.Cmd;
+using Incrementalist.Cmd.Config;
 using Microsoft.CodeAnalysis;
 using Xunit;
 using static Incrementalist.Cmd.SlnOptionsParser;
@@ -34,4 +35,35 @@ public void ShouldSeparateDotnetArgsFromSlnOptions(string cliArg)
         Assert.Equal(0, r);
         Assert.NotNull(result);
     }
+    
+    /*
+     *  Looking for a suspected bug where:
+     * 
+     *  1. No globs specified on command line
+     *  2. globs specified in config file
+     *  3. Parse operation supplies an empty array instead of a null array
+     *  4. Merge operation fails because it uses the empty array instead of the config file globs
+     */
+    [Fact]
+    public void ShouldKeepConfigGlobs()
+    {
+        string[] expectedGlobs = ["**/*.Tests.csproj", "**/tests/*.csproj"];
+        var args = CommandLineParser
+            .SplitCommandLineIntoArguments("run", true).ToArray();
+        var r = TryParseSlnOptions(args, out SlnOptions? result);
+
+        Assert.Equal(0, r);
+        Assert.NotNull(result);
+        
+        var config = new IncrementalistConfig
+        {
+            TargetGlob = null,
+            SkipGlob = expectedGlobs,
+            GitBranch = "dev",
+            WorkingDirectory = @"C:\user\workingdir"
+        };
+        
+        var merged = ConfigMerger.Merge(result, config);
+        Assert.Equivalent(expectedGlobs, merged.SkipGlobs);
+    }
 }