Strengthen test hygiene and fix vacuous assertions

src/SIL.Harmony.Tests/DataModelSimpleChanges.cs

@@ -177,7 +177,15 @@ public async Task CanTrackMultipleEntries()
     public async Task CanCreate2EntriesOutOfOrder()
     {
         var commit1 = await WriteNextChange(SetWord(_entity1Id, "entity1"));
-        await WriteChangeBefore(commit1, SetWord(_entity2Id, "entity2"));
+        var commit2 = await WriteChangeBefore(commit1, SetWord(_entity2Id, "entity2"));
+
+        commit2.DateTime.Should().BeBefore(commit1.DateTime);
+        (await DataModel.GetLatest<Word>(_entity1Id))!.Text.Should().Be("entity1");
+        (await DataModel.GetLatest<Word>(_entity2Id))!.Text.Should().Be("entity2");
+        //commit1 has the later timestamp, so it must be the model's most recent change even though
+        //commit2 was written after it — proving commits are ordered by timestamp, not insertion order
+        var snapshot = await DataModel.GetProjectSnapshot();
+        snapshot.LastChange.Should().Be(commit1.DateTime);
     }
 
     [Fact]

src/SIL.Harmony.Tests/ModelSnapshotTests.cs

@@ -6,9 +6,14 @@ namespace SIL.Harmony.Tests;
 public class ModelSnapshotTests : DataModelTestBase
 {
     [Fact]
-    public void CanGetEmptyModelSnapshot()
+    public async Task CanGetEmptyModelSnapshot()
     {
-        DataModel.GetProjectSnapshot().Should().NotBeNull();
+        var snapshot = await DataModel.GetProjectSnapshot();
+        snapshot.Should().NotBeNull();
+        snapshot.Snapshots.Should().BeEmpty();
+        snapshot.LastChange.Should().BeNull();
+        snapshot.LastCommitId.Should().BeNull();
+        snapshot.LastCommitHash.Should().BeNull();
     }
 
     [Fact]

src/SIL.Harmony.Tests/MultiThreadingTests.cs

@@ -1,66 +1,99 @@
 using Microsoft.Data.Sqlite;
+using SIL.Harmony.Sample.Models;
 
 namespace SIL.Harmony.Tests;
 
 public class MultiThreadingTests(ITestOutputHelper output)
 {
     private const string _connectionString = "Data Source=file:MultiThreadingTests.db?mode=memory&cache=shared";
-    private static async Task<Exception?> Run(ITestOutputHelper output,
+    private const int _changesPerThread = 100;
+
+    private static async Task<(Guid id, string lastValue, Exception? exception)> Run(ITestOutputHelper output,
         CancellationTokenSource cancellationTokenSource,
         bool debug)
     {
         return await Task.Run(() =>
         {
             Exception? exception = null;
+            var id = Guid.NewGuid();
+            var lastValue = "";
             var t = new Thread(() =>
             {
                 var random = new Random();
                 var fixture = new DataModelTestBase(new SqliteConnection(_connectionString));
-                fixture.InitializeAsync().GetAwaiter().GetResult();
-                var id = Guid.NewGuid();
-                for (var i = 0; i < 100; i++)
+                try
                 {
-                    var value = "test" + i;
-                    try
+                    fixture.InitializeAsync().GetAwaiter().GetResult();
+                    for (var i = 0; i < _changesPerThread; i++)
                     {
-                        Thread.Sleep(random.Next(1, 10));
+                        var value = "test" + i;
+                        try
+                        {
+                            Thread.Sleep(random.Next(1, 10));
 
-                        _ = fixture.WriteNextChange(fixture.SetWord(id, value)).Result;
+                            _ = fixture.WriteNextChange(fixture.SetWord(id, value)).Result;
+                            lastValue = value;
 
-                        if (debug) output.WriteLine($"id: {id}, value:{value}");
-                        if (cancellationTokenSource.IsCancellationRequested) return;
+                            if (debug) output.WriteLine($"id: {id}, value:{value}");
+                            if (cancellationTokenSource.IsCancellationRequested) return;
+                        }
+                        catch (Exception e)
+                        {
+                            output.WriteLine($"id: {id}, value:{value}, error: {e}");
+                            cancellationTokenSource.Cancel();
+                            exception = e;
+                            return;
+                        }
                     }
-                    catch (Exception e)
+                }
+                finally
+                {
+                    //dispose this thread's fixture (and its DI ServiceProvider); the test-level fixture
+                    //keeps the shared in-memory database alive until assertions complete.
+                    //guard the cleanup so a disposal failure can't surface as an unhandled exception on
+                    //this raw thread (which would crash the test host rather than fail the test).
+                    try
                     {
-                        output.WriteLine($"id: {id}, value:{value}, error: {e}");
-                        cancellationTokenSource.Cancel();
-                        exception = e;
-                        return;
+                        fixture.DisposeAsync().AsTask().GetAwaiter().GetResult();
+                    }
+                    catch (Exception disposeException)
+                    {
+                        output.WriteLine($"error disposing fixture: {disposeException}");
                     }
                 }
             });
             t.Start();
             t.Join();
-            return exception;
+            return (id, lastValue, exception);
         });
     }
 
     [Fact]
     public async Task CanApplyChangesWithoutError()
     {
-        //ensure the database is created before running the tests
-        _ = new DataModelTestBase(new SqliteConnection(_connectionString));
+        //ensure the database is created before running the tests, and keep this fixture alive (and its
+        //connection open) so the shared in-memory database survives while the worker fixtures dispose
+        await using var fixture = new DataModelTestBase(new SqliteConnection(_connectionString));
         bool debug = false;
         var cancellationTokenSource = new CancellationTokenSource();
         var results = await Task.WhenAll(
             Run(output, cancellationTokenSource, debug),
             Run(output, cancellationTokenSource, debug),
             Run(output, cancellationTokenSource, debug)
         );
-        foreach (var exception in results)
+        foreach (var (_, _, exception) in results)
         {
             exception.Should().BeNull();
         }
 
+        //every thread wrote to its own entity; assert the model converged to each thread's final write
+        //(a lost update or corruption that doesn't throw would otherwise pass unnoticed)
+        foreach (var (id, lastValue, _) in results)
+        {
+            lastValue.Should().Be("test" + (_changesPerThread - 1));
+            var word = await fixture.DataModel.GetLatest<Word>(id);
+            word.Should().NotBeNull();
+            word.Text.Should().Be(lastValue);
+        }
     }
 }

src/SIL.Harmony.Tests/RepositoryTests.cs

@@ -245,9 +245,26 @@ await _repository.AddSnapshots([
     }
 
     [Fact]
-    public async Task DeleteStaleSnapshots_Works()
+    public async Task DeleteStaleSnapshots_WithNoSnapshots_DoesNothing()
     {
+        //the empty-repository branch: nothing to delete, must not throw
         await _repository.DeleteStaleSnapshots(Commit(Guid.NewGuid(), Time(1, 0)));
+
+        _crdtDbContext.Snapshots.Should().BeEmpty();
+    }
+
+    [Fact]
+    public async Task DeleteStaleSnapshots_KeepsSnapshotsOlderThanTheCommit()
+    {
+        await _repository.AddSnapshots([
+            Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 0)),
+            Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(2, 0)),
+        ]);
+
+        //the new commit is newer than every existing snapshot, so none are stale
+        await _repository.DeleteStaleSnapshots(Commit(Guid.NewGuid(), Time(3, 0)));
+
+        _crdtDbContext.Snapshots.Should().HaveCount(2);
     }
 
     [Fact]

