Add BuilderMixins and WithInstance Tests #4232
Merged
+757
−22
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduces comprehensive unit tests for the new BuilderMixins extension methods and WithInstance overloads in ReactiveUI.Builder. Updates test targets to include net8.0, suppresses CA1812 warnings, and updates API approval files to reflect new APIs and platform requirements. Also adds null argument validation and coverage for new builder methods.
glennawatson
approved these changes
Dec 16, 2025
Deleted references to Syncfusion.Maui.Toolkit and Microsoft.Extensions.Logging.Debug from Directory.Packages.props, likely because they are no longer needed in the project.
…activeUI into ExtendBuilderTests
glennawatson
approved these changes
Dec 17, 2025
Added a null check for the reactiveUIBuilder parameter in the UsingSplatBuilder extension method to prevent ArgumentNullException. Updated tests to cover the new null check case.
glennawatson
approved these changes
Dec 17, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive unit tests for the BuilderMixins extension methods and WithInstance overloads in ReactiveUI.Builder. The changes include:
- Added null argument validation to the
UsingSplatBuilderextension method - Created two new comprehensive test files covering all BuilderMixins methods and all 16 WithInstance overloads
- Updated test target frameworks to include net8.0
- Suppressed CA1812 warnings in the test project
- Updated API approval files to reflect new APIs and Windows platform version updates (from 10.0.17763.0 to 10.0.19041.0)
- Cleaned up unused package references
Key Changes
- Null validation added to
UsingSplatBuildermethod for consistency with other extension methods - Comprehensive test coverage using reflection-based test generation to systematically test all method overloads
- Test framework updates to support net8.0 testing alongside net9.0 and net10.0
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ReactiveUI/Mixins/BuilderMixins.cs | Added null argument validation to UsingSplatBuilder method |
| src/ReactiveUI.Builder.Tests/BuilderMixinsTests.cs | New comprehensive test file (~400 lines) covering all BuilderMixins extension methods including null checks, scheduler configuration, registration, view/viewmodel registration, and platform configuration |
| src/ReactiveUI.Builder.Tests/BuilderMixinsWithInstanceTests.cs | New test file (~200 lines) using reflection-based test generation to cover all 16 WithInstance overloads for both extension methods and builder instance methods |
| src/ReactiveUI.Builder.Tests/ReactiveUIBuilderCoreTests.cs | Added three new test methods for null validation and BuildApp functionality |
| src/ReactiveUI.Builder.Tests/ReactiveUI.Builder.Tests.csproj | Suppressed CA1812 analyzer warning for test classes |
| src/ReactiveUI.Tests/API/ApiApprovalTests.ReactiveUI.DotNet8_0.verified.txt | Updated API approval file with new APIs (BuilderMixins, WithInstance overloads, RxSchedulers), platform version changes, and removal of XamForms from RegistrationNamespace enum |
| src/Directory.Build.props | Added net8.0 to ReactiveUITestTargets for both Windows and non-Windows platforms |
| src/Directory.Packages.props | Removed unused package references (Syncfusion.Maui.Toolkit and Microsoft.Extensions.Logging.Debug) |
The code follows ReactiveUI's coding guidelines with proper null checks, XML documentation, and comprehensive test coverage. The tests use modern C# features appropriately (collection expressions, file-scoped namespaces, target-typed new) and follow the repository's testing patterns.
Co-authored-by: Copilot <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4232 +/- ##
==========================================
+ Coverage 32.07% 40.33% +8.25%
==========================================
Files 146 146
Lines 6653 6655 +2
Branches 1024 1025 +1
==========================================
+ Hits 2134 2684 +550
+ Misses 4347 3779 -568
- Partials 172 192 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
tests
What is the new behavior?
Introduces comprehensive unit tests for the new BuilderMixins extension methods and WithInstance overloads in ReactiveUI.Builder. Updates test targets to include net8.0, suppresses CA1812 warnings, and updates API approval files to reflect new APIs and platform requirements. Also adds null argument validation and coverage for new builder methods.
What might this PR break?
N/A
Please check if the PR fulfills these requirements
Other information: