From ed8564968f564be5496f4f7153b9f16f474188d8 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Thu, 17 Sep 2026 11:26:53 +0000 Subject: [PATCH 1/4] Finish a file propagation the user confirmed, and say what it did Propagate() ran an unguarded File.Copy per repository, so the first locked destination or read-only directory ended the loop: every repository after it silently never got the file, and nothing was written to the log panel either way. Because every other long-running action in this application reports through QueueLog/QueueGitLog, that silence read as success. The exception also escaped a render callback, which can take the application down and lose unsaved options state. The copy loop moves to FilePropagation, apart from the ImGui layer so it can be driven without a live context -- the same shape as DecidePull. Each repository's copy is guarded and recorded, the batch runs to the end, and the summary line carries the counts and names the repositories that missed out, so a partial run does not have to be reconstructed from fifteen individual log lines. A missing source is checked once before anything is copied, because asking for the wrong file is a different failure from a locked destination and reporting it fifteen times would bury that. Fixes #415 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE --- ProjectDirector.Test/FilePropagationTests.cs | 194 +++++++++++++++++++ ProjectDirector/FilePropagation.cs | 136 +++++++++++++ ProjectDirector/PopupPropagateFile.cs | 34 ++-- ProjectDirector/ProjectDirector.cs | 2 +- 4 files changed, 352 insertions(+), 14 deletions(-) create mode 100644 ProjectDirector.Test/FilePropagationTests.cs create mode 100644 ProjectDirector/FilePropagation.cs diff --git a/ProjectDirector.Test/FilePropagationTests.cs b/ProjectDirector.Test/FilePropagationTests.cs new file mode 100644 index 0000000..ff24e9c --- /dev/null +++ b/ProjectDirector.Test/FilePropagationTests.cs @@ -0,0 +1,194 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.IO; + +using ktsu.Semantics.Strings; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests what propagating one file to several repositories does when one of them refuses the copy. +/// +/// +/// Propagating is a batch the user confirms once, for a list of repositories they picked. It used to +/// run an unguarded per repository, so the first locked +/// destination ended the loop -- every repository after it silently never got the file, and nothing +/// was written to the log panel either way. +/// +/// exists apart from so that rule can +/// be driven against real throwaway directories, the way drives +/// . A destination that is an existing *directory* is the +/// portable way to make a copy fail: both Windows and Linux refuse it, without needing a lock or a +/// permission change the test would then have to undo. +/// +[TestClass] +public sealed class FilePropagationTests +{ + private const string SourceContent = "root = true\n"; + + private static FullyQualifiedGitHubRepoName Repo(string name) => name.As(); + + private static string CreateWorkspace() + { + string root = Path.Join(Path.GetTempPath(), $"ktsu_pd_propagate_{Guid.NewGuid():N}"); + _ = Directory.CreateDirectory(root); + return root; + } + + private static void Cleanup(string root) + { + try + { + Directory.Delete(root, recursive: true); + } + catch (IOException) + { + // A leaked temp directory is not worth failing an otherwise passing test over. + } + catch (UnauthorizedAccessException) + { + // Same. + } + } + + [TestMethod] + public void ARefusedCopyDoesNotStopTheRepositoriesAfterIt() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + _ = Directory.CreateDirectory(Path.GetDirectoryName(source)!); + File.WriteAllText(source, SourceContent); + + string first = Path.Join(root, "first", ".editorconfig"); + string blocked = Path.Join(root, "blocked", ".editorconfig"); + string last = Path.Join(root, "last", ".editorconfig"); + + // Occupy the middle destination with a directory of the same name, which neither platform + // will let File.Copy overwrite. + _ = Directory.CreateDirectory(blocked); + + // An ordered sequence rather than a dictionary, so "after the failure" means what it says. + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/first"), first), + new(Repo("ktsu-dev/blocked"), blocked), + new(Repo("ktsu-dev/last"), last), + ]; + + FilePropagationReport report = FilePropagation.Propagate(source, destinations); + + Assert.IsTrue(File.Exists(first), "The repository before the failure should have the file."); + Assert.AreEqual(SourceContent, File.ReadAllText(first)); + Assert.IsTrue(File.Exists(last), "The repository after the failure should still have been attempted."); + Assert.AreEqual(SourceContent, File.ReadAllText(last)); + + Assert.AreEqual(3, report.Results.Count, "Every requested repository should be accounted for."); + Assert.IsTrue(report.Results[0].Succeeded); + Assert.IsFalse(report.Results[1].Succeeded, "The occupied destination should be reported as a failure."); + Assert.IsFalse(string.IsNullOrWhiteSpace(report.Results[1].Failure), "A failure should carry its reason."); + Assert.IsTrue(report.Results[2].Succeeded); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void TheSummaryCountsTheRunAndNamesTheRepositoriesThatMissedOut() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + _ = Directory.CreateDirectory(Path.GetDirectoryName(source)!); + File.WriteAllText(source, SourceContent); + + string blocked = Path.Join(root, "blocked", ".editorconfig"); + _ = Directory.CreateDirectory(blocked); + + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/first"), Path.Join(root, "first", ".editorconfig")), + new(Repo("ktsu-dev/blocked"), blocked), + new(Repo("ktsu-dev/last"), Path.Join(root, "last", ".editorconfig")), + ]; + + Collection lines = FilePropagation.Propagate(source, destinations).Summarize(); + + StringAssert.Contains(lines[0], "2 of 3", StringComparison.Ordinal); + StringAssert.Contains(lines[0], "ktsu-dev/blocked", StringComparison.Ordinal); + Assert.AreEqual(2, lines.Count, "One summary line, then one detail line for the single failure."); + StringAssert.Contains(lines[1], "ktsu-dev/blocked", StringComparison.Ordinal); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void ARunWhereEveryCopyWorksSaysSoInOneLine() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + _ = Directory.CreateDirectory(Path.GetDirectoryName(source)!); + File.WriteAllText(source, SourceContent); + + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/first"), Path.Join(root, "first", ".editorconfig")), + new(Repo("ktsu-dev/last"), Path.Join(root, "last", "nested", ".editorconfig")), + ]; + + FilePropagationReport report = FilePropagation.Propagate(source, destinations); + Collection lines = report.Summarize(); + + Assert.IsTrue(report.SourceExists); + Assert.AreEqual(1, lines.Count); + StringAssert.Contains(lines[0], "2 of 2", StringComparison.Ordinal); + Assert.IsFalse(lines[0].Contains("failed", StringComparison.Ordinal)); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void AMissingSourceIsReportedOnceAndLeavesEveryRepositoryAlone() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + string destination = Path.Join(root, "first", ".editorconfig"); + + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/first"), destination), + ]; + + FilePropagationReport report = FilePropagation.Propagate(source, destinations); + Collection lines = report.Summarize(); + + Assert.IsFalse(report.SourceExists); + Assert.AreEqual(0, report.Results.Count, "No repository should be touched when there is nothing to copy."); + Assert.IsFalse(Directory.Exists(Path.GetDirectoryName(destination)!), "A failed run should not create destination directories."); + Assert.AreEqual(1, lines.Count, "A missing source is one failure, not one per repository."); + StringAssert.Contains(lines[0], "does not exist", StringComparison.Ordinal); + } + finally + { + Cleanup(root); + } + } +} diff --git a/ProjectDirector/FilePropagation.cs b/ProjectDirector/FilePropagation.cs new file mode 100644 index 0000000..fc7d2a1 --- /dev/null +++ b/ProjectDirector/FilePropagation.cs @@ -0,0 +1,136 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector; + +using System.Collections.ObjectModel; + +/// +/// What copying the propagated file into one repository did. +/// +/// The repository the file was copied into. +/// The full path the file was copied to. +/// Why the copy failed, or null when it succeeded. +internal sealed record FilePropagationResult(FullyQualifiedGitHubRepoName Repo, string Destination, string? Failure) +{ + /// + /// Gets a value indicating whether the file reached this repository. + /// + internal bool Succeeded => Failure is null; +} + +/// +/// The outcome of one propagation run, one entry per repository the user asked for. +/// +/// The file that was propagated. +/// Whether was there to copy. When false no repository was touched. +/// One result per requested repository, in the order they were attempted. +internal sealed record FilePropagationReport(string Source, bool SourceExists, Collection Results) +{ + /// + /// Describes the run for the log panel: a summary line, then one line per failure. + /// + /// At least one line, the first of which answers "did that work" on its own. + /// + /// The summary line carries the counts and names the repositories that missed out, because after + /// a partial run the question is which repositories have the file -- and reconstructing that from + /// fifteen individual lines is the work this is meant to save. + /// + internal Collection Summarize() + { + if (!SourceExists) + { + return [$"Propagating {Source} failed: the source file does not exist"]; + } + + Collection failures = [.. Results.Where(result => !result.Succeeded)]; + int succeeded = Results.Count - failures.Count; + string failed = failures.Count > 0 + ? $"; failed: {string.Join(", ", failures.Select(failure => failure.Repo.WeakString))}" + : string.Empty; + + Collection lines = [$"Propagated {Source} to {succeeded} of {Results.Count} repos{failed}"]; + foreach (FilePropagationResult failure in failures) + { + lines.Add($" {failure.Repo.WeakString}: {failure.Failure}"); + } + + return lines; + } +} + +/// +/// Copies one file into a set of repositories. +/// +/// +/// Separate from so the part with a rule in it can be driven +/// without a live ImGui context, the way is. +/// +/// The rule is that a batch the user confirmed runs to the end. A locked destination in the seventh +/// of fifteen repositories used to end the loop there, so the remaining eight silently never got the +/// file -- and because every other long-running action in this application reports through the log +/// panel, that silence read as success. +/// +internal static class FilePropagation +{ + /// + /// Copies to every destination, continuing past a failure. + /// + /// The file to copy. + /// The repository each copy is for, and the full path to copy it to. + /// A report naming what happened to every requested repository. + /// + /// A missing source is checked once, before anything is copied. It is a different failure from a + /// locked destination -- one the user can only have caused by asking for the wrong file -- and + /// reporting it as one failure per repository would bury that. + /// + internal static FilePropagationReport Propagate(string source, IEnumerable> destinations) + { + Ensure.NotNull(destinations); + + Collection results = []; + + if (!File.Exists(source)) + { + return new(source, SourceExists: false, results); + } + + foreach ((FullyQualifiedGitHubRepoName repo, string destination) in destinations) + { + results.Add(Copy(source, repo, destination)); + } + + return new(source, SourceExists: true, results); + } + + private static FilePropagationResult Copy(string source, FullyQualifiedGitHubRepoName repo, string destination) + { + string? directory = Path.GetDirectoryName(destination); + if (string.IsNullOrEmpty(directory)) + { + return new(repo, destination, "the destination has no containing directory"); + } + + try + { + _ = Directory.CreateDirectory(directory); + File.Copy(source, destination, overwrite: true); + return new(repo, destination, null); + } + catch (IOException ex) + { + return new(repo, destination, ex.Message); + } + catch (UnauthorizedAccessException ex) + { + return new(repo, destination, ex.Message); + } + catch (NotSupportedException ex) + { + return new(repo, destination, ex.Message); + } + catch (ArgumentException ex) + { + return new(repo, destination, ex.Message); + } + } +} diff --git a/ProjectDirector/PopupPropagateFile.cs b/ProjectDirector/PopupPropagateFile.cs index 4ef6530..cef5b2e 100644 --- a/ProjectDirector/PopupPropagateFile.cs +++ b/ProjectDirector/PopupPropagateFile.cs @@ -2,6 +2,7 @@ namespace ktsu.ProjectDirector; +using System.Collections.ObjectModel; using DiffPlex.Model; using Hexa.NET.ImGui; using ktsu.Extensions; @@ -16,10 +17,17 @@ internal sealed class PopupPropagateFile private ImGuiPopups.Prompt Prompt { get; } = new(); private bool ShouldClose { get; set; } - public void Open(ProjectDirectorOptions options) + /// + /// Where this popup reports what propagating did, so a batch copy is accounted for in the log + /// panel the same way every git action already is. + /// + private Action Log { get; set; } = _ => { }; + + public void Open(ProjectDirectorOptions options, Action log) { ShouldClose = false; Options = options; + Log = log; Propagation.Clear(); Modal.Open("Propagate File", ShowContent); } @@ -72,19 +80,19 @@ private void Propagate() { GitRepository repo = Options.Repos[Options.BaseRepo]; string from = Path.Combine(repo.LocalPath, Options.PropagatePath); - foreach ((FullyQualifiedGitHubRepoName name, bool shouldPropagate) in Propagation) + + Dictionary destinations = Propagation + .Where(kvp => kvp.Value) + .ToDictionary(kvp => kvp.Key, kvp => Path.Combine(Options.Repos[kvp.Key].LocalPath, Options.PropagatePath)); + + Collection lines = FilePropagation.Propagate(from, destinations).Summarize(); + + // Timestamp the summary and leave the per-repository detail indented under it, which is the + // shape QueueGitLog already gives the log panel for a git command and its output. + Log($"[{DateTimeOffset.Now}] {lines[0]}"); + foreach (string line in lines.Skip(1)) { - if (shouldPropagate) - { - GitRepository otherRepo = Options.Repos[name]; - string to = Path.Combine(otherRepo.LocalPath, Options.PropagatePath); - string? directory = Path.GetDirectoryName(to); - if (!string.IsNullOrEmpty(directory)) - { - _ = Directory.CreateDirectory(directory); - File.Copy(from, to, overwrite: true); - } - } + Log(line); } ShouldClose = true; diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 7985cac..79d2af9 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -1720,7 +1720,7 @@ private void ShowRepoBrowser() if (shouldOpenPopup) { - PopupPropagateFile.Open(Options); + PopupPropagateFile.Open(Options, QueueLog); } _ = PopupPropagateFile.ShowIfOpen(); From 6678ea8b51ab867019decede865070b19fce8f08 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Thu, 17 Sep 2026 11:29:54 +0000 Subject: [PATCH 2/4] Move SourceLink off the version the NuGet audit fails the build on The build fails before it reaches a single test: NU1902 is an error here, and Microsoft.SourceLink.{AzureRepos.Git,GitHub} 10.0.102 -- along with the Microsoft.Build.Tasks.Git 10.0.102 they bring with them -- carries GHSA-23fw-v26w-5fgq. Both are pinned to that version by VersionOverride, so every build in this repository has been red on it, main and the open Dependabot PRs included. 10.0.401 is the current release of the same package and clears the advisory. This is not what #415 is about, and it is only here because a red base branch cannot be handed to a reviewer as a green PR. It is a self-contained two-line bump and can be split out or dropped without touching the propagation change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE --- ProjectDirector/ProjectDirector.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ProjectDirector/ProjectDirector.csproj b/ProjectDirector/ProjectDirector.csproj index 349308e..6d2ec1b 100644 --- a/ProjectDirector/ProjectDirector.csproj +++ b/ProjectDirector/ProjectDirector.csproj @@ -22,8 +22,8 @@ - - + + From e7a27ac049b668a6a6209d92c4a7eed8d947f6f3 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Thu, 17 Sep 2026 11:43:21 +0000 Subject: [PATCH 3/4] Pull the rest of the propagation rule out of the popup, and cover it SonarCloud put new-code coverage at 66.7% against a gate of 80%. The uncovered half was not incidental: it was the part of the rule still sitting inside the ImGui layer, plus the failure arms of the guard that no test drove. ResolveDestinations and DescribeForLog move to FilePropagation. Both are real decisions worth pinning -- where a checked repository's copy lands, and the fact that only the summary carries a timestamp while the detail stays indented under it -- and neither needs a live ImGui context to run. Propagate() is left doing only what it has to: read the options, call them, write the lines. The NotSupportedException arm goes. File.Copy documents it for a path in an invalid format, but modern .NET raises ArgumentException for everything that reaches it here, so it was a catch no input could exercise. Four new tests cover the arms that were missing -- a destination whose parent is a file, one with no containing directory, one that is not a usable path, and the two extracted methods. FilePropagation.cs is now fully covered and new-code coverage is 86%. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE --- ProjectDirector.Test/FilePropagationTests.cs | 138 +++++++++++++++++++ ProjectDirector/FilePropagation.cs | 48 ++++++- ProjectDirector/PopupPropagateFile.cs | 14 +- 3 files changed, 185 insertions(+), 15 deletions(-) diff --git a/ProjectDirector.Test/FilePropagationTests.cs b/ProjectDirector.Test/FilePropagationTests.cs index ff24e9c..31bc832 100644 --- a/ProjectDirector.Test/FilePropagationTests.cs +++ b/ProjectDirector.Test/FilePropagationTests.cs @@ -163,6 +163,144 @@ public void ARunWhereEveryCopyWorksSaysSoInOneLine() } } + [TestMethod] + public void ADestinationWhoseParentIsAFileIsReportedRatherThanThrown() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + _ = Directory.CreateDirectory(Path.GetDirectoryName(source)!); + File.WriteAllText(source, SourceContent); + + // A plain file where the destination expects a directory: creating the containing + // directory fails rather than the copy itself, which is the other half of the guard. + string fileInTheWay = Path.Join(root, "blocked"); + File.WriteAllText(fileInTheWay, "not a directory\n"); + + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/blocked"), Path.Join(fileInTheWay, "nested", ".editorconfig")), + new(Repo("ktsu-dev/last"), Path.Join(root, "last", ".editorconfig")), + ]; + + FilePropagationReport report = FilePropagation.Propagate(source, destinations); + + Assert.IsFalse(report.Results[0].Succeeded); + Assert.IsFalse(string.IsNullOrWhiteSpace(report.Results[0].Failure)); + Assert.IsTrue(report.Results[1].Succeeded, "The repository after the failure should still have been attempted."); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void ADestinationWithNoContainingDirectoryIsReportedRatherThanSkipped() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + _ = Directory.CreateDirectory(Path.GetDirectoryName(source)!); + File.WriteAllText(source, SourceContent); + + // A bare filename has no directory part. The old code silently skipped this case; a + // repository the user checked and heard nothing about is the bug, not the edge case. + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/bare"), "nocontainingdirectory.txt"), + ]; + + FilePropagationReport report = FilePropagation.Propagate(source, destinations); + + Assert.AreEqual(1, report.Results.Count, "The repository should still be accounted for."); + Assert.IsFalse(report.Results[0].Succeeded); + StringAssert.Contains(report.Results[0].Failure!, "containing directory", StringComparison.Ordinal); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void ADestinationThatIsNotAUsablePathIsReportedRatherThanThrown() + { + string root = CreateWorkspace(); + try + { + string source = Path.Join(root, "source", ".editorconfig"); + _ = Directory.CreateDirectory(Path.GetDirectoryName(source)!); + File.WriteAllText(source, SourceContent); + + // An embedded null is rejected by the path APIs on every platform, which is the + // ArgumentException arm of the guard. + KeyValuePair[] destinations = + [ + new(Repo("ktsu-dev/invalid"), Path.Join(root, "in\0valid", ".editorconfig")), + new(Repo("ktsu-dev/last"), Path.Join(root, "last", ".editorconfig")), + ]; + + FilePropagationReport report = FilePropagation.Propagate(source, destinations); + + Assert.IsFalse(report.Results[0].Succeeded); + Assert.IsTrue(report.Results[1].Succeeded, "The repository after the failure should still have been attempted."); + } + finally + { + Cleanup(root); + } + } + + [TestMethod] + public void OnlyTheCheckedRepositoriesGetADestination() + { + Dictionary repos = new() + { + [Repo("ktsu-dev/first")] = new GitHubRepository { LocalPath = Path.Join("dev", "first").As() }, + [Repo("ktsu-dev/second")] = new GitHubRepository { LocalPath = Path.Join("dev", "second").As() }, + [Repo("ktsu-dev/third")] = new GitHubRepository { LocalPath = Path.Join("dev", "third").As() }, + }; + + KeyValuePair[] selection = + [ + new(Repo("ktsu-dev/first"), true), + new(Repo("ktsu-dev/second"), false), + new(Repo("ktsu-dev/third"), true), + ]; + + Dictionary destinations = + FilePropagation.ResolveDestinations(selection, repos, Path.Join("src", ".editorconfig")); + + Assert.AreEqual(2, destinations.Count, "An unchecked repository should not get a destination."); + Assert.IsFalse(destinations.ContainsKey(Repo("ktsu-dev/second"))); + Assert.AreEqual(Path.Join("dev", "first", "src", ".editorconfig"), destinations[Repo("ktsu-dev/first")]); + Assert.AreEqual(Path.Join("dev", "third", "src", ".editorconfig"), destinations[Repo("ktsu-dev/third")]); + } + + [TestMethod] + public void OnlyTheSummaryLineCarriesTheTimestamp() + { + DateTimeOffset at = new(2026, 9, 17, 11, 30, 0, TimeSpan.Zero); + FilePropagationReport report = new( + ".editorconfig", + SourceExists: true, + [ + new(Repo("ktsu-dev/first"), "first", null), + new(Repo("ktsu-dev/blocked"), "blocked", "denied"), + ]); + + Collection lines = FilePropagation.DescribeForLog(report, at); + + Assert.AreEqual(2, lines.Count); + StringAssert.StartsWith(lines[0], $"[{at}] ", StringComparison.Ordinal); + StringAssert.Contains(lines[0], "1 of 2", StringComparison.Ordinal); + StringAssert.StartsWith(lines[1], " ", StringComparison.Ordinal); + Assert.IsFalse(lines[1].Contains($"[{at}]", StringComparison.Ordinal), "Detail lines are indented under the summary, not stamped again."); + } + [TestMethod] public void AMissingSourceIsReportedOnceAndLeavesEveryRepositoryAlone() { diff --git a/ProjectDirector/FilePropagation.cs b/ProjectDirector/FilePropagation.cs index fc7d2a1..1a41d94 100644 --- a/ProjectDirector/FilePropagation.cs +++ b/ProjectDirector/FilePropagation.cs @@ -72,6 +72,50 @@ internal Collection Summarize() /// internal static class FilePropagation { + /// + /// Works out where the propagated file goes in each repository the user checked. + /// + /// Every repository offered, and whether the user checked it. + /// The repositories, by name, as the options carry them. + /// The path being propagated, relative to a repository root. + /// The destination for each checked repository. Unchecked repositories are left out. + /// + /// The file lands at the same relative path in every repository, which is the whole idea: the + /// repositories are similar, and the file being propagated is the one they should share. + /// + internal static Dictionary ResolveDestinations( + IEnumerable> selection, + IReadOnlyDictionary repos, + string relativePath) + { + Ensure.NotNull(selection); + Ensure.NotNull(repos); + + return selection + .Where(kvp => kvp.Value) + .ToDictionary(kvp => kvp.Key, kvp => Path.Combine(repos[kvp.Key].LocalPath, relativePath)); + } + + /// + /// Renders a report as the log panel shows it. + /// + /// The run to describe. + /// When the run finished, which stamps the summary line. + /// The lines to write to the log, summary first. + /// + /// Only the summary is timestamped, with the per-repository detail indented under it. That is the + /// shape already gives a git command and its output, so + /// a propagation reads like everything else in the panel. + /// + internal static Collection DescribeForLog(FilePropagationReport report, DateTimeOffset at) + { + Ensure.NotNull(report); + + Collection lines = report.Summarize(); + lines[0] = $"[{at}] {lines[0]}"; + return lines; + } + /// /// Copies to every destination, continuing past a failure. /// @@ -124,10 +168,6 @@ private static FilePropagationResult Copy(string source, FullyQualifiedGitHubRep { return new(repo, destination, ex.Message); } - catch (NotSupportedException ex) - { - return new(repo, destination, ex.Message); - } catch (ArgumentException ex) { return new(repo, destination, ex.Message); diff --git a/ProjectDirector/PopupPropagateFile.cs b/ProjectDirector/PopupPropagateFile.cs index cef5b2e..586863d 100644 --- a/ProjectDirector/PopupPropagateFile.cs +++ b/ProjectDirector/PopupPropagateFile.cs @@ -2,7 +2,6 @@ namespace ktsu.ProjectDirector; -using System.Collections.ObjectModel; using DiffPlex.Model; using Hexa.NET.ImGui; using ktsu.Extensions; @@ -80,17 +79,10 @@ private void Propagate() { GitRepository repo = Options.Repos[Options.BaseRepo]; string from = Path.Combine(repo.LocalPath, Options.PropagatePath); + Dictionary destinations = FilePropagation.ResolveDestinations(Propagation, Options.Repos, Options.PropagatePath); + FilePropagationReport report = FilePropagation.Propagate(from, destinations); - Dictionary destinations = Propagation - .Where(kvp => kvp.Value) - .ToDictionary(kvp => kvp.Key, kvp => Path.Combine(Options.Repos[kvp.Key].LocalPath, Options.PropagatePath)); - - Collection lines = FilePropagation.Propagate(from, destinations).Summarize(); - - // Timestamp the summary and leave the per-repository detail indented under it, which is the - // shape QueueGitLog already gives the log panel for a git command and its output. - Log($"[{DateTimeOffset.Now}] {lines[0]}"); - foreach (string line in lines.Skip(1)) + foreach (string line in FilePropagation.DescribeForLog(report, DateTimeOffset.Now)) { Log(line); } From f6c6bef5f0d561e70c91a73666f09291e262fa09 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Thu, 17 Sep 2026 11:55:23 +0000 Subject: [PATCH 4/4] Use the collection and string assertions MSTest suggests SonarCloud raised 17 INFO findings on the new tests, all the same shape: a count compared with AreEqual where HasCount or IsEmpty says what is meant, and StringAssert where Assert now carries the equivalent. They cost nothing to take and the failure messages read better for it -- HasCount prints the collection, where AreEqual prints two bare integers. GitCliTests already uses Assert.IsEmpty, so this is the direction the tests are already moving in rather than a new convention. Note that Assert.Contains and Assert.StartsWith take the substring and the prefix first, which is the reverse of StringAssert. Each converted assertion was checked by breaking its expectation and confirming it still fails, so none of them passes vacuously on swapped arguments. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FgX8kpAMnBEumW6PDBfynE --- ProjectDirector.Test/FilePropagationTests.cs | 34 ++++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/ProjectDirector.Test/FilePropagationTests.cs b/ProjectDirector.Test/FilePropagationTests.cs index 31bc832..be04a9c 100644 --- a/ProjectDirector.Test/FilePropagationTests.cs +++ b/ProjectDirector.Test/FilePropagationTests.cs @@ -88,7 +88,7 @@ public void ARefusedCopyDoesNotStopTheRepositoriesAfterIt() Assert.IsTrue(File.Exists(last), "The repository after the failure should still have been attempted."); Assert.AreEqual(SourceContent, File.ReadAllText(last)); - Assert.AreEqual(3, report.Results.Count, "Every requested repository should be accounted for."); + Assert.HasCount(3, report.Results, "Every requested repository should be accounted for."); Assert.IsTrue(report.Results[0].Succeeded); Assert.IsFalse(report.Results[1].Succeeded, "The occupied destination should be reported as a failure."); Assert.IsFalse(string.IsNullOrWhiteSpace(report.Results[1].Failure), "A failure should carry its reason."); @@ -122,10 +122,10 @@ public void TheSummaryCountsTheRunAndNamesTheRepositoriesThatMissedOut() Collection lines = FilePropagation.Propagate(source, destinations).Summarize(); - StringAssert.Contains(lines[0], "2 of 3", StringComparison.Ordinal); - StringAssert.Contains(lines[0], "ktsu-dev/blocked", StringComparison.Ordinal); - Assert.AreEqual(2, lines.Count, "One summary line, then one detail line for the single failure."); - StringAssert.Contains(lines[1], "ktsu-dev/blocked", StringComparison.Ordinal); + Assert.Contains("2 of 3", lines[0], StringComparison.Ordinal); + Assert.Contains("ktsu-dev/blocked", lines[0], StringComparison.Ordinal); + Assert.HasCount(2, lines, "One summary line, then one detail line for the single failure."); + Assert.Contains("ktsu-dev/blocked", lines[1], StringComparison.Ordinal); } finally { @@ -153,8 +153,8 @@ public void ARunWhereEveryCopyWorksSaysSoInOneLine() Collection lines = report.Summarize(); Assert.IsTrue(report.SourceExists); - Assert.AreEqual(1, lines.Count); - StringAssert.Contains(lines[0], "2 of 2", StringComparison.Ordinal); + Assert.HasCount(1, lines); + Assert.Contains("2 of 2", lines[0], StringComparison.Ordinal); Assert.IsFalse(lines[0].Contains("failed", StringComparison.Ordinal)); } finally @@ -215,9 +215,9 @@ public void ADestinationWithNoContainingDirectoryIsReportedRatherThanSkipped() FilePropagationReport report = FilePropagation.Propagate(source, destinations); - Assert.AreEqual(1, report.Results.Count, "The repository should still be accounted for."); + Assert.HasCount(1, report.Results, "The repository should still be accounted for."); Assert.IsFalse(report.Results[0].Succeeded); - StringAssert.Contains(report.Results[0].Failure!, "containing directory", StringComparison.Ordinal); + Assert.Contains("containing directory", report.Results[0].Failure!, StringComparison.Ordinal); } finally { @@ -274,7 +274,7 @@ public void OnlyTheCheckedRepositoriesGetADestination() Dictionary destinations = FilePropagation.ResolveDestinations(selection, repos, Path.Join("src", ".editorconfig")); - Assert.AreEqual(2, destinations.Count, "An unchecked repository should not get a destination."); + Assert.HasCount(2, destinations, "An unchecked repository should not get a destination."); Assert.IsFalse(destinations.ContainsKey(Repo("ktsu-dev/second"))); Assert.AreEqual(Path.Join("dev", "first", "src", ".editorconfig"), destinations[Repo("ktsu-dev/first")]); Assert.AreEqual(Path.Join("dev", "third", "src", ".editorconfig"), destinations[Repo("ktsu-dev/third")]); @@ -294,10 +294,10 @@ public void OnlyTheSummaryLineCarriesTheTimestamp() Collection lines = FilePropagation.DescribeForLog(report, at); - Assert.AreEqual(2, lines.Count); - StringAssert.StartsWith(lines[0], $"[{at}] ", StringComparison.Ordinal); - StringAssert.Contains(lines[0], "1 of 2", StringComparison.Ordinal); - StringAssert.StartsWith(lines[1], " ", StringComparison.Ordinal); + Assert.HasCount(2, lines); + Assert.StartsWith($"[{at}] ", lines[0], StringComparison.Ordinal); + Assert.Contains("1 of 2", lines[0], StringComparison.Ordinal); + Assert.StartsWith(" ", lines[1], StringComparison.Ordinal); Assert.IsFalse(lines[1].Contains($"[{at}]", StringComparison.Ordinal), "Detail lines are indented under the summary, not stamped again."); } @@ -319,10 +319,10 @@ public void AMissingSourceIsReportedOnceAndLeavesEveryRepositoryAlone() Collection lines = report.Summarize(); Assert.IsFalse(report.SourceExists); - Assert.AreEqual(0, report.Results.Count, "No repository should be touched when there is nothing to copy."); + Assert.IsEmpty(report.Results, "No repository should be touched when there is nothing to copy."); Assert.IsFalse(Directory.Exists(Path.GetDirectoryName(destination)!), "A failed run should not create destination directories."); - Assert.AreEqual(1, lines.Count, "A missing source is one failure, not one per repository."); - StringAssert.Contains(lines[0], "does not exist", StringComparison.Ordinal); + Assert.HasCount(1, lines, "A missing source is one failure, not one per repository."); + Assert.Contains("does not exist", lines[0], StringComparison.Ordinal); } finally {