Skip to content

feat: add string length range assertions#4935

Merged
thomhurst merged 4 commits intomainfrom
feat/string-length-assertions
Mar 23, 2026
Merged

feat: add string length range assertions#4935
thomhurst merged 4 commits intomainfrom
feat/string-length-assertions

Conversation

@thomhurst
Copy link
Owner

Summary

  • Adds HasMinLength(int minLength) assertion that verifies a string's length is >= the specified minimum
  • Adds HasMaxLength(int maxLength) assertion that verifies a string's length is <= the specified maximum
  • Adds HasLengthBetween(int min, int max) assertion that verifies a string's length is within an inclusive range

All three follow the existing assertion patterns (condition classes in StringAssertions.cs, extension methods in AssertionExtensions.cs) and support CallerArgumentExpression for readable error messages.

Closes #4868

Test plan

  • Verify HasMinLength passes for strings at or above the minimum length
  • Verify HasMinLength fails for strings below the minimum length
  • Verify HasMaxLength passes for strings at or below the maximum length
  • Verify HasMaxLength fails for strings above the maximum length
  • Verify HasLengthBetween passes for strings within the inclusive range
  • Verify HasLengthBetween fails for strings outside the range
  • Verify all three handle null strings with appropriate error messages
  • Verify And/Or chaining works (e.g., Assert.That(str).HasMinLength(3).And.HasMaxLength(10))

@claude
Copy link
Contributor

claude bot commented Feb 19, 2026

Code Review

The implementation follows the established patterns in StringAssertions.cs correctly — constructor, CheckAsync, GetExpectation, and the CallerArgumentExpression extension methods all match the existing convention for parameterized string assertions. The null/exception guard ordering is consistent with the rest of the file.

Two issues found:


1. Missing Public API snapshot updates (CLAUDE.md violation)

Severity: Required

TUnit.PublicAPI contains snapshot tests that verify the entire public surface area of the TUnit.Assertions assembly. The PR adds six new public members, but none of the four .verified.txt files were updated:

  • TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet8_0.verified.txt
  • TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet9_0.verified.txt
  • TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet10_0.verified.txt
  • TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.Net4_7.verified.txt

The analogous existing type StringLengthAssertion and its HasLength extension method are already captured in those snapshots. Without updating these files, the Public API tests will fail. Per CLAUDE.md Critical Rule 2:

"Changes to source generator output or public APIs require running snapshot tests. Commit .verified.txt files. NEVER commit .received.txt."

Fix: Run the TUnit.PublicAPI test suite and accept the generated .received.txt snapshots:

# From the TUnit.PublicAPI directory
dotnet test
# Then accept the output:
for f in *.received.txt; do mv "$f" "${f%.received.txt}.verified.txt"; done
git add *.verified.txt

2. HasLengthBetween silently produces an unsatisfiable assertion when minLength > maxLength

Severity: Design concern

When arguments are accidentally swapped — e.g., HasLengthBetween(10, 3) — the condition value.Length >= 10 && value.Length <= 3 is impossible to satisfy, so the assertion will silently always fail. The failure message will read something like "found length 5", giving the developer no indication that the real problem is the inverted bounds.

This is a genuine usability trap: the arguments have the same type (int, int), swapping them is easy, and the resulting failure message is actively misleading.

Every comparable assertion library (FluentAssertions, Shouldly, NUnit) throws ArgumentOutOfRangeException eagerly at construction time rather than letting the assertion silently become unsatisfiable.

