tests(tfm): Improve tfm tests by using inline data#419
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.
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? |
|
@jeromelaban currently Uno-check will complain about missing workloads as soon as we are creating a Uno.Sdk managed library project to use UnoFeatures and also if we have testing projects included which require our referenced/to-be-tested Uno UI project to also have this platform less target framework. So all users who would not have everything installed just because they dont Target e.g. android or whatever, will constantly get prompted about problems found by uno-check and let me tell you, this is really annoying 😬🫤 So the idea was to make uno check runned through the vs extension for example configure-able if it finds the
|
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!