-
Notifications
You must be signed in to change notification settings - Fork 22
tests(tfm): Improve tfm tests by using inline data #419
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
base: main
Are you sure you want to change the base?
Conversation
…test case additions
- The IsWindows condition does already make sure, this only gets executed on Windows
f9641bc to
2c9884c
Compare
|
@jeromelaban this is looking for review 👍 |
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.
Pull Request Overview
This PR refactors the TFM (Target Framework Moniker) tests to improve scalability and maintainability by introducing inline data-driven tests. The changes also update vulnerable NuGet package versions to their patched versions and add pragma warnings to suppress platform compatibility warnings.
- Introduced a new parametrized test method using
[Theory]and[InlineData]attributes for easier test case management - Updated NuGet package versions from 5.11.5 to 5.11.6 to address security vulnerabilities
- Added pragma warning suppressions for platform compatibility warnings in Windows-specific code
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| UnoCheck/Util.cs | Added pragma warning suppressions for platform compatibility warnings around Windows-specific code |
| UnoCheck/UnoCheck.csproj | Updated NuGet package versions from 5.11.5 to 5.11.6 to address vulnerabilities |
| UnoCheck.Tests/UnoCheck.Tests.csproj | Updated test framework package versions to latest versions |
| UnoCheck.Tests/CheckCommandTests.cs | Added new parametrized test method and improved existing test assertions |
Comments suppressed due to low confidence (1)
UnoCheck.Tests/CheckCommandTests.cs:61
- The test assumes "net9.0-desktop" will always map to "win32" but this behavior may be OS-dependent like the existing tests. Consider adding OS-specific test cases or documenting why this assumption is valid.
[InlineData(new string[] { "net9.0-android", "net9.0-ios", "net9.0-browserwasm", "net9.0-desktop"/*, "net9.0"*/}, new string[] { "android", "ios", "web", "win32" })]
|
@jeromelaban just a thought: what do you think about using the If a dedicated tag (similar to how we override Uno packages for prerelease scenarios) were introduced, we could explicitly signal whether all outputs, none, or a specific set should be included. If this seems like a viable option, I’d be happy to create a separate PR for it. |
I don't understand what you mean there. What's the intent? |
GitHub Issue: closes #
PR Type:
What is the current behavior? 🤔
using only fact for the os specific tfm checks may be good for this specific task, but requires rewriting or changing current tested values
What is the new behavior? 🚀
PR Checklist ✅
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information ℹ️
there are still left warnings that will make the ci fail but those are coming from the not longer supported net3.1+6 net version the check tool is targeting!