From 3750e25395acd33cdd546ca2b238e45d197adbd9 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 15 Jan 2026 16:08:41 +0000 Subject: [PATCH] feat: add comprehensive kitchen sink migration tests and improve code fixers - Add comprehensive kitchen sink tests for NUnit, MSTest, and XUnit migrations covering setup/teardown hooks, various test attributes, assertion methods, collection assertions, and skip/ignore functionality - Fix NUnit migration code fixer to handle: - Does.Contain() -> .Contains() - Contains.Item() -> .Contains() - Description attribute -> [Property("Description", "...")] - Update test expectation: NUnit Description attribute now converts to Property instead of being removed - All 864 migration tests pass (520 NUnit + 144 MSTest + 208 XUnit) Co-Authored-By: Claude Opus 4.5 --- .../Base/BaseMigrationCodeFixProvider.cs | 65 +- .../MSTestMigrationCodeFixProvider.cs | 31 +- .../NUnitMigrationCodeFixProvider.cs | 83 ++- .../MSTestMigrationAnalyzerTests.cs | 434 ++++++++++++ .../NUnitMigrationAnalyzerTests.cs | 620 +++++++++++++++++- .../XUnitMigrationAnalyzerTests.cs | 227 +++++++ .../Migrators/Base/MigrationHelpers.cs | 3 +- 7 files changed, 1427 insertions(+), 36 deletions(-) diff --git a/TUnit.Analyzers.CodeFixers/Base/BaseMigrationCodeFixProvider.cs b/TUnit.Analyzers.CodeFixers/Base/BaseMigrationCodeFixProvider.cs index 112a15cdda..24b696f0c9 100644 --- a/TUnit.Analyzers.CodeFixers/Base/BaseMigrationCodeFixProvider.cs +++ b/TUnit.Analyzers.CodeFixers/Base/BaseMigrationCodeFixProvider.cs @@ -141,33 +141,46 @@ protected async Task ConvertCodeAsync(Document document, SyntaxNode? r /// protected static CompilationUnitSyntax CleanupClassMemberLeadingTrivia(CompilationUnitSyntax root) { - var classesToFix = root.DescendantNodes().OfType() - .Where(c => c.Members.Any()) - .ToList(); - var currentRoot = root; - foreach (var classDecl in classesToFix) + + // Use a while loop to re-query after each modification. + // This is necessary because ReplaceNode returns a new tree, and node references + // from the original tree won't match nodes in the new tree. + ClassDeclarationSyntax? classToFix; + while ((classToFix = FindClassWithExcessiveLeadingTrivia(currentRoot)) != null) { - var firstMember = classDecl.Members.First(); + var firstMember = classToFix.Members.First(); var leadingTrivia = firstMember.GetLeadingTrivia(); - int newlineCount = leadingTrivia.Count(t => t.IsKind(SyntaxKind.EndOfLineTrivia)); - if (newlineCount > 0) - { - // Keep only indentation (whitespace), remove all newlines - var triviaToKeep = leadingTrivia - .Where(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)) - .Where(t => t.IsKind(SyntaxKind.WhitespaceTrivia) || - (!t.IsKind(SyntaxKind.WhitespaceTrivia) && !t.IsKind(SyntaxKind.EndOfLineTrivia))) - .ToList(); + // Keep only indentation (whitespace), remove all newlines + var triviaToKeep = leadingTrivia + .Where(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)) + .Where(t => t.IsKind(SyntaxKind.WhitespaceTrivia) || + (!t.IsKind(SyntaxKind.WhitespaceTrivia) && !t.IsKind(SyntaxKind.EndOfLineTrivia))) + .ToList(); - var newFirstMember = firstMember.WithLeadingTrivia(triviaToKeep); - var updatedClass = classDecl.ReplaceNode(firstMember, newFirstMember); - currentRoot = currentRoot.ReplaceNode(classDecl, updatedClass); - } + var newFirstMember = firstMember.WithLeadingTrivia(triviaToKeep); + var updatedClass = classToFix.ReplaceNode(firstMember, newFirstMember); + currentRoot = currentRoot.ReplaceNode(classToFix, updatedClass); } - return (CompilationUnitSyntax)currentRoot; + return currentRoot; + } + + /// + /// Finds a class with excessive leading trivia on its first member. + /// Returns null if no such class exists. + /// + private static ClassDeclarationSyntax? FindClassWithExcessiveLeadingTrivia(CompilationUnitSyntax root) + { + return root.DescendantNodes() + .OfType() + .Where(c => c.Members.Any()) + .FirstOrDefault(c => + { + var leadingTrivia = c.Members.First().GetLeadingTrivia(); + return leadingTrivia.Any(t => t.IsKind(SyntaxKind.EndOfLineTrivia)); + }); } /// @@ -251,11 +264,11 @@ public abstract class AttributeRewriter : CSharpSyntaxRewriter var hookAttributeList = MigrationHelpers.ConvertHookAttribute(attribute, FrameworkName); if (hookAttributeList != null) { - // Preserve only the leading trivia (indentation) from the original node - // and strip any trailing trivia to prevent extra blank lines - return hookAttributeList - .WithLeadingTrivia(node.GetLeadingTrivia()) - .WithTrailingTrivia(node.GetTrailingTrivia()); + // Add converted hook attribute(s) to the list - don't return early! + // This preserves other attributes that may be in the same attribute list. + // e.g., [SetUp, Category("Unit")] -> [Before(HookType.Test), Category("Unit")] + attributes.AddRange(hookAttributeList.Attributes); + continue; } } @@ -268,6 +281,8 @@ public abstract class AttributeRewriter : CSharpSyntaxRewriter return attributes.Count > 0 ? node.WithAttributes(SyntaxFactory.SeparatedList(attributes)) + .WithLeadingTrivia(node.GetLeadingTrivia()) + .WithTrailingTrivia(node.GetTrailingTrivia()) : null; } diff --git a/TUnit.Analyzers.CodeFixers/MSTestMigrationCodeFixProvider.cs b/TUnit.Analyzers.CodeFixers/MSTestMigrationCodeFixProvider.cs index 02b4b7c411..685c3c4d82 100644 --- a/TUnit.Analyzers.CodeFixers/MSTestMigrationCodeFixProvider.cs +++ b/TUnit.Analyzers.CodeFixers/MSTestMigrationCodeFixProvider.cs @@ -161,14 +161,17 @@ private AttributeArgumentListSyntax ConvertOwnerArguments(AttributeArgumentListS public override SyntaxNode? VisitAttributeList(AttributeListSyntax node) { // Handle ClassInitialize and ClassCleanup specially - they need static context parameter removed + // Process all attributes, don't return early to preserve sibling attributes var attributes = new List(); - + bool hasClassLifecycleAttribute = false; + foreach (var attribute in node.Attributes) { var attributeName = MigrationHelpers.GetAttributeName(attribute); - + if (attributeName is "ClassInitialize" or "ClassCleanup") { + hasClassLifecycleAttribute = true; var hookType = attributeName == "ClassInitialize" ? "Before" : "After"; var newAttribute = SyntaxFactory.Attribute( SyntaxFactory.IdentifierName(hookType), @@ -184,10 +187,30 @@ private AttributeArgumentListSyntax ConvertOwnerArguments(AttributeArgumentListS ) ) ); - return SyntaxFactory.AttributeList(SyntaxFactory.SingletonSeparatedList(newAttribute)); + attributes.Add(newAttribute); + // Don't return early - continue processing other attributes! + } + else + { + // For non-ClassInitialize/ClassCleanup, use base conversion logic + var converted = ConvertAttribute(attribute); + if (converted != null) + { + attributes.Add(converted); + } } } - + + // If we processed class lifecycle attributes, return our combined list + if (hasClassLifecycleAttribute) + { + return attributes.Count > 0 + ? node.WithAttributes(SyntaxFactory.SeparatedList(attributes)) + .WithLeadingTrivia(node.GetLeadingTrivia()) + .WithTrailingTrivia(node.GetTrailingTrivia()) + : null; + } + return base.VisitAttributeList(node); } } diff --git a/TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs b/TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs index bcff65b4eb..a731f3dd84 100644 --- a/TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs +++ b/TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs @@ -70,7 +70,7 @@ protected override bool IsFrameworkAttribute(string attributeName) "Test" or "Theory" or "TestCase" or "TestCaseSource" or "SetUp" or "TearDown" or "OneTimeSetUp" or "OneTimeTearDown" or "TestFixture" or "Category" or "Ignore" or "Explicit" or "Apartment" or - "Platform" or "Description" or + "Platform" or "Description" or "Author" or // Parallelization attributes "Parallelizable" or "NonParallelizable" or // Repeat attribute (same in TUnit) @@ -145,9 +145,45 @@ protected override bool IsFrameworkAttribute(string attributeName) return ConvertPlatformAttribute(attribute); } + // [Description("...")] -> [Property("Description", "...")] + if (attributeName == "Description") + { + return ConvertToPropertyAttribute("Description", attribute); + } + + // [Author("...")] -> [Property("Author", "...")] + if (attributeName == "Author") + { + return ConvertToPropertyAttribute("Author", attribute); + } + return base.ConvertAttribute(attribute); } + private AttributeSyntax? ConvertToPropertyAttribute(string propertyName, AttributeSyntax attribute) + { + // Get the value from the attribute argument + if (attribute.ArgumentList == null || attribute.ArgumentList.Arguments.Count == 0) + { + return null; // No value, remove the attribute + } + + var valueExpression = attribute.ArgumentList.Arguments[0].Expression; + + // Create [Property("propertyName", value)] + return SyntaxFactory.Attribute( + SyntaxFactory.IdentifierName("Property"), + SyntaxFactory.AttributeArgumentList( + SyntaxFactory.SeparatedList(new[] + { + SyntaxFactory.AttributeArgument( + SyntaxFactory.LiteralExpression( + SyntaxKind.StringLiteralExpression, + SyntaxFactory.Literal(propertyName))), + SyntaxFactory.AttributeArgument(valueExpression) + }))); + } + private AttributeSyntax? ConvertApartmentAttribute(AttributeSyntax attribute) { // Check if the argument is ApartmentState.STA @@ -915,15 +951,17 @@ private ExpressionSyntax ConvertConstraintToTUnitWithMessage(ExpressionSyntax ac }; } - // Handle Does.StartWith, Does.EndWith, Does.Match, Contains.Substring + // Handle Does.StartWith, Does.EndWith, Does.Contain, Does.Match, Contains.Substring, Contains.Item if (memberAccess.Expression is IdentifierNameSyntax { Identifier.Text: "Does" or "Contains" }) { return methodName switch { "StartWith" => CreateTUnitAssertionWithMessage("StartsWith", actualValue, message, constraint.ArgumentList.Arguments.ToArray()), "EndWith" => CreateTUnitAssertionWithMessage("EndsWith", actualValue, message, constraint.ArgumentList.Arguments.ToArray()), + "Contain" => CreateTUnitAssertionWithMessage("Contains", actualValue, message, constraint.ArgumentList.Arguments.ToArray()), "Match" => CreateTUnitAssertionWithMessage("Matches", actualValue, message, constraint.ArgumentList.Arguments.ToArray()), "Substring" => CreateTUnitAssertionWithMessage("Contains", actualValue, message, constraint.ArgumentList.Arguments.ToArray()), + "Item" => CreateTUnitAssertionWithMessage("Contains", actualValue, message, constraint.ArgumentList.Arguments.ToArray()), _ => CreateTUnitAssertionWithMessage("IsEqualTo", actualValue, message, SyntaxFactory.Argument(constraint)) }; } @@ -1115,15 +1153,17 @@ private ExpressionSyntax ConvertConstraintToTUnit(ExpressionSyntax actualValue, }; } - // Handle Does.StartWith, Does.EndWith, Does.Match, Contains.Substring + // Handle Does.StartWith, Does.EndWith, Does.Contain, Does.Match, Contains.Substring, Contains.Item if (memberAccess.Expression is IdentifierNameSyntax { Identifier.Text: "Does" or "Contains" }) { return methodName switch { "StartWith" => CreateTUnitAssertion("StartsWith", actualValue, constraint.ArgumentList.Arguments.ToArray()), "EndWith" => CreateTUnitAssertion("EndsWith", actualValue, constraint.ArgumentList.Arguments.ToArray()), + "Contain" => CreateTUnitAssertion("Contains", actualValue, constraint.ArgumentList.Arguments.ToArray()), "Match" => CreateTUnitAssertion("Matches", actualValue, constraint.ArgumentList.Arguments.ToArray()), "Substring" => CreateTUnitAssertion("Contains", actualValue, constraint.ArgumentList.Arguments.ToArray()), + "Item" => CreateTUnitAssertion("Contains", actualValue, constraint.ArgumentList.Arguments.ToArray()), _ => CreateTUnitAssertion("IsEqualTo", actualValue, SyntaxFactory.Argument(constraint)) }; } @@ -2037,13 +2077,46 @@ public class NUnitLifecycleRewriter : CSharpSyntaxRewriter { // Lifecycle methods are handled by attribute conversion // Just ensure they're public and have correct signature + // NOTE: Check for ORIGINAL NUnit attribute names since this runs BEFORE attribute conversion var hasLifecycleAttribute = node.AttributeLists .SelectMany(al => al.Attributes) - .Any(a => a.Name.ToString() is "Before" or "After"); + .Select(a => MigrationHelpers.GetAttributeName(a)) + .Any(name => name is "SetUp" or "TearDown" or "OneTimeSetUp" or "OneTimeTearDown"); if (hasLifecycleAttribute && !node.Modifiers.Any(SyntaxKind.PublicKeyword)) { - return node.AddModifiers(SyntaxFactory.Token(SyntaxKind.PublicKeyword)); + // Remove existing access modifiers (private, protected, internal) before adding public + var accessModifierKinds = new[] + { + SyntaxKind.PrivateKeyword, + SyntaxKind.ProtectedKeyword, + SyntaxKind.InternalKeyword + }; + + var newModifiers = node.Modifiers + .Where(m => !accessModifierKinds.Contains(m.Kind())) + .ToList(); + + // Preserve leading trivia from the first modifier, or get indentation from return type + var leadingTrivia = node.Modifiers.Any() + ? node.Modifiers.First().LeadingTrivia + : node.ReturnType.GetLeadingTrivia(); + + var publicToken = SyntaxFactory.Token(SyntaxKind.PublicKeyword) + .WithLeadingTrivia(leadingTrivia) + .WithTrailingTrivia(SyntaxFactory.Space); + + newModifiers.Insert(0, publicToken); + + // If there were no modifiers, we need to strip the leading trivia from the return type + // since it's now on the public keyword + var newNode = node.WithModifiers(SyntaxFactory.TokenList(newModifiers)); + if (!node.Modifiers.Any()) + { + newNode = newNode.WithReturnType(newNode.ReturnType.WithLeadingTrivia()); + } + + return newNode; } return base.VisitMethodDeclaration(node); diff --git a/TUnit.Analyzers.Tests/MSTestMigrationAnalyzerTests.cs b/TUnit.Analyzers.Tests/MSTestMigrationAnalyzerTests.cs index 8140ac97c4..143a1a1822 100644 --- a/TUnit.Analyzers.Tests/MSTestMigrationAnalyzerTests.cs +++ b/TUnit.Analyzers.Tests/MSTestMigrationAnalyzerTests.cs @@ -1335,6 +1335,440 @@ public static void ClassTeardown() ); } + /// + /// Tests that ClassInitialize/ClassCleanup with sibling attributes preserve all attributes. + /// Bug fix: Early return in VisitAttributeList was losing sibling attributes. + /// + [Test] + public async Task MSTest_ClassLifecycle_With_Sibling_Attributes_Preserved() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + {|#0:[TestClass]|} + public class TestClass + { + [ClassInitialize, Description("Class setup")] + public static void ClassSetup(TestContext context) + { + } + + [ClassCleanup, Description("Class teardown")] + public static void ClassTeardown() + { + } + + [TestInitialize, Description("Test setup")] + public void TestSetup() + { + } + + [TestCleanup, Description("Test teardown")] + public void TestTeardown() + { + } + + [TestMethod] + public void MyTest() + { + } + } + """, + Verifier.Diagnostic(Rules.MSTestMigration).WithLocation(0), + """ + public class TestClass + { + [Before(HookType.Class), Description("Class setup")] + public static void ClassSetup() + { + } + + [After(HookType.Class), Description("Class teardown")] + public static void ClassTeardown() + { + } + + [Before(HookType.Test), Description("Test setup")] + public void TestSetup() + { + } + + [After(HookType.Test), Description("Test teardown")] + public void TestTeardown() + { + } + + [Test] + public void MyTest() + { + } + } + """, + ConfigureMSTestTest + ); + } + + /// + /// Tests that multiple classes in a single file are all processed correctly. + /// Bug fix: Trivia cleanup was failing on second class due to stale node references. + /// + [Test] + public async Task MSTest_Multiple_Classes_In_File_All_Converted() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + {|#0:[TestClass]|} + public class FirstTestClass + { + [TestMethod] + public void FirstTest() + { + Assert.IsTrue(true); + } + } + + [TestClass] + public class SecondTestClass + { + [TestMethod] + public void SecondTest() + { + Assert.IsFalse(false); + } + } + + [TestClass] + public class ThirdTestClass + { + [TestInitialize] + public void Setup() + { + } + + [TestMethod] + public void ThirdTest() + { + Assert.AreEqual(1, 1); + } + } + """, + Verifier.Diagnostic(Rules.MSTestMigration).WithLocation(0), + """ + using System.Threading.Tasks; + + public class FirstTestClass + { + [Test] + public async Task FirstTest() + { + await Assert.That(true).IsTrue(); + } + } + public class SecondTestClass + { + [Test] + public async Task SecondTest() + { + await Assert.That(false).IsFalse(); + } + } + public class ThirdTestClass + { + [Before(HookType.Test)] + public void Setup() + { + } + + [Test] + public async Task ThirdTest() + { + await Assert.That(1).IsEqualTo(1); + } + } + """, + ConfigureMSTestTest + ); + } + + [Test] + public async Task MSTest_Comprehensive_Kitchen_Sink_Migration() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + using System; + using System.Collections.Generic; + + {|#0:[TestClass]|} + [Description("Comprehensive MSTest fixture for migration")] + [TestCategory("Migration")] + public class ComprehensiveTests + { + private List _log; + private int _counter; + + [ClassInitialize] + public static void ClassSetup(TestContext context) + { + // Class-level setup + } + + [ClassCleanup] + public static void ClassTeardown() + { + // Class-level teardown + } + + [TestInitialize] + public void TestSetup() + { + _log = new List(); + _counter = 0; + } + + [TestCleanup] + public void TestTeardown() + { + _log.Clear(); + _counter = -1; + } + + [TestMethod] + [Description("Simple test without parameters")] + public void SimpleTest() + { + Assert.IsTrue(true); + Assert.IsFalse(false); + } + + [TestMethod] + [TestCategory("Math")] + public void MathTest() + { + var result = 2 + 2; + Assert.AreEqual(4, result); + Assert.AreNotEqual(5, result); + } + + [TestMethod] + [DataRow(1, 2, 3)] + [DataRow(5, 5, 10)] + [DataRow(-1, 1, 0)] + [Description("Parameterized addition test")] + public void AdditionTest(int a, int b, int expected) + { + var result = a + b; + Assert.AreEqual(expected, result); + } + + [TestMethod] + [DataRow("hello", 5)] + [DataRow("world", 5)] + public void StringLengthTest(string input, int expectedLength) + { + Assert.AreEqual(expectedLength, input.Length); + Assert.IsNotNull(input); + } + + [TestMethod] + [TestCategory("Null")] + public void NullAndTypeTests() + { + object obj = "test"; + object nullObj = null; + + Assert.IsNotNull(obj); + Assert.IsNull(nullObj); + Assert.IsInstanceOfType(obj, typeof(string)); + } + + [TestMethod] + public void CollectionTests() + { + var list = new List { 1, 2, 3 }; + var empty = new List(); + + CollectionAssert.Contains(list, 2); + CollectionAssert.DoesNotContain(list, 4); + } + + [TestMethod] + public void StringTests() + { + var str = "Hello World"; + + StringAssert.StartsWith(str, "Hello"); + StringAssert.EndsWith(str, "World"); + StringAssert.Contains(str, "lo Wo"); + } + + [TestMethod] + [Ignore("This test is temporarily disabled")] + public void IgnoredTest() + { + Assert.Fail("Should not run"); + } + } + + [TestClass] + [TestCategory("Secondary")] + public class SecondaryTests + { + [TestMethod] + public void AnotherTest() + { + Assert.AreEqual(1, 1); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public void BooleanTest(bool value) + { + Assert.AreEqual(value, value); + } + } + """, + Verifier.Diagnostic(Rules.MSTestMigration).WithLocation(0), + """ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + + [Description("Comprehensive MSTest fixture for migration")] + [Property("Category", "Migration")] + public class ComprehensiveTests + { + private List _log; + private int _counter; + + [Before(HookType.Class)] + public static void ClassSetup() + { + // Class-level setup + } + + [After(HookType.Class)] + public static void ClassTeardown() + { + // Class-level teardown + } + + [Before(HookType.Test)] + public void TestSetup() + { + _log = new List(); + _counter = 0; + } + + [After(HookType.Test)] + public void TestTeardown() + { + _log.Clear(); + _counter = -1; + } + + [Test] + [Description("Simple test without parameters")] + public async Task SimpleTest() + { + await Assert.That(true).IsTrue(); + await Assert.That(false).IsFalse(); + } + + [Test] + [Property("Category", "Math")] + public async Task MathTest() + { + var result = 2 + 2; + await Assert.That(result).IsEqualTo(4); + await Assert.That(result).IsNotEqualTo(5); + } + + [Test] + [Arguments(1, 2, 3)] + [Arguments(5, 5, 10)] + [Arguments(-1, 1, 0)] + [Description("Parameterized addition test")] + public async Task AdditionTest(int a, int b, int expected) + { + var result = a + b; + await Assert.That(result).IsEqualTo(expected); + } + + [Test] + [Arguments("hello", 5)] + [Arguments("world", 5)] + public async Task StringLengthTest(string input, int expectedLength) + { + await Assert.That(input.Length).IsEqualTo(expectedLength); + await Assert.That(input).IsNotNull(); + } + + [Test] + [Property("Category", "Null")] + public async Task NullAndTypeTests() + { + object obj = "test"; + object nullObj = null; + + await Assert.That(obj).IsNotNull(); + await Assert.That(nullObj).IsNull(); + await Assert.That(obj).IsAssignableTo(typeof(string)); + } + + [Test] + public async Task CollectionTests() + { + var list = new List { 1, 2, 3 }; + var empty = new List(); + + await Assert.That(list).Contains(2); + await Assert.That(list).DoesNotContain(4); + } + + [Test] + public async Task StringTests() + { + var str = "Hello World"; + + await Assert.That(str).StartsWith("Hello"); + await Assert.That(str).EndsWith("World"); + await Assert.That(str).Contains("lo Wo"); + } + + [Test] + [Ignore("This test is temporarily disabled")] + public async Task IgnoredTest() + { + await Assert.Fail("Should not run"); + } + } + [Property("Category", "Secondary")] + public class SecondaryTests + { + [Test] + public async Task AnotherTest() + { + await Assert.That(1).IsEqualTo(1); + } + + [Test] + [Arguments(true)] + [Arguments(false)] + public async Task BooleanTest(bool value) + { + await Assert.That(value).IsEqualTo(value); + } + } + """, + ConfigureMSTestTest + ); + } + private static void ConfigureMSTestTest(Verifier.Test test) { test.TestState.AdditionalReferences.Add(typeof(TestMethodAttribute).Assembly); diff --git a/TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs b/TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs index addfd328c1..5ee49ee67f 100644 --- a/TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs +++ b/TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs @@ -2737,7 +2737,7 @@ public void TestMethod() } [Test] - public async Task NUnit_Description_Attribute_Removed() + public async Task NUnit_Description_Attribute_Converted_To_Property() { await CodeFixer.VerifyCodeFixAsync( """ @@ -2758,6 +2758,7 @@ public void TestMethod() public class MyClass { [Test] + [Property("Description", "This is a test description")] public void TestMethod() { } @@ -4573,6 +4574,623 @@ public void ClassTeardown() ); } + /// + /// Tests that hook attributes with sibling attributes preserve all attributes. + /// Bug fix: Early return in VisitAttributeList was losing sibling attributes. + /// Note: [Description] is currently removed; [Author] is kept inline (current behavior). + /// + [Test] + public async Task NUnit_Hook_With_Sibling_Attributes_Preserved() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using NUnit.Framework; + + {|#0:[TestFixture]|} + public class TestClass + { + [SetUp, Category("Unit")] + public void Setup() + { + // Setup code + } + + [TearDown, Category("Unit")] + public void Teardown() + { + // Teardown code + } + + [OneTimeSetUp, Explicit] + public void ClassSetup() + { + // Class setup + } + + [Test] + public void MyTest() + { + } + } + """, + Verifier.Diagnostic(Rules.NUnitMigration).WithLocation(0), + """ + public class TestClass + { + [Before(HookType.Test), Category("Unit")] + public void Setup() + { + // Setup code + } + + [After(HookType.Test), Category("Unit")] + public void Teardown() + { + // Teardown code + } + + [Before(HookType.Class), Explicit] + public void ClassSetup() + { + // Class setup + } + + [Test] + public void MyTest() + { + } + } + """, + ConfigureNUnitTest + ); + } + + /// + /// Tests that multiple classes in a single file are all processed correctly. + /// Bug fix: Trivia cleanup was failing on second class due to stale node references. + /// + [Test] + public async Task NUnit_Multiple_Classes_In_File_All_Converted() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using NUnit.Framework; + + {|#0:[TestFixture]|} + public class FirstTestClass + { + [Test] + public void FirstTest() + { + Assert.That(true, Is.True); + } + } + + [TestFixture] + public class SecondTestClass + { + [Test] + public void SecondTest() + { + Assert.That(false, Is.False); + } + } + + [TestFixture] + public class ThirdTestClass + { + [SetUp] + public void Setup() + { + } + + [Test] + public void ThirdTest() + { + Assert.That(1, Is.EqualTo(1)); + } + } + """, + Verifier.Diagnostic(Rules.NUnitMigration).WithLocation(0), + """ + using System.Threading.Tasks; + + public class FirstTestClass + { + [Test] + public async Task FirstTest() + { + await Assert.That(true).IsTrue(); + } + } + public class SecondTestClass + { + [Test] + public async Task SecondTest() + { + await Assert.That(false).IsFalse(); + } + } + public class ThirdTestClass + { + [Before(HookType.Test)] + public void Setup() + { + } + + [Test] + public async Task ThirdTest() + { + await Assert.That(1).IsEqualTo(1); + } + } + """, + ConfigureNUnitTest + ); + } + + /// + /// Tests that non-public lifecycle methods get public modifier added. + /// Bug fix: Lifecycle rewriter was checking for converted attribute names instead of original NUnit names. + /// + [Test] + public async Task NUnit_NonPublic_Lifecycle_Methods_Made_Public() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using NUnit.Framework; + + {|#0:[TestFixture]|} + public class TestClass + { + [SetUp] + void PrivateSetup() + { + } + + [TearDown] + internal void InternalTeardown() + { + } + + [OneTimeSetUp] + protected void ProtectedClassSetup() + { + } + + [OneTimeTearDown] + private void PrivateClassTeardown() + { + } + + [Test] + public void MyTest() + { + } + } + """, + Verifier.Diagnostic(Rules.NUnitMigration).WithLocation(0), + """ + public class TestClass + { + [Before(HookType.Test)] + public void PrivateSetup() + { + } + + [After(HookType.Test)] + public void InternalTeardown() + { + } + + [Before(HookType.Class)] + public void ProtectedClassSetup() + { + } + + [After(HookType.Class)] + public void PrivateClassTeardown() + { + } + + [Test] + public void MyTest() + { + } + } + """, + ConfigureNUnitTest + ); + } + + /// + /// Tests that [Description] and [Author] attributes are converted to [Property] attributes. + /// + [Test] + public async Task NUnit_Description_And_Author_Converted_To_Property() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using NUnit.Framework; + + {|#0:[TestFixture]|} + [Description("Test fixture description")] + public class TestClass + { + [Test] + [Description("Test method description")] + [Author("Test Author")] + public void MyTest() + { + } + + [Test] + [Author("Another Author")] + public void AnotherTest() + { + } + } + """, + Verifier.Diagnostic(Rules.NUnitMigration).WithLocation(0), + """ + [Property("Description", "Test fixture description")] + public class TestClass + { + [Test] + [Property("Description", "Test method description")] + [Property("Author", "Test Author")] + public void MyTest() + { + } + + [Test] + [Property("Author", "Another Author")] + public void AnotherTest() + { + } + } + """, + ConfigureNUnitTest + ); + } + + /// + /// Comprehensive "kitchen sink" test that exercises all NUnit migration code paths: + /// - Class-level lifecycle (OneTimeSetUp, OneTimeTearDown) + /// - Test-level lifecycle (SetUp, TearDown) + /// - Multiple test methods with various attributes + /// - Data-driven tests (TestCase with arguments) + /// - Metadata attributes (Description, Author, Category) + /// - Explicit tests + /// - Various assertion types + /// - Non-public lifecycle methods + /// - Multiple attributes on same method + /// + [Test] + public async Task NUnit_Comprehensive_Kitchen_Sink_Migration() + { + await CodeFixer.VerifyCodeFixAsync( + """ + using NUnit.Framework; + using System; + using System.Collections.Generic; + + {|#0:[TestFixture]|} + [Description("Comprehensive test fixture for migration testing")] + [Category("Migration")] + [Author("Test Developer")] + public class ComprehensiveTests + { + private List _log; + private int _counter; + + [OneTimeSetUp] + public void ClassSetup() + { + _log = new List(); + } + + [OneTimeTearDown] + public void ClassTeardown() + { + _log.Clear(); + } + + [SetUp] + public void TestSetup() + { + _counter = 0; + } + + [TearDown, Category("Cleanup")] + public void TestTeardown() + { + _counter = -1; + } + + [Test] + [Description("Simple test without parameters")] + public void SimpleTest() + { + Assert.That(true, Is.True); + Assert.That(false, Is.False); + } + + [Test] + [Category("Math")] + [Author("Math Developer")] + public void MathTest() + { + var result = 2 + 2; + Assert.That(result, Is.EqualTo(4)); + Assert.That(result, Is.Not.EqualTo(5)); + Assert.That(result, Is.GreaterThan(3)); + Assert.That(result, Is.LessThan(5)); + } + + [TestCase(1, 2, 3)] + [TestCase(5, 5, 10)] + [TestCase(-1, 1, 0)] + [Description("Parameterized addition test")] + public void AdditionTest(int a, int b, int expected) + { + var result = a + b; + Assert.That(result, Is.EqualTo(expected)); + } + + [TestCase("hello", 5)] + [TestCase("world", 5)] + [TestCase("", 0)] + public void StringLengthTest(string input, int expectedLength) + { + Assert.That(input.Length, Is.EqualTo(expectedLength)); + Assert.That(input, Is.Not.Null); + } + + [Test] + [Explicit("This test is slow")] + public void SlowTest() + { + Assert.That(true, Is.True); + } + + [Test] + [Category("Null")] + public void NullAndTypeTests() + { + object obj = "test"; + object nullObj = null; + + Assert.That(obj, Is.Not.Null); + Assert.That(nullObj, Is.Null); + Assert.That(obj, Is.TypeOf()); + Assert.That(obj, Is.InstanceOf()); + } + + [Test] + public void CollectionTests() + { + var list = new List { 1, 2, 3 }; + var empty = new List(); + + Assert.That(list, Is.Not.Empty); + Assert.That(empty, Is.Empty); + Assert.That(list, Has.Count.EqualTo(3)); + Assert.That(list, Contains.Item(2)); + } + + [Test] + public void StringTests() + { + var str = "Hello World"; + + Assert.That(str, Does.StartWith("Hello")); + Assert.That(str, Does.EndWith("World")); + Assert.That(str, Does.Contain("lo Wo")); + Assert.That(str, Is.Not.Empty); + } + + [Test] + [Ignore("This test is temporarily disabled")] + public void IgnoredTest() + { + Assert.Fail("Should not run"); + } + + [SetUp] + void PrivateSetup() + { + // Second setup - non-public + } + + [TearDown] + internal void InternalTeardown() + { + // Internal teardown + } + } + + [TestFixture] + [Category("Secondary")] + public class SecondaryTests + { + [Test] + public void AnotherTest() + { + Assert.That(1, Is.EqualTo(1)); + } + + [TestCase(true)] + [TestCase(false)] + public void BooleanTest(bool value) + { + Assert.That(value, Is.EqualTo(value)); + } + } + """, + Verifier.Diagnostic(Rules.NUnitMigration).WithLocation(0), + """ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + + [Property("Description", "Comprehensive test fixture for migration testing")] + [Category("Migration")] + [Property("Author", "Test Developer")] + public class ComprehensiveTests + { + private List _log; + private int _counter; + + [Before(HookType.Class)] + public void ClassSetup() + { + _log = new List(); + } + + [After(HookType.Class)] + public void ClassTeardown() + { + _log.Clear(); + } + + [Before(HookType.Test)] + public void TestSetup() + { + _counter = 0; + } + + [After(HookType.Test), Category("Cleanup")] + public void TestTeardown() + { + _counter = -1; + } + + [Test] + [Property("Description", "Simple test without parameters")] + public async Task SimpleTest() + { + await Assert.That(true).IsTrue(); + await Assert.That(false).IsFalse(); + } + + [Test] + [Category("Math")] + [Property("Author", "Math Developer")] + public async Task MathTest() + { + var result = 2 + 2; + await Assert.That(result).IsEqualTo(4); + await Assert.That(result).IsNotEqualTo(5); + await Assert.That(result).IsGreaterThan(3); + await Assert.That(result).IsLessThan(5); + } + + [Test] + [Arguments(1, 2, 3)] + [Arguments(5, 5, 10)] + [Arguments(-1, 1, 0)] + [Property("Description", "Parameterized addition test")] + public async Task AdditionTest(int a, int b, int expected) + { + var result = a + b; + await Assert.That(result).IsEqualTo(expected); + } + + [Test] + [Arguments("hello", 5)] + [Arguments("world", 5)] + [Arguments("", 0)] + public async Task StringLengthTest(string input, int expectedLength) + { + await Assert.That(input.Length).IsEqualTo(expectedLength); + await Assert.That(input).IsNotNull(); + } + + [Test] + [Explicit("This test is slow")] + public async Task SlowTest() + { + await Assert.That(true).IsTrue(); + } + + [Test] + [Category("Null")] + public async Task NullAndTypeTests() + { + object obj = "test"; + object nullObj = null; + + await Assert.That(obj).IsNotNull(); + await Assert.That(nullObj).IsNull(); + await Assert.That(obj).IsTypeOf(); + await Assert.That(obj).IsAssignableTo(); + } + + [Test] + public async Task CollectionTests() + { + var list = new List { 1, 2, 3 }; + var empty = new List(); + + await Assert.That(list).IsNotEmpty(); + await Assert.That(empty).IsEmpty(); + await Assert.That(list).Count().IsEqualTo(3); + await Assert.That(list).Contains(2); + } + + [Test] + public async Task StringTests() + { + var str = "Hello World"; + + await Assert.That(str).StartsWith("Hello"); + await Assert.That(str).EndsWith("World"); + await Assert.That(str).Contains("lo Wo"); + await Assert.That(str).IsNotEmpty(); + } + + [Test] + [Skip("This test is temporarily disabled")] + public void IgnoredTest() + { + Fail.Test("Should not run"); + } + + [Before(HookType.Test)] + public void PrivateSetup() + { + // Second setup - non-public + } + + [After(HookType.Test)] + public void InternalTeardown() + { + // Internal teardown + } + } + [Category("Secondary")] + public class SecondaryTests + { + [Test] + public async Task AnotherTest() + { + await Assert.That(1).IsEqualTo(1); + } + + [Test] + [Arguments(true)] + [Arguments(false)] + public async Task BooleanTest(bool value) + { + await Assert.That(value).IsEqualTo(value); + } + } + """, + ConfigureNUnitTest + ); + } + private static void ConfigureNUnitTest(Verifier.Test test) { test.TestState.AdditionalReferences.Add(typeof(NUnit.Framework.TestAttribute).Assembly); diff --git a/TUnit.Analyzers.Tests/XUnitMigrationAnalyzerTests.cs b/TUnit.Analyzers.Tests/XUnitMigrationAnalyzerTests.cs index 8e777ea0f3..3d616e0ce6 100644 --- a/TUnit.Analyzers.Tests/XUnitMigrationAnalyzerTests.cs +++ b/TUnit.Analyzers.Tests/XUnitMigrationAnalyzerTests.cs @@ -1511,6 +1511,233 @@ public async Task MyTest() ); } + [Test] + public async Task XUnit_Comprehensive_Kitchen_Sink_Migration() + { + await CodeFixer.VerifyCodeFixAsync( + """ + {|#0:using Xunit; + using System; + using System.Collections.Generic; + + public class ComprehensiveTests + { + private List _log; + private int _counter; + + public ComprehensiveTests() + { + // Constructor acts as setup + _log = new List(); + _counter = 0; + } + + [Fact] + public void SimpleTest() + { + Assert.True(true); + Assert.False(false); + } + + [Fact] + public void MathTest() + { + var result = 2 + 2; + Assert.Equal(4, result); + Assert.NotEqual(5, result); + } + + [Theory] + [InlineData(1, 2, 3)] + [InlineData(5, 5, 10)] + [InlineData(-1, 1, 0)] + public void AdditionTest(int a, int b, int expected) + { + var result = a + b; + Assert.Equal(expected, result); + } + + [Theory] + [InlineData("hello", 5)] + [InlineData("world", 5)] + public void StringLengthTest(string input, int expectedLength) + { + Assert.Equal(expectedLength, input.Length); + Assert.NotNull(input); + } + + [Fact] + public void NullAndTypeTests() + { + object obj = "test"; + object nullObj = null; + + Assert.NotNull(obj); + Assert.Null(nullObj); + Assert.IsType(obj); + Assert.IsAssignableFrom(obj); + } + + [Fact] + public void CollectionTests() + { + var list = new List { 1, 2, 3 }; + var empty = new List(); + + Assert.NotEmpty(list); + Assert.Empty(empty); + Assert.Contains(2, list); + Assert.DoesNotContain(4, list); + } + + [Fact] + public void StringTests() + { + var str = "Hello World"; + + Assert.StartsWith("Hello", str); + Assert.EndsWith("World", str); + Assert.Contains("lo Wo", str); + } + + [Fact(Skip = "This test is temporarily disabled")] + public void SkippedTest() + { + Assert.True(false, "Should not run"); + } + } + + public class SecondaryTests + { + [Fact] + public void AnotherTest() + { + Assert.Equal(1, 1); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void BooleanTest(bool value) + { + Assert.Equal(value, value); + } + }|} + """, + Verifier.Diagnostic(Rules.XunitMigration).WithLocation(0), + """ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + + public class ComprehensiveTests + { + private List _log; + private int _counter; + + public ComprehensiveTests() + { + // Constructor acts as setup + _log = new List(); + _counter = 0; + } + + [Test] + public async Task SimpleTest() + { + await Assert.That(true).IsTrue(); + await Assert.That(false).IsFalse(); + } + + [Test] + public async Task MathTest() + { + var result = 2 + 2; + await Assert.That(result).IsEqualTo(4); + await Assert.That(result).IsNotEqualTo(5); + } + + [Test] + [Arguments(1, 2, 3)] + [Arguments(5, 5, 10)] + [Arguments(-1, 1, 0)] + public async Task AdditionTest(int a, int b, int expected) + { + var result = a + b; + await Assert.That(result).IsEqualTo(expected); + } + + [Test] + [Arguments("hello", 5)] + [Arguments("world", 5)] + public async Task StringLengthTest(string input, int expectedLength) + { + await Assert.That(input.Length).IsEqualTo(expectedLength); + await Assert.That(input).IsNotNull(); + } + + [Test] + public async Task NullAndTypeTests() + { + object obj = "test"; + object nullObj = null; + + await Assert.That(obj).IsNotNull(); + await Assert.That(nullObj).IsNull(); + await Assert.That(obj).IsTypeOf(); + await Assert.That(obj).IsAssignableTo(); + } + + [Test] + public async Task CollectionTests() + { + var list = new List { 1, 2, 3 }; + var empty = new List(); + + await Assert.That(list).IsNotEmpty(); + await Assert.That(empty).IsEmpty(); + await Assert.That(list).Contains(2); + await Assert.That(list).DoesNotContain(4); + } + + [Test] + public async Task StringTests() + { + var str = "Hello World"; + + await Assert.That(str).StartsWith("Hello"); + await Assert.That(str).EndsWith("World"); + await Assert.That(str).Contains("lo Wo"); + } + + [Test, Skip("This test is temporarily disabled")] + public async Task SkippedTest() + { + await Assert.That(false).IsTrue().Because("Should not run"); + } + } + + public class SecondaryTests + { + [Test] + public async Task AnotherTest() + { + await Assert.That(1).IsEqualTo(1); + } + + [Test] + [Arguments(true)] + [Arguments(false)] + public async Task BooleanTest(bool value) + { + await Assert.That(value).IsEqualTo(value); + } + } + """, + ConfigureXUnitTest + ); + } + private static void ConfigureXUnitTest(Verifier.Test test) { var globalUsings = ("GlobalUsings.cs", SourceText.From("global using Xunit;")); diff --git a/TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs b/TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs index 87fb00e95c..66250badca 100644 --- a/TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs +++ b/TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs @@ -136,7 +136,8 @@ public static bool ShouldRemoveAttribute(string attributeName, string framework) // Note: Parallelizable is NOT listed here because it needs special handling in the code fixer // for ParallelScope.None -> [NotInParallel]. ConvertAttribute handles all cases. // Note: Platform is NOT listed here because it gets converted to RunOn/ExcludeOn. - "NUnit" => attributeName is "TestFixture" or "Description", + // Note: Description is NOT listed here because it gets converted to Property("Description", ...). + "NUnit" => attributeName is "TestFixture", "MSTest" => attributeName is "TestClass", _ => false };