From 57377751dfd31d2110c2637e9126250e4aeebb54 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 17 Aug 2026 21:58:49 +1000 Subject: [PATCH] Close the object for empty combination results, and keep the space in flattened messages Two edge cases in CombinationResultsConverter: * Write started an object then returned early for empty results without closing it, so anything written after the results was swallowed into them. Reachable by constructing CombinationResults directly, since the runner rejects empty lists. * FlattenMessage only appended ". " when a line did not already end in a period, so a line that did was run straight into the next one. The two line net48 ArgumentNullException rendered as "Value cannot be null.Parameter name: p". --- docs/combinations.md | 8 +++-- ...CombinationTests.EmptyResults.verified.txt | 1 + ...ationTests.EmptyResultsNested.verified.txt | 4 +++ ...ts.MultiLineArgumentException.verified.txt | 4 +++ src/Verify.Tests/CombinationTests.cs | 32 +++++++++++++++++++ .../CombinationResultsConverter.cs | 6 +++- src/todo.md | 4 +-- 7 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 src/Verify.Tests/CombinationTests.EmptyResults.verified.txt create mode 100644 src/Verify.Tests/CombinationTests.EmptyResultsNested.verified.txt create mode 100644 src/Verify.Tests/CombinationTests.MultiLineArgumentException.verified.txt diff --git a/docs/combinations.md b/docs/combinations.md index ebcd5c37f5..82f82242c0 100644 --- a/docs/combinations.md +++ b/docs/combinations.md @@ -198,6 +198,7 @@ public class CombinationResultsConverter : var items = results.Items; if (items.Count == 0) { + writer.WriteEndObject(); return; } @@ -363,8 +364,11 @@ public class CombinationResultsConverter : builder.Append(trimmed); if (!trimmed.EndsWith('.')) { - builder.Append(". "); + builder.Append('.'); } + + // separate from the next line even when the sentence was already terminated + builder.Append(' '); } builder.TrimEnd(); @@ -372,7 +376,7 @@ public class CombinationResultsConverter : } } ``` -snippet source | anchor +snippet source | anchor diff --git a/src/Verify.Tests/CombinationTests.EmptyResults.verified.txt b/src/Verify.Tests/CombinationTests.EmptyResults.verified.txt new file mode 100644 index 0000000000..22fdca1b26 --- /dev/null +++ b/src/Verify.Tests/CombinationTests.EmptyResults.verified.txt @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/src/Verify.Tests/CombinationTests.EmptyResultsNested.verified.txt b/src/Verify.Tests/CombinationTests.EmptyResultsNested.verified.txt new file mode 100644 index 0000000000..ab63efffc4 --- /dev/null +++ b/src/Verify.Tests/CombinationTests.EmptyResultsNested.verified.txt @@ -0,0 +1,4 @@ +{ + results: {}, + after: TheValue +} \ No newline at end of file diff --git a/src/Verify.Tests/CombinationTests.MultiLineArgumentException.verified.txt b/src/Verify.Tests/CombinationTests.MultiLineArgumentException.verified.txt new file mode 100644 index 0000000000..62a8b70635 --- /dev/null +++ b/src/Verify.Tests/CombinationTests.MultiLineArgumentException.verified.txt @@ -0,0 +1,4 @@ +{ + 1: ArgumentException: Value cannot be null. Parameter name: p., + 10: ArgumentException: Value cannot be null. Parameter name: p. +} \ No newline at end of file diff --git a/src/Verify.Tests/CombinationTests.cs b/src/Verify.Tests/CombinationTests.cs index e9bebf7c6a..962fbf1995 100644 --- a/src/Verify.Tests/CombinationTests.cs +++ b/src/Verify.Tests/CombinationTests.cs @@ -159,6 +159,38 @@ public Task RecordingWithExceptionPausedTest() .IgnoreStackTrace(); } + static string ThrowMultiLine(int value) => + throw new ArgumentException( + """ + Value cannot be null. + Parameter name: p + """); + + // ArgumentException messages are flattened onto one line, so the parts of the + // message need a separator between them + [Fact] + public Task MultiLineArgumentException() => + Combination(captureExceptions: true) + .Verify( + ThrowMultiLine, + params1); + + // Only reachable by constructing the results directly, since the runner requires + // every list to have at least one item + [Fact] + public Task EmptyResults() => + Verify(new CombinationResults([], [], null)); + + // an unclosed object would swallow everything written after it + [Fact] + public Task EmptyResultsNested() => + Verify( + new + { + results = new CombinationResults([], [], null), + after = "TheValue" + }); + [Fact] public Task RecordingPausedTest() { diff --git a/src/Verify/Combinations/CombinationResultsConverter.cs b/src/Verify/Combinations/CombinationResultsConverter.cs index 6094398f4b..b7d5f1b2e7 100644 --- a/src/Verify/Combinations/CombinationResultsConverter.cs +++ b/src/Verify/Combinations/CombinationResultsConverter.cs @@ -10,6 +10,7 @@ public override void Write(VerifyJsonWriter writer, CombinationResults results) var items = results.Items; if (items.Count == 0) { + writer.WriteEndObject(); return; } @@ -175,8 +176,11 @@ static string FlattenMessage(string message) builder.Append(trimmed); if (!trimmed.EndsWith('.')) { - builder.Append(". "); + builder.Append('.'); } + + // separate from the next line even when the sentence was already terminated + builder.Append(' '); } builder.TrimEnd(); diff --git a/src/todo.md b/src/todo.md index 03784f69b0..61adbb3dc7 100644 --- a/src/todo.md +++ b/src/todo.md @@ -67,10 +67,10 @@ All six resolved 2026-08-16 (five fixed here; the inline item resolved as by-des - [ ] **MSTest source generator ignores `record` test classes.** `Verify.MSTest.SourceGenerator/UsesVerifyGenerator.cs:125-126` — only `ClassDeclarationSyntax` is eligible; `[UsesVerify] [TestClass] partial record` compiles then fails at runtime with the misleading "TestContext is null" error. `Parser.GetParentClasses` similarly stops at a `record struct` parent. -- [ ] **Unclosed JSON object for empty `CombinationResults`.** +- [x] **Unclosed JSON object for empty `CombinationResults`.** `Verify/Combinations/CombinationResultsConverter.cs:8-14` — `WriteStartObject()` then early `return` with no `WriteEndObject()`. Only reachable by constructing `CombinationResults([], ...)` directly. -- [ ] **`FlattenMessage` omits the joining space after a line ending in `.`.** +- [x] **`FlattenMessage` omits the joining space after a line ending in `.`.** `Verify/Combinations/CombinationResultsConverter.cs:168-183` — two-line messages (net48 `ArgumentNullException`) render as `"Value cannot be null.Parameter name: p"`. - [ ] **Negative sub-hour offsets render unsigned.**