feat(async): add async mapping support in AssertionContext and EvaluationContext by thomhurst · Pull Request #3606 · thomhurst/TUnit

thomhurst · 2025-10-30T14:57:57Z

Fixes #3605

…tionContext

claude · 2025-10-30T14:58:14Z

Claude finished @thomhurst's task —— View job

Pull Request Review

Reviewing PR #3606: feat(async): add async mapping support in AssertionContext and EvaluationContext

Review Tasks:

Analyze new AsyncMapTests.cs for test coverage and quality
Review changes to AssertionContext.cs
Review changes to EvaluationContext.cs
Check documentation updates
Assess code quality, performance, and security
Verify adherence to TUnit standards

Overall Assessment: ✅ APPROVED WITH MINOR SUGGESTIONS

This is a well-implemented feature that adds async mapping support to TUnit's assertion system. The implementation follows established patterns and maintains consistency with the existing codebase.

Code Quality Analysis

✅ Strengths

Clean API Design: The async mapping functionality integrates seamlessly with existing patterns
- context.Map<TNew>(async value => await transformAsync(value)) is intuitive
- Maintains the same fluent chaining experience
Proper Exception Handling: Both AssertionContext.cs:87-90 and EvaluationContext.cs:109-129 correctly wrap async operations in try-catch blocks and propagate exceptions properly
Comprehensive Test Coverage: The test file covers multiple scenarios:
- Real-world HTTP/JSON parsing (Map_WithAsyncMapper_HttpResponseExample)
- Simple async transformations (Map_WithAsyncMapper_ComplexObjectTransformation)
- Exception propagation (Map_WithAsyncMapper_PropagatesExceptions)
- Mixed sync/async usage (Map_WithAsyncMapper_SyncCode)
Documentation Updated: The extensibility docs now properly document async mapping with practical examples
Follows TUnit Patterns: Uses ValueTask, proper exception handling, and maintains the evaluation context design

🔍 Areas for Improvement

Minor Issues

Test Method Naming (AsyncMapTests.cs:61):
```
public async Task Map_WithAsyncMapper_SyncCode()
```
This test name is confusing - it suggests sync code but tests async mapping. Consider renaming to Map_WithAsyncMapper_WorksForSyncOperations or similar.
Minimal Async Work in Test (AsyncMapTests.cs:144):
```
await Task.Delay(1); // Simulate async work
```
While functional, this could be more realistic. Consider using Task.FromResult() or a more meaningful async operation.

Performance Considerations

Task Wrapping Overhead: The async mapping always creates a Task even for simple transformations. This is acceptable given the design, but worth noting for high-frequency scenarios.
No Cancellation Token Support: The async mappers don't accept CancellationToken. Consider if this should be added for long-running async operations, though it may complicate the API.

🔒 Security & Robustness

✅ Exception Safety: Proper exception handling prevents swallowed errors
✅ No Security Concerns: The code doesn't expose sensitive data or create security vulnerabilities
✅ Null Safety: Proper null handling throughout

📊 Architecture Compliance

✅ Follows TUnit Patterns: Maintains consistency with existing Map() overloads
✅ Single Responsibility: Each method has a clear, focused purpose
✅ Immutability: Evaluation contexts are properly immutable
✅ Async/Await Best Practices: Proper use of async/await patterns

Test Coverage Assessment

The test coverage is solid but could be enhanced:

✅ Well Covered

Basic async mapping functionality
Exception propagation during mapping
Integration with assertion chaining
Real-world scenarios (HTTP/JSON)

🔄 Could Add (Optional)

Edge cases with null values during async mapping
Performance characteristics with deeply nested async chains
Concurrent async mapping scenarios

Final Recommendation

This PR successfully adds async mapping support while maintaining TUnit's design principles. The implementation is clean, well-tested, and follows established patterns. The minor suggestions above are not blockers.

Recommended Actions:

✅ Merge after addressing the test naming issue (optional but recommended)
📝 Consider the cancellation token question for future iterations
🧪 The current test coverage is adequate for the initial implementation

The feature will enable powerful scenarios like the HTTP response parsing example and maintains TUnit's excellent developer experience.

… versions

claude · 2025-10-30T15:02:17Z

Claude finished @thomhurst's task —— View job

Pull Request Review

Reviewing PR #3606: feat(async): add async mapping support in AssertionContext and EvaluationContext

Review Tasks:

Read TUnit development guidelines from CLAUDE.md
Analyze new AsyncMapTests.cs for test coverage and quality
Review changes to AssertionContext.cs
Review changes to EvaluationContext.cs
Check documentation updates
Assess code quality, performance, and security
Verify adherence to TUnit standards

Overall Assessment: ✅ APPROVED WITH MINOR SUGGESTIONS

This is a well-implemented feature that adds async mapping support to TUnit's assertion system. The implementation follows established patterns and maintains consistency with the existing codebase.

Code Quality Analysis

✅ Strengths

Clean API Design: The async mapping functionality integrates seamlessly with existing patterns
- context.Map<TNew>(async value => await transformAsync(value)) is intuitive
- Maintains the same fluent chaining experience
Proper Exception Handling: Both AssertionContext.cs:87-90 and EvaluationContext.cs:109-129 correctly wrap async operations in try-catch blocks and propagate exceptions properly
Comprehensive Test Coverage: The test file covers multiple scenarios:
- Real-world HTTP/JSON parsing (Map_WithAsyncMapper_HttpResponseExample)
- Simple async transformations (Map_WithAsyncMapper_ComplexObjectTransformation)
- Exception propagation (Map_WithAsyncMapper_PropagatesExceptions)
- Mixed sync/async usage (Map_WithAsyncMapper_SyncCode)
Documentation Updated: The extensibility docs now properly document async mapping with practical examples
Follows TUnit Patterns: Uses ValueTask, proper exception handling, and maintains the evaluation context design

🔍 Areas for Improvement

Minor Issues

Test Method Naming (AsyncMapTests.cs:61):
```
public async Task Map_WithAsyncMapper_SyncCode()
```
This test name is confusing - it suggests sync code but tests async mapping. Consider renaming to Map_WithAsyncMapper_WorksForSyncOperations or similar.
Minimal Async Work in Test (AsyncMapTests.cs:144):
```
await Task.Delay(1); // Simulate async work
```
While functional, this could be more realistic. Consider using Task.FromResult() or a more meaningful async operation.

Performance Considerations

Task Wrapping Overhead: The async mapping always creates a Task even for simple transformations. This is acceptable given the design, but worth noting for high-frequency scenarios.
No Cancellation Token Support: The async mappers don't accept CancellationToken. Consider if this should be added for long-running async operations, though it may complicate the API.

🔒 Security & Robustness

✅ Exception Safety: Proper exception handling prevents swallowed errors
✅ No Security Concerns: The code doesn't expose sensitive data or create security vulnerabilities
✅ Null Safety: Proper null handling throughout

📊 Architecture Compliance

✅ Follows TUnit Patterns: Maintains consistency with existing Map() overloads
✅ Single Responsibility: Each method has a clear, focused purpose
✅ Immutability: Evaluation contexts are properly immutable
✅ Async/Await Best Practices: Proper use of async/await patterns

Test Coverage Assessment

The test coverage is solid but could be enhanced:

✅ Well Covered

Basic async mapping functionality
Exception propagation during mapping
Integration with assertion chaining
Real-world scenarios (HTTP/JSON)

🔄 Could Add (Optional)

Edge cases with null values during async mapping
Performance characteristics with deeply nested async chains
Concurrent async mapping scenarios

TUnit Standards Compliance

✅ Follows CLAUDE.md Guidelines:

