From 647ed80b4bd23341a08e8cac28d361001e6292f6 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 14 Aug 2026 18:08:58 +1000 Subject: [PATCH] Require InlinePatch.TestName The viewer labels and groups queue entries by TestName and falls back to the call site without one, so a producer that never set it was indistinguishable from a test with no name. Verify was that producer, and it went unnoticed until a queue of bare file:line rows made someone ask why. Required rather than non-null. A patch that never reaches a queue -- a Remove, or an apply straight through InlineApplier -- has no reviewable identity, and now says so with an explicit null instead of by omission. InlinePatchFile.TryParse was the only production construction site and already used an object initializer, so nothing outside tests changed. Test files where no patch is ever queued state the null once in a local helper rather than at every call site; helpers that already threaded framework gained a testName beside it. --- src/DiffEngine.Tests/InlineApplierTests.cs | 70 ++++++++++++------- src/DiffEngine.Tests/InlineQueueTests.cs | 10 ++- src/DiffEngine.Tests/ViewerProtocolTests.cs | 35 +++++++--- src/DiffEngine/Inline/InlinePatch.cs | 14 +++- src/DiffEngineTray.Tests/DebugReportTests.cs | 5 +- .../OwnedInlineHostTest.cs | 10 ++- .../TrayViewerSyncTest.cs | 3 +- .../EngineInlineTests.cs | 31 +++++--- src/DiffEngineViewer.Tests/IpcTests.cs | 2 +- .../ViewerLaunchTests.cs | 35 ++++++---- 10 files changed, 149 insertions(+), 66 deletions(-) diff --git a/src/DiffEngine.Tests/InlineApplierTests.cs b/src/DiffEngine.Tests/InlineApplierTests.cs index e54c9060..4b536c9a 100644 --- a/src/DiffEngine.Tests/InlineApplierTests.cs +++ b/src/DiffEngine.Tests/InlineApplierTests.cs @@ -25,13 +25,26 @@ static byte[] Utf8(string text, bool bom) const string source = "class C\n{\n void M() => Verify(value).Snapshot(\"old\");\n}"; + // Nothing here queues a patch, so none of them has a reviewable identity. Stated once rather + // than at every call site below. + static InlinePatch Patch( + string sourceFile, + int lineHint, + string? originalExpression, + string newContent, + InlinePatchMode mode = InlinePatchMode.Set) => + new(sourceFile, lineHint, originalExpression, newContent, mode) + { + TestName = null + }; + [Test] public async Task Utf8BomPreserved() { var path = WriteTemp(Utf8(source, bom: true)); try { - var result = InlineApplier.Apply(new(path, 3, "\"old\"", "new")); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", "new")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); var bytes = await File.ReadAllBytesAsync(path); await Assert.That(bytes[0]).IsEqualTo((byte)0xEF); @@ -51,7 +64,7 @@ public async Task NoBomStaysNoBom() var path = WriteTemp(Utf8(source, bom: false)); try { - var result = InlineApplier.Apply(new(path, 3, "\"old\"", "new")); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", "new")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); var bytes = await File.ReadAllBytesAsync(path); await Assert.That(bytes[0]).IsEqualTo((byte)'c'); @@ -69,7 +82,7 @@ public async Task Utf16Preserved() var path = WriteTemp([.. encoding.GetPreamble(), .. encoding.GetBytes(source)]); try { - var result = InlineApplier.Apply(new(path, 3, "\"old\"", "new")); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", "new")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); var bytes = await File.ReadAllBytesAsync(path); await Assert.That(bytes[0]).IsEqualTo((byte)0xFF); @@ -96,7 +109,7 @@ .. Utf8("class C\n{\n // caf", bom: false), var path = WriteTemp(bytes); try { - var result = InlineApplier.Apply(new(path, 4, "\"old\"", "new")); + var result = InlineApplier.Apply(Patch(path, 4, "\"old\"", "new")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Failed); await Assert.That(result.Message!).Contains("Convert it to UTF-8"); @@ -115,7 +128,7 @@ public async Task NonAsciiUtf8IsPreserved() var path = WriteTemp(Utf8(text, bom: false)); try { - var result = InlineApplier.Apply(new(path, 4, "\"old\"", "naïve ☕")); + var result = InlineApplier.Apply(Patch(path, 4, "\"old\"", "naïve ☕")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); var after = await File.ReadAllTextAsync(path); @@ -134,7 +147,7 @@ public async Task CrlfPreserved() var path = WriteTemp(Utf8(source.Replace("\n", "\r\n"), bom: false)); try { - var result = InlineApplier.Apply(new(path, 3, "\"old\"", "a\nb")); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", "a\nb")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); var text = await File.ReadAllTextAsync(path); await Assert.That(text).DoesNotContain("a\nb"); @@ -163,7 +176,7 @@ public async Task EolAndBomCombinations(string fileEol, string contentEol, bool try { var content = "a" + contentEol + "b"; - var result = InlineApplier.Apply(new(path, 3, "\"old\"", content)); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", content)); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); var bytes = await File.ReadAllBytesAsync(path); @@ -202,7 +215,7 @@ public async Task LfFileIsNotConvertedToCrlf() var path = WriteTemp(Utf8(source, bom: false)); try { - var result = InlineApplier.Apply(new(path, 3, "\"old\"", "a\nb")); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", "a\nb")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied); await Assert.That(await File.ReadAllTextAsync(path)).DoesNotContain("\r"); } @@ -217,7 +230,7 @@ public async Task LfFileIsNotConvertedToCrlf() [Test] public async Task TryParseToleratesLeadingBom() { - var patch = new InlinePatch(@"C:\proj\Tests.cs", 7, "\"old\"", "new content"); + var patch = Patch(@"C:\proj\Tests.cs", 7, "\"old\"", "new content"); var payload = "" + InlinePatchFile.Build(patch); var read = InlinePatchFile.TryParse(payload, out var result); @@ -231,7 +244,7 @@ public async Task TryParseToleratesLeadingBom() [Test] public async Task MissingFileFails() { - var result = InlineApplier.Apply(new(Path.Combine(Path.GetTempPath(), "does-not-exist-inline.cs"), 1, null, "x")); + var result = InlineApplier.Apply(Patch(Path.Combine(Path.GetTempPath(), "does-not-exist-inline.cs"), 1, null, "x")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Failed); } @@ -242,7 +255,7 @@ public async Task AlreadyAppliedDoesNotWrite() try { var before = File.GetLastWriteTimeUtc(path); - var result = InlineApplier.Apply(new(path, 3, "\"old\"", "old")); + var result = InlineApplier.Apply(Patch(path, 3, "\"old\"", "old")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.AlreadyApplied); await Assert.That(File.GetLastWriteTimeUtc(path)).IsEqualTo(before); } @@ -259,8 +272,8 @@ public async Task ParallelAppliesToSameFile() var path = WriteTemp(Utf8(multi, bom: false)); try { - var taskA = Task.Run(() => InlineApplier.Apply(new(path, 3, "\"oldA\"", "newA"))); - var taskB = Task.Run(() => InlineApplier.Apply(new(path, 4, "\"oldB\"", "newB"))); + var taskA = Task.Run(() => InlineApplier.Apply(Patch(path, 3, "\"oldA\"", "newA"))); + var taskB = Task.Run(() => InlineApplier.Apply(Patch(path, 4, "\"oldB\"", "newB"))); var results = await Task.WhenAll(taskA, taskB); await Assert.That(results[0].Status).IsEqualTo(InlineApplyStatus.Applied); await Assert.That(results[1].Status).IsEqualTo(InlineApplyStatus.Applied); @@ -300,8 +313,8 @@ public async Task ParallelAppliesWithIdenticalLiterals() var path = WriteTemp(Utf8(multi, bom: false)); try { - var taskA = Task.Run(() => InlineApplier.Apply(new(path, 3, "\"old\"", "same"))); - var taskB = Task.Run(() => InlineApplier.Apply(new(path, 4, "\"old\"", "same"))); + var taskA = Task.Run(() => InlineApplier.Apply(Patch(path, 3, "\"old\"", "same"))); + var taskB = Task.Run(() => InlineApplier.Apply(Patch(path, 4, "\"old\"", "same"))); var results = await Task.WhenAll(taskA, taskB); await Assert.That(results[0].Status).IsEqualTo(InlineApplyStatus.Applied); @@ -324,8 +337,8 @@ public async Task SequentialAppliesWithIdenticalLiterals() var path = WriteTemp(Utf8(multi, bom: false)); try { - var first = InlineApplier.Apply(new(path, 3, "\"old\"", "newA")); - var second = InlineApplier.Apply(new(path, 4, "\"old\"", "newB")); + var first = InlineApplier.Apply(Patch(path, 3, "\"old\"", "newA")); + var second = InlineApplier.Apply(Patch(path, 4, "\"old\"", "newB")); await Assert.That(first.Status).IsEqualTo(InlineApplyStatus.Applied); await Assert.That(second.Status).IsEqualTo(InlineApplyStatus.Applied); @@ -351,7 +364,7 @@ public async Task NotFoundWhenSourceChanged() var path = WriteTemp(Utf8(source, bom: false)); try { - var result = InlineApplier.Apply(new(path, 3, "\"gone-expression\"", "new")); + var result = InlineApplier.Apply(Patch(path, 3, "\"gone-expression\"", "new")); await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.NotFound); await Assert.That(result.Message!).Contains("Re-run the test"); } @@ -367,7 +380,10 @@ public class InlinePatchFileTests [Test] public async Task RoundTrip() { - var patch = new InlinePatch(@"C:\proj\Tests.cs", 42, "\"\"\"\nold\n\"\"\"", "line1\nline2"); + var patch = new InlinePatch(@"C:\proj\Tests.cs", 42, "\"\"\"\nold\n\"\"\"", "line1\nline2") + { + TestName = null + }; var path = Path.Combine(Path.GetTempPath(), $"InlinePatchFileTests_{Guid.NewGuid():N}.inlinepatch"); try { @@ -388,7 +404,10 @@ public async Task RoundTrip() [Test] public async Task RoundTripNullExpression() { - var patch = new InlinePatch("Tests.cs", 1, null, "content"); + var patch = new InlinePatch("Tests.cs", 1, null, "content") + { + TestName = null + }; var path = Path.Combine(Path.GetTempPath(), $"InlinePatchFileTests_{Guid.NewGuid():N}.inlinepatch"); try { @@ -409,7 +428,10 @@ public async Task RoundTripNullExpression() [Arguments(InlinePatchMode.Remove)] public async Task RoundTripMode(InlinePatchMode mode) { - var patch = new InlinePatch("Tests.cs", 1, null, "content", mode); + var patch = new InlinePatch("Tests.cs", 1, null, "content", mode) + { + TestName = null + }; var read = InlinePatchFile.TryParse(InlinePatchFile.Build(patch), out var result); @@ -420,7 +442,7 @@ public async Task RoundTripMode(InlinePatchMode mode) [Test] public async Task DefaultModeIsSet() { - var read = InlinePatchFile.TryParse(InlinePatchFile.Build(new("Tests.cs", 1, null, "content")), out var result); + var read = InlinePatchFile.TryParse(InlinePatchFile.Build(new("Tests.cs", 1, null, "content") { TestName = null }), out var result); await Assert.That(read).IsTrue(); await Assert.That(result!.Mode).IsEqualTo(InlinePatchMode.Set); @@ -481,7 +503,7 @@ public async Task MetadataRoundTrips() [Test] public async Task NullMetadataRoundTripsAsNull() { - var read = InlinePatchFile.TryParse(InlinePatchFile.Build(new("Tests.cs", 1, null, "content")), out var result); + var read = InlinePatchFile.TryParse(InlinePatchFile.Build(new("Tests.cs", 1, null, "content") { TestName = null }), out var result); await Assert.That(read).IsTrue(); await Assert.That(result!.TestName).IsNull(); @@ -533,7 +555,7 @@ public async Task MetadataOrderIsFlexible() [Test] public async Task UnknownTrailingLinesAreIgnored() { - var payload = InlinePatchFile.Build(new("Tests.cs", 1, null, "content")) + "future: value\n"; + var payload = InlinePatchFile.Build(new("Tests.cs", 1, null, "content") { TestName = null }) + "future: value\n"; var read = InlinePatchFile.TryParse(payload, out var result); diff --git a/src/DiffEngine.Tests/InlineQueueTests.cs b/src/DiffEngine.Tests/InlineQueueTests.cs index 261cbe64..9b68a84c 100644 --- a/src/DiffEngine.Tests/InlineQueueTests.cs +++ b/src/DiffEngine.Tests/InlineQueueTests.cs @@ -5,10 +5,16 @@ /// public class InlineQueueTests { - static InlinePatch Patch(string source = "Sample.cs", int line = 42, string content = "new", string? framework = null) => + static InlinePatch Patch( + string source = "Sample.cs", + int line = 42, + string content = "new", + string? framework = null, + string? testName = null) => new(source, line, "\"old\"", content) { - Framework = framework + Framework = framework, + TestName = testName }; static InlineApplyResult Fails(InlinePatch patch) => diff --git a/src/DiffEngine.Tests/ViewerProtocolTests.cs b/src/DiffEngine.Tests/ViewerProtocolTests.cs index dfffcd05..be9513af 100644 --- a/src/DiffEngine.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngine.Tests/ViewerProtocolTests.cs @@ -10,10 +10,25 @@ /// public class ViewerProtocolTests { + // These pin the wire shape rather than what a reviewer reads, so nothing here is named. The + // one test that is about the name says so itself. + static InlinePatch Patch( + string source, + int line, + string? expression, + string content, + InlinePatchMode mode = InlinePatchMode.Set, + string? framework = null) => + new(source, line, expression, content, mode) + { + TestName = null, + Framework = framework + }; + [Test] public async Task InlineMessageRoundTrips() { - var patch = new InlinePatch("Tests.cs", 42, "\"old\"", "new content"); + var patch = Patch("Tests.cs", 42, "\"old\"", "new content"); var payload = new ViewerMessage(ViewerVerb.Inline, Body: InlinePatchFile.Build(patch)).Build(); @@ -33,7 +48,7 @@ public async Task InlineMessageRoundTrips() public async Task AwkwardSnapshotTextSurvivesTheRoundTrip() { var content = "line \"one\"\n\tbraces {} and | pipes\r\nversion: 1\nverb: quit\n"; - var patch = new InlinePatch("Tests.cs", 1, null, content); + var patch = Patch("Tests.cs", 1, null, content); var payload = new ViewerMessage(ViewerVerb.Inline, Body: InlinePatchFile.Build(patch)).Build(); @@ -53,7 +68,7 @@ public async Task EveryModeRoundTrips() foreach (var name in Enum.GetNames(typeof(InlinePatchMode))) { var mode = (InlinePatchMode) Enum.Parse(typeof(InlinePatchMode), name); - var patch = new InlinePatch("Tests.cs", 1, null, "content", mode); + var patch = Patch("Tests.cs", 1, null, "content", mode); var payload = new ViewerMessage(ViewerVerb.Inline, Body: InlinePatchFile.Build(patch)).Build(); await Assert.That(ViewerMessage.TryParse(payload, out var message)).IsTrue(); @@ -151,7 +166,7 @@ public async Task AListingItemCarriesKeyNameAndStatus() [Test] public async Task AFullListingRoundTripsThePatch() { - var patch = new InlinePatch("Tests.cs", 42, "\"old\"", "new content"); + var patch = Patch("Tests.cs", 42, "\"old\"", "new content"); var listing = ViewerResponse.Listing( [ new("tests.cs|42", "Tests.cs:42", "locked", InlinePatchFile.Build(patch)) @@ -174,7 +189,7 @@ public async Task AFullListingRoundTripsThePatch() [Test] public async Task AFullListingHasNoItemLines() { - var patch = InlinePatchFile.Build(new("Tests.cs", 1, null, "content")); + var patch = InlinePatchFile.Build(Patch("Tests.cs", 1, null, "content")); var text = ViewerResponse.Listing([new("key", "Tests.cs:1", null, patch)]).Build(); await Assert.That(Fields(text, "item: ")).IsEmpty(); @@ -214,7 +229,7 @@ public async Task SettleCarriesTheOriginInTheBody() [Test] public async Task AFullListingCarriesThePrimaryOrigins() { - var patch = InlinePatchFile.Build(new("Tests.cs", 42, "\"old\"", "new content")); + var patch = InlinePatchFile.Build(Patch("Tests.cs", 42, "\"old\"", "new content")); var listing = ViewerResponse.Listing( [ new("tests.cs|42", "Tests.cs:42", null, patch) @@ -236,8 +251,8 @@ public async Task AFullListingCarriesThePrimaryOrigins() [Test] public async Task AVariantLineRoundTrips() { - var primary = InlinePatchFile.Build(new("Tests.cs", 42, "\"old\"", "eight")); - var other = InlinePatchFile.Build(new("Tests.cs", 42, "\"old\"", "nine")); + var primary = InlinePatchFile.Build(Patch("Tests.cs", 42, "\"old\"", "eight")); + var other = InlinePatchFile.Build(Patch("Tests.cs", 42, "\"old\"", "nine")); var listing = ViewerResponse.Listing( [ new("tests.cs|42", "Tests.cs:42", null, primary) @@ -338,8 +353,8 @@ public async Task TrackedKeysCannotCollideWithInlineKeys() [Test] public async Task AConflictedEntryListsItsStatus() { - var eight = new InlinePatch("Tests.cs", 42, "\"old\"", "eight") { Framework = "net8.0" }; - var nine = new InlinePatch("Tests.cs", 42, "\"old\"", "nine") { Framework = "net9.0" }; + var eight = Patch("Tests.cs", 42, "\"old\"", "eight", framework: "net8.0"); + var nine = Patch("Tests.cs", 42, "\"old\"", "nine", framework: "net9.0"); var entry = new PendingInline([new(eight, ["net8.0"]), new(nine, ["net9.0"])]); var listed = ViewerListing.Items([entry], withPatches: false).Single(); diff --git a/src/DiffEngine/Inline/InlinePatch.cs b/src/DiffEngine/Inline/InlinePatch.cs index ea4d2894..368765bc 100644 --- a/src/DiffEngine/Inline/InlinePatch.cs +++ b/src/DiffEngine/Inline/InlinePatch.cs @@ -49,10 +49,18 @@ public InlinePatch( public InlinePatchMode Mode { get; set; } /// - /// Display name of the test that produced this patch. Optional; supplied by the caller - /// (Verify). Null when the caller did not provide one. + /// Display name of the test that produced this patch, supplied by the caller (Verify). The + /// viewer labels and groups queue entries by it, falling back to the call site without one. + /// + /// Required, though still nullable: a patch that never reaches a queue — an + /// , or an apply straight through + /// — has no reviewable identity and says so with an explicit null. + /// Omission and decision were previously indistinguishable, and a producer that simply never + /// set it went unnoticed for as long as it did because the viewer's fallback reads as an + /// unnamed test rather than as a missing field. + /// /// - public string? TestName { get; set; } + public required string? TestName { get; set; } /// /// Short target framework of the test process that produced this patch ("net9.0", "net48"). diff --git a/src/DiffEngineTray.Tests/DebugReportTests.cs b/src/DiffEngineTray.Tests/DebugReportTests.cs index c38b6dce..8e6c3e58 100644 --- a/src/DiffEngineTray.Tests/DebugReportTests.cs +++ b/src/DiffEngineTray.Tests/DebugReportTests.cs @@ -78,7 +78,10 @@ public async Task Owned() var source = Path.Combine(directory, "SampleTests.cs"); await File.WriteAllTextAsync(source, ""); - var patch = new InlinePatch(source, 42, "\"old\"", "line one\nline two"); + var patch = new InlinePatch(source, 42, "\"old\"", "line one\nline two") + { + TestName = null + }; // Over the socket, as the test process that failed the assertion sends it if (!ViewerClient.TrySend(new(ViewerVerb.Inline, Body: InlinePatchFile.Build(patch)), out _, host.Port)) { diff --git a/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs b/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs index 983eed14..94305cff 100644 --- a/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs +++ b/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs @@ -31,10 +31,16 @@ public ViewerResponse Send(ViewerMessage message) return response; } - public ViewerResponse Queue(string source = @"c:\repo\SampleTests.cs", int line = 42, string content = "new", string? framework = null) => + public ViewerResponse Queue( + string source = @"c:\repo\SampleTests.cs", + int line = 42, + string content = "new", + string? framework = null, + string? testName = null) => Send(new(ViewerVerb.Inline, Body: InlinePatchFile.Build(new(source, line, "\"old\"", content) { - Framework = framework + Framework = framework, + TestName = testName }))); public void Dispose() => diff --git a/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs b/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs index abf3d61a..d70b50fc 100644 --- a/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs +++ b/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs @@ -648,7 +648,8 @@ static string Payload(string source, int line, string content, string? framework InlinePatchFile.Build( new(source, line, "\"old\"", content) { - Framework = framework + Framework = framework, + TestName = null }); /// diff --git a/src/DiffEngineViewer.Tests/EngineInlineTests.cs b/src/DiffEngineViewer.Tests/EngineInlineTests.cs index b1619f83..b022fb9c 100644 --- a/src/DiffEngineViewer.Tests/EngineInlineTests.cs +++ b/src/DiffEngineViewer.Tests/EngineInlineTests.cs @@ -18,11 +18,24 @@ [NotInParallel] public class EngineInlineTests { + // These cover the path a patch takes, not what a reviewer reads at the end of it, so nothing + // here is named. + static EnginePatch Patch( + string source, + int line, + string? expression, + string content, + engine::DiffEngine.InlinePatchMode mode = engine::DiffEngine.InlinePatchMode.Set) => + new(source, line, expression, content, mode) + { + TestName = null + }; + [Test] public async Task QueuesIntoARunningViewer() { using var scope = new EngineScope(); - var patch = new EnginePatch("Sample.cs", 42, "\"old\"", "new content"); + var patch = Patch("Sample.cs", 42, "\"old\"", "new content"); var result = await EngineRunner.AddInlineAsync(patch); @@ -43,8 +56,8 @@ public async Task ARepeatOfTheSameCallSiteReplaces() { using var scope = new EngineScope(); - await EngineRunner.AddInlineAsync(new("Sample.cs", 42, "\"old\"", "first")); - await EngineRunner.AddInlineAsync(new("Sample.cs", 42, "\"old\"", "second")); + await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "first")); + await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "second")); var queue = scope.Fixture.Host.State.Queue; await Assert.That(queue).HasSingleItem(); @@ -55,8 +68,8 @@ public async Task ARepeatOfTheSameCallSiteReplaces() public async Task SettleDropsTheEntry() { using var scope = new EngineScope(); - await EngineRunner.AddInlineAsync(new("Sample.cs", 42, "\"old\"", "new")); - await EngineRunner.AddInlineAsync(new("Other.cs", 7, "\"old\"", "new")); + await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "new")); + await EngineRunner.AddInlineAsync(Patch("Other.cs", 7, "\"old\"", "new")); EngineRunner.SettleInline("Sample.cs", 42); @@ -69,7 +82,7 @@ public async Task SettleDropsTheEntry() public async Task SettleForAnUnknownCallSiteIsHarmless() { using var scope = new EngineScope(); - await EngineRunner.AddInlineAsync(new("Sample.cs", 42, "\"old\"", "new")); + await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "new")); EngineRunner.SettleInline("Nothing.cs", 1); @@ -85,7 +98,7 @@ public async Task DisabledDoesNotReachTheViewer() { using var scope = new EngineScope(disabled: true); - var result = await EngineRunner.AddInlineAsync(new("Sample.cs", 42, "\"old\"", "new")); + var result = await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "new")); await Assert.That(result).IsEqualTo(EngineResult.Disabled); await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty(); @@ -99,7 +112,7 @@ public async Task DisabledDoesNotReachTheViewer() public async Task ARemovePatchIsRefused() { using var scope = new EngineScope(); - var patch = new EnginePatch("Sample.cs", 42, "\"old\"", "", engine::DiffEngine.InlinePatchMode.Remove); + var patch = Patch("Sample.cs", 42, "\"old\"", "", engine::DiffEngine.InlinePatchMode.Remove); await Assert.That(() => EngineRunner.AddInlineAsync(patch)).Throws(); await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty(); @@ -110,7 +123,7 @@ public async Task TheOptOutDoesNotReachTheViewer() { using var scope = new EngineScope(optOut: true); - var result = await EngineRunner.AddInlineAsync(new("Sample.cs", 42, "\"old\"", "new")); + var result = await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "new")); await Assert.That(result).IsEqualTo(EngineResult.NoViewerFound); await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty(); diff --git a/src/DiffEngineViewer.Tests/IpcTests.cs b/src/DiffEngineViewer.Tests/IpcTests.cs index be96c4bf..efd2f1ce 100644 --- a/src/DiffEngineViewer.Tests/IpcTests.cs +++ b/src/DiffEngineViewer.Tests/IpcTests.cs @@ -291,7 +291,7 @@ public async Task ARemovePatchIsRejected() { using var fixture = new ServerFixture(); - var response = fixture.Send(Inline(new("Sample.cs", 1, "\"old\"", "", InlinePatchMode.Remove))); + var response = fixture.Send(Inline(new("Sample.cs", 1, "\"old\"", "", InlinePatchMode.Remove) { TestName = null })); await Assert.That(response.Ok).IsFalse(); await Assert.That(fixture.Host.State.Queue).IsEmpty(); diff --git a/src/DiffEngineViewer.Tests/ViewerLaunchTests.cs b/src/DiffEngineViewer.Tests/ViewerLaunchTests.cs index 6ebc0e01..5d95e4ab 100644 --- a/src/DiffEngineViewer.Tests/ViewerLaunchTests.cs +++ b/src/DiffEngineViewer.Tests/ViewerLaunchTests.cs @@ -38,6 +38,21 @@ public class ViewerLaunchTests public static void Enable() => ManualViewer.Enable(); + // Unnamed, so the queue labels each of these by its call site — which is what the expectations + // below tell the reader to look for. The cases that are about naming set one themselves. + static engine::DiffEngine.InlinePatch Patch( + string source, + int line, + string? expression, + string content, + EnginePatchMode mode = EnginePatchMode.Set, + string? framework = null) => + new(source, line, expression, content, mode) + { + TestName = null, + Framework = framework + }; + /// /// The belt to WaitForClose's braces: a case that throws before it gets there would otherwise /// leave a hidden viewer to answer the next run. @@ -144,7 +159,7 @@ public async Task InlineReplacesALiteral() "Accept rewrites the literal in the source file"); var result = await EngineRunner.AddInlineAsync( - new(source, 6, "\"old value\"", "new value")); + Patch(source, 6, "\"old value\"", "new value")); await Assert.That(result).IsEqualTo(EngineResult.Queued); await ManualViewer.WaitForClose(); @@ -169,7 +184,7 @@ public async Task InlineAppendsToACallWithNoSnapshot() "Accept adds a .Snapshot(...) call after the verify call"); var result = await EngineRunner.AddInlineAsync( - new(source, 6, null, "brand new", EnginePatchMode.Append)); + Patch(source, 6, null, "brand new", EnginePatchMode.Append)); await Assert.That(result).IsEqualTo(EngineResult.Queued); await ManualViewer.WaitForClose(); @@ -205,7 +220,7 @@ public async Task InlineQueueFromSeparateLaunches() }) { var mode = expression is null ? EnginePatchMode.Append : EnginePatchMode.Set; - var result = await EngineRunner.AddInlineAsync(new(source, 6, expression, content, mode)); + var result = await EngineRunner.AddInlineAsync(Patch(source, 6, expression, content, mode)); await Assert.That(result).IsEqualTo(EngineResult.Queued); } @@ -233,7 +248,7 @@ public async Task InlineLongEnoughToScroll() "Accept writes the whole thing as a raw string literal"); var result = await EngineRunner.AddInlineAsync( - new(source, 6, "\"old value\"", Long(changed: true))); + Patch(source, 6, "\"old value\"", Long(changed: true))); await Assert.That(result).IsEqualTo(EngineResult.Queued); await ManualViewer.WaitForClose(); @@ -257,7 +272,7 @@ public async Task InlineDiscardLeavesTheSourceAlone() "Press Discard, or d", "The window closes because the queue is empty"); - await EngineRunner.AddInlineAsync(new(source, 6, "\"old value\"", "new value")); + await EngineRunner.AddInlineAsync(Patch(source, 6, "\"old value\"", "new value")); await ManualViewer.WaitForClose(); await Assert.That(await File.ReadAllTextAsync(source)).IsEqualTo(before); @@ -325,14 +340,8 @@ public async Task GroupedQueue() { TestName = "Order is stable" }, - new(conflicted, 6, "\"framework value\"", "eight") - { - Framework = "net8.0" - }, - new(conflicted, 6, "\"framework value\"", "nine") - { - Framework = "net9.0" - } + Patch(conflicted, 6, "\"framework value\"", "eight", framework: "net8.0"), + Patch(conflicted, 6, "\"framework value\"", "nine", framework: "net9.0") }) { var result = await EngineRunner.AddInlineAsync(patch);