feat: enhance failure messages for All assertions to include value

[thomhurst]

Fixes #4137

[claude]

Code Review for PR #4139: Enhance failure messages for All assertions

Summary

This PR successfully addresses issue #4137 by improving the error message when .All() assertions fail. The change adds the actual value that failed the predicate to the error message, making debugging significantly easier.

---

Review Details

Code Quality: Excellent

• Change is minimal and focused: Single line modification in CollectionAssertions.cs:426
• Message format is consistent: Uses the same [{item}] bracket notation as other assertions in the file (lines 796, 852)
• Follows existing patterns: The error message structure matches other index-based failures in the codebase

Test Coverage: Excellent

The PR includes 3 comprehensive test cases covering:

1. Numeric values: Tests with negative number (-5) in array of integers
2. String values: Tests with empty string in array of strings
3. First failing item: Verifies it shows the first failing item, not subsequent ones

Test quality observations:

• Tests properly verify both the index AND the value in the error message
• Good edge case coverage (empty string, negative numbers)
• Tests use .WithMessageContaining() appropriately for partial message matching

Potential Issues & Suggestions

Minor: Null item handling

Issue: The current implementation uses string interpolation [{item}] which will display [] for null items, which could be ambiguous with empty strings.

Example scenario:

var items = new string?[] { "Alice", null, "Bob" };
await Assert.That(items).All(x => x != null);
// Current: "item at index 1 with value [] does not satisfy predicate"
// Could be confused with empty string

Suggestion: Consider adding a test case for null items to verify the behavior is acceptable, or enhance message formatting for null values.

Observation: Complex object ToString()

For complex objects without custom ToString() implementations, the output may not be helpful (e.g., [MyNamespace.MyClass]). This is acceptable for now since:

• It is consistent with other assertions in the codebase
• Users can implement custom ToString() if needed
• It is still better than not showing the value at all

---

Security: No Concerns

• No security implications for this change
• String interpolation is safe here (not used in SQL/command contexts)

Performance: No Concerns

• Negligible performance impact (single string interpolation during assertion failure)
• Assertion failures are not hot path scenarios

Adherence to TUnit Guidelines

Checking against CLAUDE.md requirements:

• Modern C# syntax: String interpolation is appropriate
• Code style: Follows existing patterns in the file
• Dual-mode not applicable: This is in the assertions library, not core execution
• No API changes: Internal implementation detail only
• AOT/Trimming: No reflection concerns

---

Recommendation: APPROVE with suggestion

The PR is ready to merge with the current implementation. The suggested null handling improvement is optional and could be addressed in a follow-up if desired.

Rationale:

1. Successfully solves the reported issue [Feature]: Clearer assertion messages stating the actual versus expected value #4137
2. Excellent test coverage for common scenarios
3. Consistent with existing codebase patterns
4. Improves developer experience significantly

Optional follow-up: Consider adding null item test case to verify behavior is acceptable, or enhance message formatting for null values.

---

Final Notes

Great work on this improvement! The enhanced error messages will make debugging failed .All() assertions much easier. The test coverage is particularly well done, covering multiple data types and edge cases.

[Copilot]

Pull request overview

This PR enhances the failure message for the All assertion on collections to include the actual value of the failing item, making it easier to diagnose test failures. Previously, the error message only indicated which index failed; now it shows both the index and the value.

Key Changes

• Enhanced error message format in CollectionAllAssertion to include the failing item's value
• Added comprehensive test coverage for the new error message format with multiple scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
TUnit.Assertions/Conditions/CollectionAssertions.cs	Updated error message in CollectionAllAssertion.CheckAsync to include with value [{item}] for better diagnostics
TUnit.Assertions.Tests/CollectionAssertionTests.cs	Added three test cases verifying the enhanced error messages work correctly for integers, strings (including empty), and multiple failing items

[Copilot]

The error message format uses square brackets [{item}] to display the value, but other similar error messages in this file use parentheses ({item}) format (see lines 796 and 852 in CollectionIsInOrderAssertion and CollectionIsInDescendingOrderAssertion). For consistency, consider using parentheses format: "item at index {index} ({item}) does not satisfy predicate" to match the existing pattern in the codebase.

Suggested change

	return Task.FromResult(AssertionResult.Failed($"item at index {index} with value [{item}] does not satisfy predicate"));
	return Task.FromResult(AssertionResult.Failed($"item at index {index} ({item}) does not satisfy predicate"));

[Copilot]

The error message directly interpolates {item} which could display as an empty string if the item is null. Consider adding a test case for collections containing null values to verify the error message displays appropriately (e.g., "with value [null]" or similar).