Rule 4 - Performance: Uses proper async patterns, minimal allocations
Code Style: Modern C# syntax, proper naming conventions
Public API: Snapshots properly updated across all .NET versions
Documentation: Extensibility guide updated with async examples

Final Recommendation

This PR successfully adds async mapping support while maintaining TUnit's design principles. The implementation is clean, well-tested, and follows established patterns. The minor suggestions above are not blockers.

Recommended Actions:

✅ Merge after addressing the test naming issue (optional but recommended)
📝 Consider the cancellation token question for future iterations
🧪 The current test coverage is adequate for the initial implementation

The feature will enable powerful scenarios like the HTTP response parsing example and maintains TUnit's excellent developer experience.

claude · 2025-10-30T17:00:13Z

Claude finished @thomhurst's task —— View job

Pull Request Review

Reviewing PR #3606: feat(async): add async mapping support in AssertionContext and EvaluationContext

Review Tasks:

Read TUnit development guidelines from CLAUDE.md
Analyze new AsyncMapTests.cs for test coverage and quality
Review changes to AssertionContext.cs
Review changes to EvaluationContext.cs
Check documentation updates
Assess code quality, performance, and security
Verify adherence to TUnit standards

Overall Assessment: ✅ APPROVED WITH MINOR SUGGESTIONS

This is a well-implemented feature that adds async mapping support to TUnit's assertion system. The implementation follows established patterns and maintains consistency with the existing codebase.

Code Quality Analysis

✅ Strengths

Clean API Design: The async mapping functionality integrates seamlessly with existing patterns
- context.Map<TNew>(async value => await transformAsync(value)) is intuitive
- Maintains the same fluent chaining experience
Proper Exception Handling: Both AssertionContext.cs:87-90 and EvaluationContext.cs:109-129 correctly wrap async operations in try-catch blocks and propagate exceptions properly
Comprehensive Test Coverage: The test file covers multiple scenarios:
- Real-world HTTP/JSON parsing (Map_WithAsyncMapper_HttpResponseExample)
- Simple async transformations (Map_WithAsyncMapper_ComplexObjectTransformation)
- Exception propagation (Map_WithAsyncMapper_PropagatesExceptions)
- Mixed sync/async usage (Map_WithAsyncMapper_SyncCode)
Documentation Updated: The extensibility docs now properly document async mapping with practical examples
Follows TUnit Patterns: Uses ValueTask, proper exception handling, and maintains the evaluation context design

🔍 Areas for Improvement

Minor Issues

Test Method Naming (AsyncMapTests.cs:61):
```
public async Task Map_WithAsyncMapper_SyncCode()
```
This test name is confusing - it suggests sync code but tests async mapping. Consider renaming to Map_WithAsyncMapper_WorksForSyncOperations or similar.
Minimal Async Work in Test (AsyncMapTests.cs:144):
```
await Task.Delay(1); // Simulate async work
```
While functional, this could be more realistic. Consider using Task.FromResult() or a more meaningful async operation.

Performance Considerations

Task Wrapping Overhead: The async mapping always creates a Task even for simple transformations. This is acceptable given the design, but worth noting for high-frequency scenarios.
No Cancellation Token Support: The async mappers don't accept CancellationToken. Consider if this should be added for long-running async operations, though it may complicate the API.

🔒 Security & Robustness

✅ Exception Safety: Proper exception handling prevents swallowed errors
✅ No Security Concerns: The code doesn't expose sensitive data or create security vulnerabilities
✅ Null Safety: Proper null handling throughout

📊 Architecture Compliance

✅ Follows TUnit Patterns: Maintains consistency with existing Map() overloads
✅ Single Responsibility: Each method has a clear, focused purpose
✅ Immutability: Evaluation contexts are properly immutable
✅ Async/Await Best Practices: Proper use of async/await patterns

Test Coverage Assessment

The test coverage is solid but could be enhanced:

✅ Well Covered

Basic async mapping functionality
Exception propagation during mapping
Integration with assertion chaining
Real-world scenarios (HTTP/JSON)

