Strengthen the test suite: 298 -> 395 tests, and fix three bugs it found

Reviewed the whole suite for coverage gaps, redundancy, brittleness, and
maintainability. The suite was already in good shape -- fast, deterministic,
no external dependencies -- so this keeps those properties (still ~3s) and
targets the concentrated weaknesses.

Coverage gaps closed:
- ClipExporter.ExportAsync had zero tests. Added an optional `runFfmpeg`
  delegate so its failure, cancellation, and script-cleanup paths run
  without spawning ffmpeg.
- PackageManager had no test file at all; ExtractFFmpegBin now has one.
- Nine of thirteen converters were untested, including every null and
  wrong-type branch.
- VideoPlayerController's transport commands (Play/Toggle/Next/Previous/
  GoToClip) were never called by any test.
- MainWindowViewModel's scan empty/error states and the keyboard transport
  shortcuts were unreachable; deleting the clip that is actually playing
  deadlocked on Dispatcher.Invoke, so RunOnUiThread got a `uiInvoker` seam.
- Mp4DurationReader's box-parsing branches (truncated moov, size 0 and 1
  forms, undersized box, moov without mvhd).
- New CultureInvarianceTests pins the four load-bearing InvariantCulture
  arguments. Dropping one silently returns zero clips under ar-SA or
  mis-dates every clip by 543 years under th-TH, and no existing test
  could see it.

Fixture and reliability work:
- TestClipFiles wrote 60s chunks spaced 60s apart, which made probed
  duration and nominal spacing indistinguishable in every timeline
  calculation. It now takes per-chunk durations, so chunk-offset
  arithmetic is actually observable.
- FakeClipMediaSourceBuilder exposed raw Lists that tests read while
  Build() appended off-thread; the bookkeeping is now private behind its
  lock and handed out as snapshots.
- FfconcatMediaSourceBuilderTests no longer leaks a playlist file per test
  into %TEMP%, and CamStorageTests no longer recursively scans the build
  output directory.

Removed or merged eight tests that duplicated a sibling's code path or
asserted framework behavior, and fixed several that passed for the wrong
reason -- notably two ClipPlaylist tests whose central assertions held
before the act ran.

Three production fixes, each pinned by a test verified to fail without it:
- FfconcatMediaSourceBuilder registered a header-only playlist for a camera
  whose first chunk was unreadable, so a corrupt side camera surfaced
  "Failed to open" instead of the actionable "no footage" message.
- ExtractFFmpegBin had no zip-slip containment check, and it extracts next
  to the app's own binaries.
- NowPlayingConverter treated DependencyProperty.UnsetValue as a real
  value, so two unresolved bindings compared equal and could paint the
  now-playing badge on a row that is not playing.

CI hardening: build and test in Release (three #if DEBUG blocks mean Debug
green said nothing about shipped binaries), a --blame-hang inactivity guard
so a deadlock regression fails in minutes instead of burning the runner, a
trx artifact uploaded on failure, and a `dotnet format` gate -- which caught
formatting drift in this very PR.

Author: danielchalmers

.github/actions/full-build/action.yml

@@ -17,18 +17,37 @@ runs:
       with:
         dotnet-version: 10.0.x
 
-    - name: Install WiX v6
+    # Release, not the default Debug: shipped code has #if DEBUG blocks (log level, the update
+    # check, drive scanning), so a Debug-only run says nothing about the binaries users get.
+    - name: Build
       shell: bash
-      run: |
-        dotnet tool install --global wix --version 6.0.*
+      run: dotnet build -c Release
 
-    - name: Build
+    # AGENTS.md requires `dotnet format`, and without a gate the rule gets missed. Runs after
+    # Build so a compile error is reported first — format's analyzers give a much worse message
+    # on a tree that doesn't compile.
+    - name: Verify formatting
       shell: bash
-      run: dotnet build
+      run: dotnet format --verify-no-changes --no-restore
 
+    # --blame-hang is an inactivity timeout on the test host: a deadlocked dispatcher call is not
+    # covered by the suite's polling deadlines, and without this a hang regression burns the
+    # runner's entire budget instead of failing in minutes with the offending test named.
+    # The whole suite finishes in seconds, so 5m cannot fire on a healthy run.
     - name: Test
       shell: bash
-      run: dotnet test
+      run: dotnet test -c Release --no-build --blame-hang --blame-hang-timeout 5m --logger "trx;LogFileName=test-results.trx" --results-directory TestResults
+
+    # Explicitly named so it doesn't collide with the unnamed publish artifact below, and kept as
+    # its own step so that one can keep if-no-files-found:error without a failed test run — which
+    # never reaches the publish step — tripping it on an empty directory.
+    - name: Upload test results
+      if: always()
+      uses: actions/upload-artifact@v7
+      with:
+        name: test-results
+        path: TestResults/**
+        if-no-files-found: ignore
 
     - name: Create Binaries
       shell: bash
@@ -45,6 +64,13 @@ runs:
           Compress-Archive -Path "publish/$arch/*" -DestinationPath "publish/SentryDeck-${{ inputs.version }}-$arch.zip"
         }
 
+    # Installed here rather than up front so build, format, and test failures short-circuit ahead
+    # of the tool install. setup-dotnet keeps ~/.dotnet/tools on PATH for the whole job.
+    - name: Install WiX v6
+      shell: bash
+      run: |
+        dotnet tool install --global wix --version 6.0.*
+
     - name: Create MSI Installers
       shell: bash
       run: |

.github/workflows/build.yml

@@ -5,6 +5,13 @@ on:
     branches: [ main ]
   pull_request:
     branches: [ main ]
+  workflow_dispatch:
+
+# Superseded runs are cancelled, including on main. That's safe here specifically because this
+# workflow's artifacts are never consumed — releases are built by the tag-triggered deploy.yml.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
 
 permissions:
   contents: read
@@ -14,5 +21,5 @@ jobs:
     runs-on: windows-2025
     steps:
       - uses: actions/checkout@v5
-      
+
       - uses: ./.github/actions/full-build

AGENTS.md

@@ -34,5 +34,6 @@ These instructions apply to Codex and other coding agents working in this repo.
 ## Testing/Validation
 - Always run `dotnet format`.
 - Tests live in `SentryDeck.Tests` and use xUnit/Shouldly.
+- Name tests `Subject_Scenario_Expectation` on a `public sealed class` with instance methods.
 - Add tests for new view-model and domain logic — view-models are plain objects that can be constructed directly in tests (see `MainWindowViewModelTests`).
 - If you change UI behavior, mention how to verify it (e.g., which view to open, what to click).

SentryDeck.Data/Playback/ClipExporter.cs

@@ -32,7 +32,13 @@ public interface IClipExporter
 /// are fast and lossless. Stream copy cuts at keyframes, so the actual bounds can land up to a
 /// GOP (~1s in Tesla footage) before the requested ones.
 /// </summary>
-public sealed class ClipExporter(Func<string> ffmpegDirectoryResolver) : IClipExporter
+/// <param name="runFfmpeg">
+/// Runs FFmpeg with an executable path and an argument string. Defaults to launching the real
+/// process; overridable for tests, which must not spawn ffmpeg.
+/// </param>
+public sealed class ClipExporter(
+    Func<string> ffmpegDirectoryResolver,
+    Func<string, string, CancellationToken, Task> runFfmpeg = null) : IClipExporter
 {
     private static readonly string ExportScriptDirectory =
         Path.Combine(Path.GetTempPath(), "SentryDeck", "exports");
@@ -41,6 +47,8 @@ public async Task ExportAsync(ClipExportRequest request, CancellationToken cance
     {
         ArgumentNullException.ThrowIfNull(request);
 
+        runFfmpeg ??= RunFfmpegAsync;
+
         var ffmpegDirectory = ffmpegDirectoryResolver()
             ?? throw new InvalidOperationException("FFmpeg is not installed. Restart the app to download it.");
         var ffmpegPath = Path.Combine(ffmpegDirectory, "ffmpeg.exe");
@@ -58,7 +66,7 @@ public async Task ExportAsync(ClipExportRequest request, CancellationToken cance
 
         try
         {
-            await RunFfmpegAsync(ffmpegPath, BuildArguments(scriptPath, request.OutputPath), cancellationToken);
+            await runFfmpeg(ffmpegPath, BuildArguments(scriptPath, request.OutputPath), cancellationToken);
             Log.Information(
                 "Exported clip range. Clip={ClipName}; Camera={Camera}; Start={Start}; End={End}; Output={Output}; ElapsedMs={ElapsedMs}",
                 request.Clip.Name,

SentryDeck.Data/Playback/FfconcatMediaSourceBuilder.cs

@@ -103,6 +103,15 @@ public ClipMediaSource Build(CamClip clip, IReadOnlySet<int> excludedChunkIndice
                 entries.Add((file.FullPath, chunkDurations[i]));
             }
 
+            // A camera whose first included file is unreadable ends up with nothing to play, so it is
+            // omitted entirely just like one missing from that chunk (the guard above). Registering a
+            // header-only playlist would advertise a camera that cannot be opened, hiding the
+            // actionable "no footage" message behind an FFmpeg failure at play time.
+            if (entries.Count == 0)
+            {
+                continue;
+            }
+
             var playlistPath = Path.Combine(PlaylistDirectory, $"{clipToken}-{camera}.ffconcat");
             WritePlaylist(playlistPath, entries);
             playlistPaths[camera] = playlistPath;

SentryDeck.Tests/CamDiscoveryResilienceTests.cs

@@ -6,7 +6,7 @@ namespace SentryDeck.Tests;
 /// Discovery must tolerate a single malformed/unreadable entry without discarding the whole
 /// library (regression guard for the "one bad filename empties the timeline" bug).
 /// </summary>
-public static class CamDiscoveryResilienceTests
+public sealed class CamDiscoveryResilienceTests
 {
     private static string CreateTempDir()
     {
@@ -22,7 +22,7 @@ private static void Touch(string dir, string name)
         => File.WriteAllBytes(Path.Combine(dir, name), []);
 
     [Fact]
-    public static void FindFiles_SkipsCalendarInvalidFileName()
+    public void FindFiles_SkipsCalendarInvalidFileName()
     {
         var dir = CreateTempDir();
         try
@@ -42,7 +42,7 @@ public static void FindFiles_SkipsCalendarInvalidFileName()
     }
 
     [Fact]
-    public static void FindFiles_CanonicalizesLegacyRearViewSuffixToBack()
+    public void FindFiles_CanonicalizesLegacyRearViewSuffixToBack()
     {
         var dir = CreateTempDir();
         try
@@ -61,7 +61,7 @@ public static void FindFiles_CanonicalizesLegacyRearViewSuffixToBack()
     }
 
     [Fact]
-    public static void FindClips_OneCalendarInvalidFileDoesNotDiscardOtherClips()
+    public void FindClips_OneCalendarInvalidFileDoesNotDiscardOtherClips()
     {
         var root = CreateTempDir();
         try
@@ -87,7 +87,7 @@ public static void FindClips_OneCalendarInvalidFileDoesNotDiscardOtherClips()
     }
 
     [Fact]
-    public static void Map_DateLessFolderWithoutEvent_FallsBackToFirstChunkTimestamp()
+    public void Map_DateLessFolderWithoutEvent_FallsBackToFirstChunkTimestamp()
     {
         // A folder like Tesla's RecentClips: loose files directly inside, no date-named subfolder and
         // no event.json. The clip timestamp must come from the file names, not DateTime.MinValue.
@@ -110,7 +110,7 @@ public static void Map_DateLessFolderWithoutEvent_FallsBackToFirstChunkTimestamp
     }
 
     [Fact]
-    public static void Map_CalendarInvalidFolderName_DoesNotThrowAndKeepsChunks()
+    public void Map_CalendarInvalidFolderName_DoesNotThrowAndKeepsChunks()
     {
         var root = CreateTempDir();
         try
@@ -128,4 +128,34 @@ public static void Map_CalendarInvalidFolderName_DoesNotThrowAndKeepsChunks()
             Directory.Delete(root, true);
         }
     }
+
+    [Fact]
+    public void Map_BackAndRearViewAtOneTimestamp_KeepsOneChunkAndDoesNotDropTheClip()
+    {
+        // What a drive spanning a firmware transition (or two drives merged by hand) actually holds:
+        // both rear-camera suffixes at the same timestamp. CamFile canonicalizes rear_view to back, so
+        // the two files collide on one camera key -- and an unguarded ToDictionary would throw there,
+        // with CamClip.TryMap swallowing it and the whole clip folder vanishing from the library.
+        var dir = CreateTempDir();
+        try
+        {
+            Touch(dir, "2023-02-23_14-14-48-front.mp4");
+            Touch(dir, "2023-02-23_14-14-48-back.mp4");
+            Touch(dir, "2023-02-23_14-14-48-rear_view.mp4");
+
+            var chunks = CamChunk.Map(dir);
+
+            chunks.Count.ShouldBe(1);
+
+            // Exactly one of the two rear files survives; which one follows enumeration order, so the
+            // winner is deliberately not pinned here.
+            chunks[0].Files.Keys.ShouldBe([CameraNames.Front, CameraNames.Back], ignoreOrder: true);
+
+            CamClip.Map(dir).ShouldNotBeNull();
+        }
+        finally
+        {
+            Directory.Delete(dir, true);
+        }
+    }
 }

SentryDeck.Tests/CamEventTests.cs

@@ -1,9 +1,9 @@
 namespace SentryDeck.Tests;
 
-public static class CamEventTests
+public sealed class CamEventTests
 {
     [Fact]
-    public static void Deserializes_Correctly()
+    public void Deserialize_FullEventJson_PopulatesEveryField()
     {
         // Arrange
         var json = """
@@ -31,7 +31,7 @@ public static void Deserializes_Correctly()
     }
 
     [Fact]
-    public static void Deserialization_OptionalProperties()
+    public void Deserialization_OptionalProperties()
     {
         // Arrange
         var json = """
@@ -53,7 +53,7 @@ public static void Deserialization_OptionalProperties()
     }
 
     [Fact]
-    public static void Deserialization_RecoversValidFieldsFromMalformedJson()
+    public void Deserialization_RecoversValidFieldsFromMalformedJson()
     {
         // Every field here is well-formed JSON but several are semantically bad (bad date, non-numeric
         // lat/lon, non-integer camera). Strict deserialization throws; the lenient fallback keeps the
@@ -83,7 +83,7 @@ public static void Deserialization_RecoversValidFieldsFromMalformedJson()
     }
 
     [Fact]
-    public static void Deserialization_BlankCoordinateKeepsCityAndTimestamp()
+    public void Deserialization_BlankCoordinateKeepsCityAndTimestamp()
     {
         // Tesla occasionally writes an incomplete est_lat; a single blank field must not discard the
         // city and the event timestamp the clip name falls back to.
@@ -110,17 +110,23 @@ public static void Deserialization_BlankCoordinateKeepsCityAndTimestamp()
     }
 
     [Fact]
-    public static void Deserialization_ReturnsNullForNonObjectJson()
+    public void Deserialization_ReturnsNullForNonObjectJson()
     {
         CamEvent.Deserialize("\"just a string\"").ShouldBeNull();
         CamEvent.Deserialize("not json at all {").ShouldBeNull();
     }
 
     [Fact]
-    public static void FromFile()
+    public void FromFile_ReadsEventJsonFromDisk()
     {
         var camEvent = CamEvent.FromFile("Mocks/2023-02-23_14-16-15/event.json");
 
+        // Assert the parsed values, not just non-null: every field could silently fall back to its
+        // default and still leave a CamEvent behind -- exactly what the sibling Deserialize tests
+        // exist to catch, and this is the only one that goes through the file-reading path.
         camEvent.ShouldNotBeNull();
+        camEvent.Timestamp.ShouldBe(new DateTime(2023, 2, 23, 14, 16, 7));
+        camEvent.City.ShouldBe("Austin");
+        camEvent.Reason.ShouldBe("user_interaction_honk");
     }
 }

SentryDeck.Tests/CamStorageTests.cs

@@ -1,19 +1,29 @@
 namespace SentryDeck.Tests;
 
-public static class CamStorageTests
+public sealed class CamStorageTests
 {
     [Fact]
-    public static void TraverseFindsAllClips()
+    public void Map_RootWithMixedFolders_ReturnsOnlyPlayableClips()
     {
-        var storage = CamStorage.Map(".");
+        var storage = CamStorage.Map("Mocks");
 
-        storage.Clips.Count.ShouldBe(3); // Ignores the "No Camera Files" folder.
+        // Two mock folders are deliberately unplayable and must not surface: "No Camera Files" holds
+        // only an event.json, and "No Front Angle" has every angle except the front one -- CamChunk.Map
+        // keeps only timestamp groups containing a front file, so it yields no chunks at all. The
+        // "Mocks" root is itself a clip candidate, but it holds no media either.
+        storage.Clips.Select(clip => clip.Name).ShouldBe(
+            [
+                "02/23/2023 14:16:15",
+                "Custom Folder Name",
+                "Missing Left Camera Angle on Second Chunk",
+            ],
+            ignoreOrder: true);
     }
 
     [Theory]
     [InlineData("Mocks/2023-02-23_14-16-15", "02/23/2023 14:16:15")]
     [InlineData("Mocks/Custom Folder Name", "Custom Folder Name")]
-    public static void ClipName(string path, string expectedName)
+    public void Map_ClipName_ComesFromFolderNameOrTimestamp(string path, string expectedName)
     {
         var clip = CamClip.Map(path);
 
@@ -22,7 +32,7 @@ public static void ClipName(string path, string expectedName)
     }
 
     [Fact]
-    public static void MapClipWithNonstandardNameFallsBackToEventDataForTimestamp()
+    public void MapClipWithNonstandardNameFallsBackToEventDataForTimestamp()
     {
         var clip = CamClip.Map("Mocks/Custom Folder Name");
 
@@ -34,30 +44,26 @@ public static void MapClipWithNonstandardNameFallsBackToEventDataForTimestamp()
     [InlineData("Mocks/2023-02-23_14-16-15", 2)]
     [InlineData("Mocks/Missing Left Camera Angle on Second Chunk", 2)]
     [InlineData("Mocks/No Front Angle", 0)]
-    public static void FindsAllChunks(string path, int expectedCount)
+    public void FindsAllChunks(string path, int expectedCount)
     {
         var chunks = CamChunk.Map(path);
 
         chunks.Count.ShouldBe(expectedCount);
     }
 
-    [Theory]
-    [InlineData("Mocks/2023-02-23_14-16-15")]
-    public static void ChunksAreInCorrectOrder(string path)
+    [Fact]
+    public void ChunksAreInCorrectOrder()
     {
-        var chunks = CamChunk.Map(path);
-
-        for (var i = 1; i < chunks.Count; i++)
-        {
-            var currentTimestamp = chunks[i - 1].Timestamp;
-            var nextTimestamp = chunks[i].Timestamp;
+        var chunks = CamChunk.Map("Mocks/2023-02-23_14-16-15");
 
-            nextTimestamp.ShouldBeGreaterThan(currentTimestamp, "each timestamp should be more recent than the previous one");
-        }
+        // The count assertion is load-bearing: an ordering check alone passes vacuously on an empty
+        // or single-chunk result, so it would stay green if discovery stopped finding chunks at all.
+        chunks.Count.ShouldBe(2);
+        chunks.Select(chunk => chunk.Timestamp).ShouldBeInOrder();
     }
 
     [Fact]
-    public static void MapRoot_WhenRootIsClipFolder_ReturnsThatClip()
+    public void MapRoot_WhenRootIsClipFolder_ReturnsThatClip()
     {
         var storage = CamStorage.Map("Mocks/2023-02-23_14-16-15");
 
@@ -68,7 +74,7 @@ public static void MapRoot_WhenRootIsClipFolder_ReturnsThatClip()
     [Theory]
     [InlineData("Mocks/2023-02-23_14-16-15", 8)]
     [InlineData("Mocks/Missing Left Camera Angle on Second Chunk", 7)]
-    public static void FindsAllFiles(string path, int expectedCount)
+    public void FindFiles_ReturnsEveryCameraFile(string path, int expectedCount)
     {
         var files = CamFile.FindFiles(path).ToList();