Suggested fix — add a guard in HasLengthBetween (the extension method is the right place since it's the public entry point):

public static StringLengthBetweenAssertion HasLengthBetween(
    this IAssertionSource<string> source,
    int minLength,
    int maxLength,
    [CallerArgumentExpression(nameof(minLength))] string? minExpression = null,
    [CallerArgumentExpression(nameof(maxLength))] string? maxExpression = null)
{
    if (minLength > maxLength)
        throw new ArgumentOutOfRangeException(nameof(minLength),
            $"minLength ({minLength}) must be less than or equal to maxLength ({maxLength}).");
    source.Context.ExpressionBuilder.Append($".HasLengthBetween({minExpression}, {maxExpression})");
    return new StringLengthBetweenAssertion(source.Context, minLength, maxLength);
}

Reviewed by Claude — checked for bugs and CLAUDE.md compliance.

@thomhurst thomhurst force-pushed the feat/string-length-assertions branch from b950983 to 28ce284 Compare February 19, 2026 08:05
@thomhurst thomhurst force-pushed the feat/string-length-assertions branch from 28ce284 to e23c286 Compare February 19, 2026 09:38
@thomhurst thomhurst force-pushed the feat/string-length-assertions branch from e23c286 to cf5f7b1 Compare February 19, 2026 11:46
@thomhurst thomhurst force-pushed the feat/string-length-assertions branch from cf5f7b1 to c10dc39 Compare February 19, 2026 14:06
@thomhurst thomhurst force-pushed the feat/string-length-assertions branch from 1401303 to cf3d677 Compare February 19, 2026 16:40
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Add String Length Range Assertions

Summary

This PR adds three new string assertions:

  • HasMinLength(int minLength) — passes when string.Length >= minLength
  • HasMaxLength(int maxLength) — passes when string.Length <= maxLength
  • HasLengthBetween(int minLength, int maxLength) — passes when length is within an inclusive range

The implementation correctly follows the [GenerateAssertion] source generator pattern. ~380 of 512 changed lines are unrelated snapshot updates for a separate covariance improvement to exception assertions.


Previous Review Comments

The prior review flagged two issues: null handling and missing tests.

  • Null handling — the criticism was a false positive. The MethodAssertionGenerator automatically injects a null guard for all reference types, so condition methods never receive null. The null tests are still valuable regression coverage.
  • Tests — fully addressed with StringLengthAssertionTests.cs (30 tests added).

Issues Found

1. Missing source generator snapshot tests (medium priority)

The project has snapshot tests that verify exact generated code for each [GenerateAssertion]-decorated class (e.g., AssertionMethodGeneratorTests.GeneratesStringAssertions.*.verified.txt). The PR doesn't add corresponding snapshot tests for StringLengthRangeAssertionExtensions. This means the exact shape of the generated String_HasMinLength_Int_Assertion, String_HasMaxLength_Int_Assertion, and String_HasLengthBetween_Int_Int_Assertion classes is unverified — a regression in the generator would go undetected.

2. Unrelated covariance change bundled in (low priority)

About 380 of 512 changed lines are snapshot updates for a separate feature: making exception assertion extension methods covariant using <TActual> where TActual : TException. The covariance change itself looks correct, but mixing it here reduces reviewability and commit history clarity. Ideally this would be a separate PR.

3. Test file scope (minor)

Roughly half the tests in StringLengthAssertionTests.cs test the existing Length() numeric chain API (Length_IsEqualTo_*, Length_IsGreaterThan_*, etc.), not the new assertions. These belong in a pre-existing test file rather than one named after the new feature. They inflate the diff without testing what this PR adds.

4. Design note: HasLengthBetween validation timing

// StringLengthRangeAssertionExtensions.cs
public static AssertionResult HasLengthBetween(this string value, int minLength, int maxLength)
{
    if (minLength > maxLength)
        throw new ArgumentOutOfRangeException(nameof(minLength), ...);
    ...
}

The ArgumentOutOfRangeException test works because the exception propagates out of the generated CheckAsync, which the test wraps in Assert.That(async () => ...).Throws<ArgumentOutOfRangeException>(). This is functionally correct, but a programming mistake (min > max) surfaces as a runtime exception only when the assertion is awaited.

A more defensive alternative would be to validate at construction time — in the generated extension method rather than inside the condition method. However, with the current generator architecture this isn't possible without hand-authoring. Not a blocking issue; just worth noting that the guard is assertion-await-dependent.


What's Done Well

  1. Correct use of [GenerateAssertion] pattern[EditorBrowsable(EditorBrowsableState.Never)] is present, ExpectationMessage is provided for all three methods.
  2. Null safety is handled correctly by the generator's automatic null guard injection.
  3. Public API snapshots updated — all 4 target framework variants (net4.7, net8, net9, net10) have updated .verified.txt files.
  4. ArgumentOutOfRangeException guard in HasLengthBetween — prevents nonsensical range assertions.

Overall

The core feature is correct, well-implemented, and follows project conventions. The main recommendation is to add source generator snapshot tests for the new assertion classes. The null handling that was flagged in the previous review is handled correctly by the generator infrastructure — no changes needed there.

@thomhurst thomhurst enabled auto-merge (squash) March 23, 2026 14:05
@thomhurst thomhurst merged commit bb4d985 into main Mar 23, 2026
13 of 15 checks passed
@thomhurst thomhurst deleted the feat/string-length-assertions branch March 23, 2026 14:35
intellitect-bot pushed a commit to IntelliTect/EssentialCSharp.Web that referenced this pull request Mar 25, 2026
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.21.6 to
1.21.20.

<details>
<summary>Release notes</summary>

_Sourced from [TUnit's
releases](https://github.com/thomhurst/TUnit/releases)._

## 1.21.20

<!-- Release notes generated using configuration in .github/release.yml
at v1.21.20 -->

## What's Changed
### Other Changes
* fix: respect TUnitImplicitUsings set in Directory.Build.props by
@​thomhurst in thomhurst/TUnit#5225
* feat: covariant assertions for interfaces and non-sealed classes by
@​thomhurst in thomhurst/TUnit#5226
* feat: support string-to-parseable type conversions in [Arguments] by
@​thomhurst in thomhurst/TUnit#5227
* feat: add string length range assertions by @​thomhurst in
thomhurst/TUnit#4935
* Fix BeforeEvery/AfterEvery hooks for Class and Assembly not being
executed by @​Copilot in thomhurst/TUnit#5239
### Dependencies
* chore(deps): update tunit to 1.21.6 by @​thomhurst in
thomhurst/TUnit#5228
* chore(deps): update dependency gitversion.msbuild to 6.7.0 by
@​thomhurst in thomhurst/TUnit#5229
* chore(deps): update dependency gitversion.tool to v6.7.0 by
@​thomhurst in thomhurst/TUnit#5230
* chore(deps): update aspire to 13.2.0 - autoclosed by @​thomhurst in
thomhurst/TUnit#5232
* chore(deps): update dependency typescript to v6 by @​thomhurst in
thomhurst/TUnit#5233
* chore(deps): update dependency polyfill to 9.23.0 by @​thomhurst in
thomhurst/TUnit#5235
* chore(deps): update dependency polyfill to 9.23.0 by @​thomhurst in
thomhurst/TUnit#5236


**Full Changelog**:
thomhurst/TUnit@v1.21.6...v1.21.20

Commits viewable in [compare
view](thomhurst/TUnit@v1.21.6...v1.21.20).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=TUnit&package-manager=nuget&previous-version=1.21.6&new-version=1.21.20)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add string length range assertions (HasMinLength, HasMaxLength, HasLengthBetween)

1 participant