-
Notifications
You must be signed in to change notification settings - Fork 288
Add FrameworkConditionAttribute for conditional test execution based on .NET framework #6071
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Evangelink <[email protected]>
Co-authored-by: Evangelink <[email protected]>
…ication Co-authored-by: Evangelink <[email protected]>
FrameworkConditionAttribute|
|
||
| // Check for .NET 5+ | ||
| if (frameworkDescription.StartsWith(".NET ", StringComparison.OrdinalIgnoreCase) || | ||
| frameworkDescription.StartsWith(".NET Core", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right. The check for ".NET Core" in line 82 was redundant since that case is already handled by the check on lines 75-78 which would return early. I've removed the redundant condition. Fixed in commit 4fe0d32.
| /// <summary> | ||
| /// Represents .NET 5 and later versions (unified platform). | ||
| /// </summary> | ||
| Net = 1 << 2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing. This one will be net5.0 only per the current implementation. I'm also not sure if it's good to add the versions to this enum:
- We will have to drop entries from the enum when we are dropping support for old target frameworks.
- If .NET 10 is used with an MSTest version that doesn't have an enum entry for .NET 10, what the behavior will be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right about both concerns. I've simplified the Frameworks enum to address these issues:
- Removed version-specific flags: Eliminated
Net6OrGreater,Net7OrGreater,Net8OrGreater, andNet9OrGreater - Kept only basic framework types:
NetFramework,NetCore, andNet(for .NET 5+)
This design is much cleaner and follows the same pattern as OperatingSystems enum. The Net flag now covers all .NET 5+ versions (5, 6, 7, 8, 9, etc.) without requiring new enum entries for each version. If users need version-specific logic, they can check Environment.Version in their test code.
Benefits:
- No maintenance burden of adding/removing version flags
- Forward compatibility (new .NET versions automatically work)
- Simpler, less confusing API
- Consistent with existing condition attribute patterns
Fixed in commit f3fa939.
Co-authored-by: Youssef1313 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be revisited for UWP.
Co-authored-by: Youssef1313 <[email protected]>
Co-authored-by: Youssef1313 <[email protected]>
Added UWP support to the FrameworkConditionAttribute. The implementation includes a new |
|
Closing stale PR. |
Summary
This PR implements
FrameworkConditionAttribute, a new condition attribute that enables conditional test execution based on the .NET framework version. This provides a cleaner alternative to preprocessor directives (#if NET8_0_OR_GREATER, etc.) in test scenarios.Problem
Currently, developers need to use preprocessor directives to conditionally compile tests for different .NET frameworks:
This approach has several drawbacks:
Solution
The new
FrameworkConditionAttributefollows the same pattern asCIConditionAttributeandOSConditionAttribute, enabling runtime conditional test execution:Implementation Details
New Types Added
Frameworksenum - Flags enum supporting:NetFramework- .NET FrameworkNetCore- .NET Core 1.x, 2.x, 3.xNet- .NET 5 and laterNet6OrGreater- .NET 6+Net7OrGreater- .NET 7+Net8OrGreater- .NET 8+Net9OrGreater- .NET 9+FrameworkConditionAttribute- Condition attribute that:ConditionBaseAttributeIncludeandExcludemodesRuntimeInformation.FrameworkDescriptionandEnvironment.Versionfor detectionFramework Detection Logic
RuntimeInformation.FrameworkDescriptionto identify framework typeEnvironment.Versionfor version-specific detectionNet,Net6OrGreater,Net7OrGreater,Net8OrGreater)Net)Test Coverage
Comprehensive test suite covering:
Benefits
ConditionBaseAttributeconventionsFiles Changed
Frameworks.cs- Framework enumerationFrameworkConditionAttribute.cs- Main attribute implementationFrameworkConditionAttributeTests.cs- Comprehensive test suitePublicAPI.Unshipped.txt- Added public API entriesFixes #6070.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.