src/SIL.Harmony.Tests/SnapshotTests.cs

@@ -88,18 +88,27 @@ await AddCommitsViaSync([
             await WriteNextChange(SetWord(Guid.NewGuid(), "test 1"), add: false),
             await WriteNextChange(SetWord(entityId, "test 2"), add: false),
         ]);
+
+        (await DataModel.GetLatest<Word>(entityId))!.Text.Should().Be("test 2");
+        DbContext.Set<Word>().Should().HaveCount(2);
     }
 
     [Fact]
     public async Task CanRecreateUniqueConstraintConflictingValueInOneCommit()
     {
         var entityId = Guid.NewGuid();
+        var recreatedTagId = Guid.NewGuid();
         await WriteNextChange(SetTag(entityId, "tag-1"));
         await WriteNextChange(
             [
                 DeleteTag(entityId),
-                SetTag(Guid.NewGuid(), "tag-1"),
+                SetTag(recreatedTagId, "tag-1"),
             ]);
+
+        (await DataModel.GetLatest<Tag>(entityId))!.DeletedAt.Should().NotBeNull("the original tag was deleted");
+        var recreatedTag = await DataModel.GetLatest<Tag>(recreatedTagId);
+        recreatedTag!.Text.Should().Be("tag-1");
+        recreatedTag.DeletedAt.Should().BeNull();
     }
 
     [Fact]