Skip to content

Conversation

@DevTKSS
Copy link
Contributor

@DevTKSS DevTKSS commented Jun 28, 2025

GitHub Issue: closes #

PR Type:

  • 🔄 Refactoring (no functional changes, no api changes)

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

  • the Test project and the check project itself had vulnurable dotnet package warnings

What is the new behavior? 🚀

  • To be able to scale the tested cases and values also in future appearing issues, this added test introduces the option to "Just" add a InlineData line and check if the Parse Method is the culprit
  • Updated the versions to the official released patched version (NO namespace changes or similar!)

PR Checklist ✅

Please check if your PR fulfills the following requirements:

  • 📝 Commits must be following the Conventional Commits specification.
  • 🧪 Added test (for bug fixes / features) (if applicable)
  • 📚 Docs have been added/updated which fit documentation template (for bug fixes / features)
  • 🖼️ Validated PR Screenshots Compare Test Run results.
    test run result
  • ❗ Contains NO breaking changes

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!

DevTKSS added 3 commits June 29, 2025 01:12
- The IsWindows condition does already make sure, this only gets executed on Windows
@DevTKSS DevTKSS force-pushed the improve-tfm-Tests branch from f9641bc to 2c9884c Compare June 28, 2025 23:12
@DevTKSS
Copy link
Contributor Author

DevTKSS commented Jul 18, 2025

@jeromelaban this is looking for review 👍

@jeromelaban jeromelaban requested a review from Copilot July 18, 2025 18:55
Copy link
Contributor

Copilot AI left a 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" })]

@DevTKSS
Copy link
Contributor Author

DevTKSS commented Nov 28, 2025

@jeromelaban just a thought: what do you think about using the <GenerateLibraryLayout>true</GenerateLibraryLayout> project tag — or alternatively <OutputType>Library</OutputType> — if we can parse them like the Tfm's?
Both are typically required anyway. But that way we could provide maybe Uno.Check capability to not file us all tfms by default like in real UI Client apps 💡

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.

@jeromelaban
Copy link
Member

@jeromelaban just a thought: what do you think about using the true project tag — or alternatively Library — if we can parse them like the Tfm's?

I don't understand what you mean there. What's the intent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants