Add unit tests, dedupe FFmpeg output filtering, harden settings records

Implements the suggestions from the last review pass: - Add AmiReel.Tests (MSTest), covering the pure logic in Models/ and Services/: SupportedVideoFormats, AppSettings normalization, UserFacingErrors.Summarize, the new FfmpegProgressParser/ FfmpegOutputFilter, RenderPipeline.Validate, BuildProgressMessage, and MoveSourceVideos. 59 tests, all passing. UI code-behind and anything that spawns an actual FFmpeg process are left to manual/integration testing. Exclude AmiReel.Tests\**\*.cs from AmiReel.csproj's default item glob — it's a subfolder of the app project now, so without the exclude the app itself was compiling the MSTest-only test files. - Extract the FFmpeg version/library-banner boilerplate list that ProcessRunner (live log filter) and UserFacingErrors (error summarizer) had each duplicated into a shared FfmpegOutputFilter.IsBoilerplateLine; each caller keeps its own remaining context-specific checks on top. Also extract ProcessRunner's line-parsing regexes into a standalone FfmpegProgressParser so it's directly unit-testable without spawning a process. - Convert AppSettings and RenderSettings from positional record constructors to named `required` init properties. Both records had runs of same-typed consecutive parameters (three string timing fields in AppSettings; three doubles then five ints in RenderSettings) that a positional constructor would let get silently transposed at a call site without the compiler catching it. Update the two call sites (MainPage.xaml.cs) to object-initializer syntax. - Add Properties/AssemblyInfo.cs with InternalsVisibleTo("AmiReel.Tests") and make Validate/BuildProgressMessage/IsNoise internal so tests can reach them directly instead of only through process-spawning entry points. - README: document `dotnet test`, and note that Package.appxmanifest's Identity is a local-dev placeholder that needs a real publisher/cert before MSIX distribution. Verified: dotnet build (solution + test project) is 0 warnings/errors, dotnet test is 59/59 passing, and the app still launches unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-13 14:27:57 +02:00
parent 3de3357d23
commit c412469773
19 changed files with 885 additions and 174 deletions
@@ -1,46 +1,58 @@
 namespace AmiReel.Models;

-public sealed record AppSettings(
-    string OutputFolder,
-    string EndCardPath,
-    string FfmpegPath,
-    string FfprobePath,
-    string PreviewPlayerPath,
-    string Theme,
-    string Encoder,
-    string TrimStart,
-    string FadeSeconds,
-    string EndCardHoldSeconds,
-    string ThumbnailInterval,
-    bool? MoveSourcesToOriginals)
+/// <summary>
+/// Persisted user settings (%LOCALAPPDATA%\AmiReel\settings.json). Uses named init
+/// properties rather than a positional constructor: several members share the same
+/// <see langword="string"/> type (e.g. TrimStart/FadeSeconds/EndCardHoldSeconds), so a
+/// positional record would let two arguments be silently transposed at a call site
+/// without the compiler catching it.
+/// </summary>
+public sealed record AppSettings
 {
+    public required string OutputFolder { get; init; }
+    public required string EndCardPath { get; init; }
+    public required string FfmpegPath { get; init; }
+    public required string FfprobePath { get; init; }
+    public required string PreviewPlayerPath { get; init; }
+    public required string Theme { get; init; }
+    public required string Encoder { get; init; }
+    public required string TrimStart { get; init; }
+    public required string FadeSeconds { get; init; }
+    public required string EndCardHoldSeconds { get; init; }
+    public required string ThumbnailInterval { get; init; }
+    public bool? MoveSourcesToOriginals { get; init; }
+
    public bool ShouldMoveSourcesToOriginals => MoveSourcesToOriginals != false;

-    public static AppSettings Default(string outputFolder) => new(
-        outputFolder,
-        "",
-        "",
-        "",
-        "",
-        "Dark",
-        "Auto",
-        "4.414",
-        "3",
-        "4",
-        "10",
-        true);
+    public static AppSettings Default(string outputFolder) => new()
+    {
+        OutputFolder = outputFolder,
+        EndCardPath = "",
+        FfmpegPath = "",
+        FfprobePath = "",
+        PreviewPlayerPath = "",
+        Theme = "Dark",
+        Encoder = "Auto",
+        TrimStart = "4.414",
+        FadeSeconds = "3",
+        EndCardHoldSeconds = "4",
+        ThumbnailInterval = "10",
+        MoveSourcesToOriginals = true,
+    };

-    public AppSettings Normalize(string fallbackOutputFolder) => new(
-        string.IsNullOrWhiteSpace(OutputFolder) ? fallbackOutputFolder : OutputFolder,
-        EndCardPath ?? "",
-        FfmpegPath ?? "",
-        FfprobePath ?? "",
-        PreviewPlayerPath ?? "",
-        string.IsNullOrWhiteSpace(Theme) ? "Dark" : Theme,
-        string.IsNullOrWhiteSpace(Encoder) ? "Auto" : Encoder,
-        string.IsNullOrWhiteSpace(TrimStart) ? "4.414" : TrimStart,
-        string.IsNullOrWhiteSpace(FadeSeconds) ? "3" : FadeSeconds,
-        string.IsNullOrWhiteSpace(EndCardHoldSeconds) ? "4" : EndCardHoldSeconds,
-        string.IsNullOrWhiteSpace(ThumbnailInterval) ? "10" : ThumbnailInterval,
-        MoveSourcesToOriginals ?? true);
+    public AppSettings Normalize(string fallbackOutputFolder) => this with
+    {
+        OutputFolder = string.IsNullOrWhiteSpace(OutputFolder) ? fallbackOutputFolder : OutputFolder,
+        EndCardPath = EndCardPath ?? "",
+        FfmpegPath = FfmpegPath ?? "",
+        FfprobePath = FfprobePath ?? "",
+        PreviewPlayerPath = PreviewPlayerPath ?? "",
+        Theme = string.IsNullOrWhiteSpace(Theme) ? "Dark" : Theme,
+        Encoder = string.IsNullOrWhiteSpace(Encoder) ? "Auto" : Encoder,
+        TrimStart = string.IsNullOrWhiteSpace(TrimStart) ? "4.414" : TrimStart,
+        FadeSeconds = string.IsNullOrWhiteSpace(FadeSeconds) ? "3" : FadeSeconds,
+        EndCardHoldSeconds = string.IsNullOrWhiteSpace(EndCardHoldSeconds) ? "4" : EndCardHoldSeconds,
+        ThumbnailInterval = string.IsNullOrWhiteSpace(ThumbnailInterval) ? "10" : ThumbnailInterval,
+        MoveSourcesToOriginals = MoveSourcesToOriginals ?? true,
+    };
 }