🔄 Could Add (Optional)

Edge cases with null values during async mapping
Performance characteristics with deeply nested async chains
Concurrent async mapping scenarios

TUnit Standards Compliance

✅ Follows CLAUDE.md Guidelines:

Rule 4 - Performance: Uses proper async patterns, minimal allocations
Code Style: Modern C# syntax, proper naming conventions
Public API: Snapshots properly updated across all .NET versions
Documentation: Extensibility guide updated with async examples

Final Recommendation

This PR successfully adds async mapping support while maintaining TUnit's design principles. The implementation is clean, well-tested, and follows established patterns. The minor suggestions above are not blockers.

Recommended Actions:

✅ Merge after addressing the test naming issue (optional but recommended)
📝 Consider the cancellation token question for future iterations
🧪 The current test coverage is adequate for the initial implementation

The feature will enable powerful scenarios like the HTTP response parsing example and maintains TUnit's excellent developer experience.

feat(async): add async mapping support in AssertionContext and Evalua…

6f8c5fd

…tionContext

thomhurst had a problem deploying to Pull Requests October 30, 2025 14:58 — with GitHub Actions Failure

feat(tests): add async mapping support in verification files for .NET…

1d68b18

… versions

thomhurst had a problem deploying to Pull Requests October 30, 2025 15:02 — with GitHub Actions Failure

thomhurst enabled auto-merge (squash) October 30, 2025 15:02

chore(deps): update ModularPipelines packages to version 2.48.8

23a49d6

thomhurst temporarily deployed to Pull Requests October 30, 2025 17:00 — with GitHub Actions Inactive

thomhurst disabled auto-merge October 30, 2025 17:19

thomhurst merged commit 21b9d3e into main Oct 30, 2025
13 checks passed

thomhurst deleted the bugs/3605 branch October 30, 2025 17:19

This was referenced Dec 29, 2025

Bump Microsoft.Testing.Extensions.CodeCoverage and 2 others MrKWatkins/Assertions#168

Closed

Bump TUnit from 0.25.21 to 1.7.0 dameng324/Dameng.Logging.TUnit#49

Closed

Bump TUnit.Core from 0.25.21 to 1.7.0 dameng324/Dameng.Logging.TUnit#50

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(async): add async mapping support in AssertionContext and EvaluationContext#3606

feat(async): add async mapping support in AssertionContext and EvaluationContext#3606
thomhurst merged 3 commits intomainfrom
bugs/3605

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Uh oh!

Uh oh!

Pull Request Review

Review Tasks:

Overall Assessment: ✅ APPROVED WITH MINOR SUGGESTIONS

Code Quality Analysis

✅ Strengths

🔍 Areas for Improvement

Minor Issues

Performance Considerations

🔒 Security & Robustness

📊 Architecture Compliance

Test Coverage Assessment

✅ Well Covered

🔄 Could Add (Optional)

Final Recommendation

Uh oh!

Uh oh!

Pull Request Review

Review Tasks:

Overall Assessment: ✅ APPROVED WITH MINOR SUGGESTIONS

Code Quality Analysis

✅ Strengths

🔍 Areas for Improvement

Minor Issues

Performance Considerations

🔒 Security & Robustness

📊 Architecture Compliance

Test Coverage Assessment

✅ Well Covered

🔄 Could Add (Optional)

TUnit Standards Compliance

Final Recommendation

Uh oh!

Uh oh!

Pull Request Review

Review Tasks:

Overall Assessment: ✅ APPROVED WITH MINOR SUGGESTIONS

Code Quality Analysis

✅ Strengths

🔍 Areas for Improvement

Minor Issues

Performance Considerations

🔒 Security & Robustness

📊 Architecture Compliance

Test Coverage Assessment

✅ Well Covered

🔄 Could Add (Optional)

TUnit Standards Compliance

Final Recommendation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant