feat: add System.Threading.Tasks using and preserve TestCase properties#4203
feat: add System.Threading.Tasks using and preserve TestCase properties#4203
Conversation
SummaryThis PR adds comprehensive improvements to migration code fixers: automatic Critical IssuesNone found ✅ Suggestions1. Consider ValueTask for consistency (minor)The new
This is working as intended - just noting for awareness. 2. Message handling looks comprehensiveThe addition of message/format arg extraction ( 3. Test coverageThe PR description mentions:
Excellent test coverage for migration scenarios. TUnit Rules Compliance✅ Rule 1 (Dual-Mode): Not applicable - analyzer/code fixer changes only Verdict✅ APPROVE - No critical issues. The code is well-structured, follows TUnit principles, and has comprehensive test coverage. The migration improvements will significantly improve the user experience when migrating from other frameworks. |
- Add `using System.Threading.Tasks;` when async code is present in migration - Preserve NUnit TestCase named properties during migration: - Map Ignore/IgnoreReason to TUnit's Skip property - Add TODO comments for unsupported properties (TestName, Category, Description, etc.) - Update test expectations to include System.Threading.Tasks using 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
a9486ef to
cf2315d
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances the test framework migration code fixers by adding automatic using System.Threading.Tasks; imports when async code is detected, and preserving NUnit TestCase properties during migration (mapping Ignore/IgnoreReason to TUnit's Skip, and adding TODO comments for unsupported properties).
Key Changes
- Automatically adds
using System.Threading.Tasks;when async/await patterns are detected during migration - Preserves NUnit TestCase named properties with appropriate mappings and TODO comments for unsupported features
- Extended assertion conversion support across NUnit, xUnit, and MSTest migrations with message parameter handling
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs | Adds conditional using System.Threading.Tasks; when async code is detected |
| TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs | Maps NUnit TestCase properties (Ignore→Skip) and adds extensive assertion conversions with message support |
| TUnit.Analyzers.CodeFixers/NUnitExpectedResultRewriter.cs | Preserves TestCase properties in ExpectedResult scenarios with TODO comments for unsupported properties |
| TUnit.Analyzers.CodeFixers/XUnitMigrationCodeFixProvider.cs | Extended xUnit assertion conversions with comparer detection and message handling |
| TUnit.Analyzers.CodeFixers/MSTestMigrationCodeFixProvider.cs | Extended MSTest assertion conversions with proper message parameter positioning |
| TUnit.Analyzers.CodeFixers/Base/TestAttributeEnsurer.cs | New rewriter ensuring [Test] attribute exists when data attributes are present (NUnit-specific) |
| TUnit.Analyzers.CodeFixers/Base/BaseMigrationCodeFixProvider.cs | Integrates AsyncMethodSignatureRewriter and TestAttributeEnsurer into migration pipeline |
| TUnit.Analyzers.CodeFixers/Base/AsyncMethodSignatureRewriter.cs | New rewriter converting void/T methods to async Task/Task when await is present |
| TUnit.Analyzers.CodeFixers/Base/AssertionRewriter.cs | Adds message handling with .Because() chaining and helper methods for format strings and comparer detection |
| TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs | Adds comprehensive tests verifying async using import, TestCase handling, and assertion message conversion |
| TUnit.Analyzers.Tests/MSTestMigrationAnalyzerTests.cs | Adds test verifying MSTest assertion message preservation |
| .Any(n => n is AwaitExpressionSyntax || | ||
| (n is MethodDeclarationSyntax m && m.Modifiers.Any(mod => mod.IsKind(SyntaxKind.AsyncKeyword)))); | ||
|
|
||
| if (hasAsyncCode && !existingUsings.Any(u => u.Name?.ToString() == "System.Threading.Tasks")) |
There was a problem hiding this comment.
The check for existing System.Threading.Tasks using should use string comparison with qualified name. The current comparison u.Name?.ToString() == "System.Threading.Tasks" might not work correctly if the using directive uses an alias or has different formatting. Consider checking against the full qualified name more robustly or ensuring the name is normalized before comparison.
Summary
Follow-up improvements to the migration code fixers based on user feedback:
using System.Threading.Tasks;when async code is present in migrationIgnore/IgnoreReasonto TUnit'sSkippropertyTestName,Category,Description,Author,Explicit,ExplicitReason)Test plan
🤖 Generated with Claude Code