@@ -7,23 +7,33 @@ public enum EncoderMode
    CpuX264
 }

-public sealed record RenderSettings(
-    IReadOnlyList<string> InputFiles,
-    string EndCardPath,
-    string OutputDirectory,
-    string OutputName,
-    string FfmpegPath,
-    string FfprobePath,
-    EncoderMode Encoder,
-    double TrimStart,
-    double FadeSeconds,
-    double EndCardHoldSeconds,
-    int ThumbnailInterval,
-    bool MoveSourcesToOriginals = true,
-    int Width = 3840,
-    int Height = 2160,
-    int FramesPerSecond = 50,
-    int ThumbnailWidth = 1280,
-    int ThumbnailHeight = 720);
+/// <summary>
+/// One render job's parameters. Uses named init properties rather than a positional
+/// constructor: TrimStart/FadeSeconds/EndCardHoldSeconds are three consecutive
+/// <see langword="double"/> members (and Width/Height/FramesPerSecond/ThumbnailWidth/
+/// ThumbnailHeight five consecutive <see langword="int"/> members), so a positional
+/// record would let arguments be silently transposed at a call site without the
+/// compiler catching it.
+/// </summary>
+public sealed record RenderSettings
+{
+    public required IReadOnlyList<string> InputFiles { get; init; }
+    public required string EndCardPath { get; init; }
+    public required string OutputDirectory { get; init; }
+    public required string OutputName { get; init; }
+    public required string FfmpegPath { get; init; }
+    public required string FfprobePath { get; init; }
+    public required EncoderMode Encoder { get; init; }
+    public required double TrimStart { get; init; }
+    public required double FadeSeconds { get; init; }
+    public required double EndCardHoldSeconds { get; init; }
+    public required int ThumbnailInterval { get; init; }
+    public bool MoveSourcesToOriginals { get; init; } = true;
+    public int Width { get; init; } = 3840;
+    public int Height { get; init; } = 2160;
+    public int FramesPerSecond { get; init; } = 50;
+    public int ThumbnailWidth { get; init; } = 1280;
+    public int ThumbnailHeight { get; init; } = 720;
+}

 public sealed record RenderProgress(double Percent, string Stage, string Message);