Skip to content

Conversation

@thomhurst
Copy link
Owner

Summary

  • Fixes xUnit Migration: Assert.All converted to non-existent AllSatisfy method #4335
  • xUnit Assert.All(collection, action) now converts to await Assert.That(collection).All(predicate) instead of the non-existent AllSatisfy method
  • Added TryConvertActionToPredicate to extract predicates from common xUnit assertion patterns
  • Also fixed MSTest migration to use All() instead of non-existent AllSatisfy()

Test plan

  • Added test for xUnit Assert.All conversion
  • All xUnit migration tests pass (41 total)
  • All MSTest migration tests pass (23 total)

🤖 Generated with Claude Code

…atisfy

Fixes #4335

- xUnit Assert.All(collection, action) now converts to
  await Assert.That(collection).All(predicate)
- Added TryConvertActionToPredicate to extract predicates from common
  xUnit assertion patterns (Assert.True, Assert.False, Assert.NotNull, etc.)
- Also fixed MSTest migration to use All() instead of non-existent AllSatisfy()

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst
Copy link
Owner Author

Summary

Fixes xUnit and MSTest migration to use the correct All(predicate) method instead of the non-existent AllSatisfy method.

Critical Issues

None found ✅

Suggestions

None - the implementation is clean and well-tested. The predicate extraction logic handles common xUnit assertion patterns (Assert.True, Assert.False, Assert.NotNull, Assert.Null) intelligently, and the fallback behavior for unrecognized patterns is appropriate.

TUnit Rules Check

✅ No dual-mode concerns (not touching core engine or source generator)
✅ No snapshot test changes needed (analyzer uses different test approach)
✅ No public API changes
✅ No hot path performance concerns (code fixer, not runtime)
✅ No reflection or AOT compatibility issues
✅ Modern C# syntax used appropriately

Verdict

APPROVE - Clean bug fix with proper test coverage. The conversion logic correctly transforms both xUnit Assert.All(collection, action) and MSTest CollectionAssert.AllItemsAreNotNull/AllItemsAreInstancesOfType to use TUnit's All(predicate) API.

Resolve merge conflicts to include Assert_Same, Assert_NotSame, and Assert_All tests.
Updated test expectations to not expect System.Threading.Tasks since this branch
doesn't have the code fix that adds it.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst
Copy link
Owner Author

Summary

Fixes xUnit Assert.All and MSTest AllItemsAreNotNull/AllItemsAreInstancesOfType migration to use the correct TUnit .All(predicate) method instead of the non-existent .AllSatisfy() method.

Critical Issues

None found ✅

Suggestions

1. Consider edge cases in TryConvertActionToPredicate

The TryConvertActionToPredicate method handles common patterns like Assert.True, Assert.False, Assert.NotNull, and Assert.Null. However, there are other common xUnit assertion patterns that might appear in Assert.All calls:

  • Assert.Equal(expected, actual)actual == expected
  • Assert.NotEqual(expected, actual)actual != expected
  • Assert.Empty(collection)!collection.Any()
  • Assert.NotEmpty(collection)collection.Any()

While the current implementation correctly falls back to returning the original expression (which will cause a compilation error and prompt manual conversion), adding these patterns could improve the migration experience.

Impact: Low priority enhancement - current behavior is safe (compilation error prompts manual fix).

2. Lambda formatting consistency

In CreateIsInstanceOfTypeLambda and CreateNotNullLambda, you're adding .WithArrowToken(...WithTrailingTrivia(SyntaxFactory.Space)) for spacing, which is good. However, in TryConvertActionToPredicate, the same pattern is applied. This is consistent and correct.

No action needed - just confirming this is intentional and consistent.

3. Test coverage for fallback behavior

The test Assert_All_Can_Be_Converted covers the happy path where Assert.True is successfully converted to a predicate. Consider adding a test case for when conversion isn't possible (e.g., Assert.All(items, item => item.DoSomething())) to verify the fallback behavior produces expected results.

Impact: Would improve test coverage but not critical since fallback behavior is simple.

Verdict

APPROVE - No critical issues

The fix correctly addresses issue #4335 by:

  1. Using the actual TUnit .All(predicate) method signature
  2. Intelligently converting common xUnit action patterns to predicates
  3. Providing safe fallback behavior for unconvertible patterns
  4. Adding test coverage for the conversion

The code is well-structured with good separation of concerns (CreateAllAssertionTryConvertActionToPredicate). The MSTest fixes are a bonus cleanup that prevents the same issue there.

@thomhurst thomhurst merged commit 14df79e into main Jan 12, 2026
13 checks passed
@thomhurst thomhurst deleted the fix/assert-all-migration-4335 branch January 12, 2026 23:29
This was referenced Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

xUnit Migration: Assert.All converted to non-existent AllSatisfy method

2 participants