Skip to content

Fix two CombinationResultsConverter edge cases - #1867

Merged
SimonCropp merged 1 commit into
mainfrom
fix-combinations-converter-edges
Aug 17, 2026
Merged

Fix two CombinationResultsConverter edge cases#1867
SimonCropp merged 1 commit into
mainfrom
fix-combinations-converter-edges

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

Unclosed object for empty results. Write calls WriteStartObject(), then returns early when there are no items without calling WriteEndObject(). The writer auto-closes at the end of the document, so a top level empty result still renders as {} and looks fine — but anything written after the results gets swallowed into them. On main:

{
  results: {
    after: TheValue     <- sibling property, now nested inside the results
  }
}

Only reachable by constructing CombinationResults directly, since CombinationRunner rejects empty lists.

Flattened messages lost the joining space. FlattenMessage appended ". " only 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:

ArgumentException: Value cannot be null.Parameter name: p.

The period and the separating space are now appended independently.

Tests in Verify.Tests/CombinationTests: EmptyResults and EmptyResultsNested for the first (the nested one is what actually fails on main — the flat one passes either way, which is why the bug is easy to miss), and MultiLineArgumentException for the second, using an explicit two line message so it does not depend on how a given framework words ArgumentNullException.

Empty lines in a message still contribute a . — unchanged, and out of scope here.

Verify.Tests (1302) and StaticSettingsTests pass. docs/combinations.md regenerates because it embeds the converter source.

… 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".
@SimonCropp SimonCropp added this to the 32.0.0 milestone Aug 17, 2026
@SimonCropp
SimonCropp merged commit 076fb6a into main Aug 17, 2026
7 checks passed
@SimonCropp
SimonCropp deleted the fix-combinations-converter-edges branch August 17, 2026 12:59
This was referenced Aug 26, 2026
This was referenced Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant