feat: Migrated to TUnit #392
Conversation
samtrion
added
state:ready for merge
WalkthroughThe pull request migrates the test suite from xUnit to TUnit framework, updating test-related dependencies, refactoring test methods to async patterns with TUnit assertions, removing net6.0 target framework, and extending EditorConfig to recognize slnx files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #392 +/- ##
============================================
- Coverage 100.00% 77.77% -22.23%
============================================
Files 12 11 -1
Lines 68 27 -41
Branches 14 2 -12
============================================
- Hits 68 21 -47
- Misses 0 5 +5
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfLessThanOrEqual.cs (1)
35-47: Assertion is not testing the actual behavior.The test should verify that
Argument.ThrowIfLessThanOrEqualreturns the argument when it's greater than the minimum. However, line 46 asserts thatminimum.IsEqualTo(1), which only checks the local variable's value rather than testing the method's return value or behavior.Apply this diff to properly test the method:
- [Test] - public async Task ThrowIfLessThanOrEqual_WhenArgumentIsGreaterThanMinimum_ReturnsArgument() - { - // Arrange - var argument = 2; - var minimum = 1; - - // Act - Argument.ThrowIfLessThanOrEqual(argument, minimum); - - // Assert - _ = await Assert.That(minimum).IsEqualTo(1); - } + [Test] + public async Task ThrowIfLessThanOrEqual_WhenArgumentIsGreaterThanMinimum_ReturnsArgument() + { + // Arrange + var argument = 2; + var minimum = 1; + + // Act + var result = Argument.ThrowIfLessThanOrEqual(argument, minimum); + + // Assert + _ = await Assert.That(result).IsEqualTo(argument); + }tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNotEqual.cs (1)
21-33: Assertion is not testing the actual behavior.Similar to the previous file, this test should verify that
Argument.ThrowIfNotEqualreturns the argument when it equals the maximum. Line 32 only checks the localmaximumvariable instead of verifying the method's return value.Apply this diff:
- [Test] - public async Task ThrowIfNotEqual_WhenArgumentIsEqualToMaximum_ReturnsArgument() - { - // Arrange - var argument = 1; - var maximum = 1; - - // Act - Argument.ThrowIfNotEqual(argument, maximum); - - // Assert - _ = await Assert.That(maximum).IsEqualTo(1); - } + [Test] + public async Task ThrowIfNotEqual_WhenArgumentIsEqualToMaximum_ReturnsArgument() + { + // Arrange + var argument = 1; + var maximum = 1; + + // Act + var result = Argument.ThrowIfNotEqual(argument, maximum); + + // Assert + _ = await Assert.That(result).IsEqualTo(argument); + }tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfGreaterThan.cs (1)
22-34: Assertion is not testing the actual behavior.The test should verify that
Argument.ThrowIfGreaterThanreturns the argument when it equals the maximum. Line 33 only checks the localmaximumvariable rather than the method's return value.Apply this diff:
- [Test] - public async Task ThrowIfGreaterThan_WhenArgumentIsEqualToMaximum_ReturnsArgument() - { - // Arrange - var argument = 1; - var maximum = 1; - - // Act - Argument.ThrowIfGreaterThan(argument, maximum); - - // Assert - _ = await Assert.That(maximum).IsEqualTo(1); - } + [Test] + public async Task ThrowIfGreaterThan_WhenArgumentIsEqualToMaximum_ReturnsArgument() + { + // Arrange + var argument = 1; + var maximum = 1; + + // Act + var result = Argument.ThrowIfGreaterThan(argument, maximum); + + // Assert + _ = await Assert.That(result).IsEqualTo(argument); + }tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfLessThan.cs (1)
21-33: Assertion is not testing the actual behavior.The test should verify that
Argument.ThrowIfLessThanreturns the argument when it equals the minimum. Line 32 only checks the localminimumvariable instead of the method's return value.Apply this diff:
- [Test] - public async Task ThrowIfLessThan_WhenArgumentIsEqualToMinimum_ReturnsArgument() - { - // Arrange - var argument = 1; - var minimum = 1; - - // Act - Argument.ThrowIfLessThan(argument, minimum); - - // Assert - _ = await Assert.That(minimum).IsEqualTo(1); - } + [Test] + public async Task ThrowIfLessThan_WhenArgumentIsEqualToMinimum_ReturnsArgument() + { + // Arrange + var argument = 1; + var minimum = 1; + + // Act + var result = Argument.ThrowIfLessThan(argument, minimum); + + // Assert + _ = await Assert.That(result).IsEqualTo(argument); + }tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNull.cs (1)
20-31: Use TUnit's simplerIsNotNull()assertion to match the method's contract.The test converts
Argument.ThrowIfNull()to async using TUnit's assertion API, but usesIsNotNullOrWhiteSpace()which validates more than the method guarantees. SinceThrowIfNullonly checks for null (not whitespace), the assertion should beIsNotNull(), which TUnit provides as a standard assertion method._ = await Assert.That(argument).IsNotNull();
🧹 Nitpick comments (2)
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNullOrWhiteSpace.cs (1)
47-58: Consider verifying the method's return value.While the assertion checks that the argument is not null or whitespace, it doesn't verify that
ThrowIfNullOrWhiteSpacereturns the expected value. If the method returns the argument, consider capturing and asserting on the return value for more complete test coverage.Optional enhancement:
[Test] public async Task ThrowIfNullOrWhiteSpace_WhenArgumentIsNotEmpty_ReturnsArgument() { // Arrange var argument = "argument"; // Act - Argument.ThrowIfNullOrWhiteSpace(argument); + var result = Argument.ThrowIfNullOrWhiteSpace(argument); // Assert - _ = await Assert.That(argument).IsNotNullOrWhiteSpace(); + _ = await Assert.That(result).IsNotNullOrWhiteSpace(); }tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNullOrEmpty.cs (1)
35-46: Consider verifying the method's return value.While the assertion validates that the argument is not null or whitespace, it doesn't verify that
ThrowIfNullOrEmptyreturns the expected value. Consider capturing and asserting on the return value for complete test coverage.Optional enhancement:
[Test] public async Task ThrowIfNullOrEmpty_WhenArgumentIsNotEmpty_ReturnsArgument() { // Arrange var argument = "argument"; // Act - Argument.ThrowIfNullOrEmpty(argument); + var result = Argument.ThrowIfNullOrEmpty(argument); // Assert - _ = await Assert.That(argument).IsNotNullOrWhiteSpace(); + _ = await Assert.That(result).IsNotNullOrWhiteSpace(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (13)
.editorconfig(1 hunks)Directory.Packages.props(1 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests.cs(1 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfEqual.cs(3 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfGreaterThan.cs(3 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfGreaterThanOrEqual.cs(4 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfLessThan.cs(3 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfLessThanOrEqual.cs(4 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNotEqual.cs(3 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNull.cs(3 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNullOrEmpty.cs(5 hunks)tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNullOrWhiteSpace.cs(5 hunks)tests/NetEvolve.Arguments.Tests.Unit/NetEvolve.Arguments.Tests.Unit.csproj(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Tests / Run Tests / Testing .NET solution
🔇 Additional comments (22)
.editorconfig (1)
45-45: LGTM!The addition of
slnxto the XML project files pattern appropriately extends EditorConfig support for the newer Visual Studio solution format.tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfLessThanOrEqual.cs (2)
7-19: LGTM!The migration from
[Fact]to[Test]is correct, and the test logic properly verifies that an ArgumentOutOfRangeException is thrown when the argument is less than the minimum.
21-33: LGTM!The test correctly verifies that an ArgumentOutOfRangeException is thrown when the argument equals the minimum.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNotEqual.cs (1)
7-19: LGTM!The test correctly verifies that an ArgumentOutOfRangeException is thrown when the argument is not equal to the maximum.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNullOrWhiteSpace.cs (3)
8-19: LGTM!The test correctly verifies that ArgumentNullException is thrown for null arguments.
21-32: LGTM!The test correctly verifies that ArgumentException is thrown for empty string arguments.
34-45: LGTM!The test correctly verifies that ArgumentException is thrown for whitespace arguments.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfGreaterThan.cs (1)
8-20: LGTM!The test correctly verifies that ArgumentOutOfRangeException is thrown when the argument exceeds the maximum.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfLessThan.cs (1)
7-19: LGTM!The test correctly verifies that ArgumentOutOfRangeException is thrown when the argument is less than the minimum.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNullOrEmpty.cs (4)
9-20: LGTM!The test correctly verifies that ArgumentNullException is thrown for null arguments.
22-33: LGTM!The test correctly verifies that ArgumentException is thrown for empty string arguments.
48-59: LGTM!The test correctly verifies that ArgumentNullException is thrown for null IEnumerable arguments.
61-65: LGTM!The test data properties are well-structured and provide appropriate test cases for empty and populated collections.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests.cs (1)
4-4: LGTM!The namespace change from
NetEvolve.Extensions.XUnittoNetEvolve.Extensions.TUnitcorrectly aligns with the framework migration.tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfGreaterThanOrEqual.cs (2)
7-7: LGTM! Test attribute migration looks correct.The migration from xUnit's
[Fact]to TUnit's[Test]attribute is properly applied.Also applies to: 21-21
35-36: The async Task signature is required and justified.The test method uses async assertions (
await Assert.That()) which TUnit requires to be awaited, necessitating the test method to return Task and be declared async. The fact that the method under test is synchronous is irrelevant; the async signature is required because the test itself performs async operations (the assertion on line 46).Likely an incorrect or invalid review comment.
Directory.Packages.props (1)
17-22: LGTM! Package updates align with the TUnit migration.The centralized package version updates properly introduce TUnit testing infrastructure while maintaining XUnit support (line 19), which may facilitate a gradual migration strategy.
tests/NetEvolve.Arguments.Tests.Unit/NetEvolve.Arguments.Tests.Unit.csproj (2)
9-12: LGTM! Package references correctly updated for TUnit migration.The package references properly replace xUnit infrastructure with TUnit equivalents, aligning with the centralized package versions in Directory.Packages.props.
3-3: The original review comment is based on outdated information and does not reflect the actual project state.The main library project (NetEvolve.Arguments.csproj) targets
netstandard2.0;net7.0;net8.0—it never targeted .NET 6.0. The test project's removal of net6.0 is a justified alignment with the library's actual supported frameworks. Additionally, .NET 6.0 support ended in November 2024; the project is now in November 2025, so the warning about ongoing support is outdated. This change requires no action.Likely an incorrect or invalid review comment.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfEqual.cs (1)
8-8: LGTM! Test attribute migration looks correct.The migration from xUnit's
[Fact]to TUnit's[Test]attribute is properly applied.Also applies to: 22-22
tests/NetEvolve.Arguments.Tests.Unit/ArgumentTests_ThrowIfNull.cs (2)
7-18: LGTM - Clean migration to TUnit.The test correctly validates that
ThrowIfNullthrowsArgumentNullExceptionwhen passed a null reference. The migration from[Fact]to[Test]is appropriate.
33-44: LGTM - Proper null pointer validation.The test correctly validates that
ThrowIfNullthrowsArgumentNullExceptionwhen passed a null pointer in an unsafe context. The migration from[Fact]to[Test]is appropriate, and the unsafe code is properly scoped.
Summary by CodeRabbit
Chores
Style
✏️ Tip: You can customize this high-level summary in your review settings.