refactor(tests): Drop Uno-native leftovers from runtime tests - #24440
MartinZikmund wants to merge 2 commits into
Conversation
Uno 7.0 removed the native (non-WinUI) renderers, so Uno.UI.RuntimeTests now compiles in exactly two configurations: Skia (net10.0;net11.0) and WinUI/WinAppSDK. `__ANDROID__`, `__IOS__`, `__TVOS__`, `__APPLE_UIKIT__`, `__WASM__`, `HAS_NATIVE_VIEWS`, `NETFX_CORE` and `WINDOWS_UWP` are dead in both, and `HAS_RENDER_TARGET_BITMAP`/`HAS_COMPOSITION_API` true in both. Strip the non-WinUI members from `RuntimeTestPlatforms` and rewrite every usage. `Native` becomes `NativeWinUI` — it included the WinUI flag, so dropping it outright would have silently re-enabled those tests on the WinUI head. `Wasm`/`Android`/`IOS` become `SkiaWasm`/`SkiaAndroid`/ `SkiaIOS`; those aliases could not simply be repointed at the Skia values because `RuntimeTestsPlatformHelper` enumerates single-bit members and would then report multiple current platforms. Collapse the dead `#if` branches to their surviving arm, keeping the `#else` arms WinUI still needs, and unwrap the two always-true symbols. `OperatingSystem.IsAndroid()`/`IsIOS()`/`IsBrowser()` checks are left alone: on the single Skia assembly they mean Skia-on-Android/iOS/WASM. The deliberately-undefined WinUI-port markers (`USING_TAEF`, `MUX_PRERELEASE`, `#if false`, …) are left alone per code-style rules, as are the `#if !IS_UNIT_TESTS` guards in `Given_ListViewBase_Items.cs`, which are live because `Uno.UI.UnitTests` Compile-links that file. Un-gate `Given_EmailMessage`/`Given_EmailRecipient`: their `#if WINAPPSDK || __IOS__ || __ANDROID__` would otherwise leave them compiling only on WinUI with half their tests excluded there. The types are platform-agnostic in Uno.WinRT, so 21 tests that ran nowhere now run on Skia. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E7YdbqsxwHyH4icDtYfUro
These test files were wrapped in a native `#if` from their first line to EOF, so they compiled on no surviving configuration: WebAssemblyRuntime, FontFamilyLoader, GeometryData and LinearGradientBrush_Faux (native WASM DOM), Launcher, Package, AnalyticsInfo, CoreDispatcher, StatusBar, WebAuthenticationBrokerProvider, Android_Relative_Shape_Rounding and the NativeCtorGenerator fixture (native Android/UIKit). Drop the native-view fixture wholesale — Given_Native_View, NativeViewPage, NativeContainer and both Native_View_Page markup pages. With native views gone, `NativeView` collapsed to an empty CheckBox subclass, leaving those tests asserting that a CheckBox can live in a Grid. Drop Given_JumpListItem, which compiled only under WINAPPSDK yet carried a class-level `Exclude, NativeWinUI`, making it unreachable in both configurations. Delete the fixtures orphaned by the tests removed in the previous commit: MyLinearLayout, When_XBind_TargetDisposed and CommandBarPages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E7YdbqsxwHyH4icDtYfUro
github-actions
Bot
added
area/skia ✏️
|
Claude finished @MartinZikmund's task in 3m 35s —— View job Review complete ✅
Verdict: Ship — no correctness issues foundFindings by severity: 0 blocking, 0 minor. No inline comments posted. What I verified
Cross-cutting notes (non-blocking)
|
There was a problem hiding this comment.
🟡 Changes recommended
A few refactoring edits introduce concrete test/runner issues (disabled test via unconditional Assert.Inconclusive(), and platform-incorrect PDF password expectations), which should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors Uno.UI.RuntimeTests to remove obsolete “native (non-WinUI) renderer” leftovers (dead #if symbols, platform flags, and native-only tests/fixtures) now that Uno 7.0 uses a single Skia test assembly plus a WinUI/WinAppSDK head.
Changes:
- Simplifies
RuntimeTestPlatformsandRuntimeTestsPlatformHelperby dropping dead native platform flags/aliases and updating call sites to use the remainingSkia*+NativeWinUIflags. - Removes dead preprocessor branches and native-only test code, converting platform gating to runtime attributes where applicable.
- Updates runtime test authoring guidance in
.claude/rules/runtime-tests.mdto reflect the surviving platform configurations.
File summaries
| File | Description |
|---|---|
| src/Uno.UI.RuntimeTests/Helpers/RuntimeTestPlatforms.cs | Removes dead native renderer flags/aliases; keeps NativeWinUI and Skia* flags. |
| src/Uno.UI.RuntimeTests/Helpers/RuntimeTestsPlatformHelper.cs | Drops native platform detection helpers and maps only to NativeWinUI/Skia*. |
| .claude/rules/runtime-tests.md | Updates guidance to reflect only Skia + WinUI native heads and the updated platform flags. |
| src/Uno.UI.RuntimeTests/MUX/Utilities/IdleSynchronizer.cs | Removes WASM-specific guard around blocking wait logic. |
| src/Uno.UI.RuntimeTests/Tests/Windows_Data/Pdf/Given_PdfDocument.cs | Removes native-platform #if branches in password-related tests. |
| src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Border.cs | Removes platform #if around Border_AntiAlias, leaving unconditional inconclusive. |
Review details
Suppressed comments (2)
src/Uno.UI.RuntimeTests/MUX/Utilities/IdleSynchronizer.cs:19
- IdleSynchronizer.Wait blocks on
WaitForIdle().Wait(...). On Skia WASM, synchronous Task.Wait is known to throwPlatformNotSupportedException(and can also deadlock single-threaded runtimes), so this helper should be a no-op (or otherwise avoid blocking) when running in the browser.
Even though the method is marked obsolete, it is still called by multiple MUX tests (e.g., FlowLayoutTests / EffectiveViewportScrollPresenterTests), so an exception here would break those runs.
src/Uno.UI.RuntimeTests/Tests/Windows_Data/Pdf/Given_PdfDocument.cs:180
PdfDocumentpassword overload is not supported on Android (it throws for any non-empty password), so this test should not expect a successful load on SkiaAndroid. If the intent is to keep the test as a future guard, it should assertNotImplementedExceptionon Android until password support is added.
- Files reviewed: 190/190 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| public async Task When_LoadFromStreamAsync_With_Valid_Password() | ||
| { | ||
| var stream = GetSreamFromResource(PdfDocument_Name_Protected)?.AsRandomAccessStream(); | ||
| Assert.IsNotNull(stream, "Not valid stream"); | ||
|
|
||
| #if __ANDROID__ | ||
| await Assert.ThrowsAsync<NotImplementedException>(async () => await PdfDocument.LoadFromStreamAsync(stream, PdfDocument_Password_Valid)); | ||
| #else | ||
| var pdfDocument = await PdfDocument.LoadFromStreamAsync(stream, PdfDocument_Password_Valid); | ||
| await CheckDocumentAsync(pdfDocument, ReferencePageImage_ProtectedUri, hasPassword: true); |
| [TestMethod] | ||
| public async Task Border_AntiAlias() | ||
| { | ||
|
|
||
| #if !__ANDROID__ | ||
| Assert.Inconclusive(); | ||
| #endif | ||
| const string secondRectBlueish = "#ff9e9eff"; |
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24440/wasm-skia-net9/index.html |
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-24440/docs/index.html |
|
The build 232357 found UI Test snapshots differences: Details
|
morning4coffe-dev
left a comment
morning4coffe-dev
left a comment
There was a problem hiding this comment.
Approved the test-only cleanup at db188f11dfce, compared with its true merge base 17a700f7f859.
I evaluated the actual compiler symbols and Compile items for Skia and WinUI on both net10 and net11, plus the linked net11 unit-test configuration. A Roslyn-based comparison covered all 183 changed C# files in those five configurations; I reviewed the remaining active differences, the six removed XAML fixtures and the authoring-rule change. The surviving platform bits are unchanged, Native exclusions retain their WinUI meaning, and the clipboard/leak-test masks retain their meaning on live platforms. I also checked the runner's separate class/method condition evaluation before accepting the removal of always-false exclusions. The removed fake-native fixtures now reduce to managed CheckBox/ContentControl wrappers or unused/never-selected tests; the shared email tests are intentionally enabled rather than discarded. No removed enum references remain in the committed C# tree.
Compile: the exact-head source-based SamplesApp built successfully for Release net11.0-desktop with the CI-pinned SDK and diagnostic analyzers disabled. Runtime: the actual Windows Skia app recorded 51 passes, two existing ListView skips and the existing Border anti-alias inconclusive case. This includes all 21 newly enabled email cases, the linked ListView tests and the remaining VisualTreeHelper cases.
The two existing automated findings are not regressions from this cleanup: Border_AntiAlias already reaches Assert.Inconclusive in both surviving configurations because neither defines __ANDROID__; Skia-on-Android also loads that shared assembly. The PDF class remains behind !HAS_UNO and excludes NativeWinUI, so these edits do not newly run its password expectations on SkiaAndroid. I left the other reviewer's threads unchanged.
This is not a green-CI or full-suite claim. Build 232357 includes this head, but its native WinUI run crashed with missing resource MergedRegression_ThemedBrush and produced no final XML. I have not established a passing base run or cleared that gate. No local native WinUI, mobile or browser run is claimed here; normal CI and independent-review requirements still apply.
GitHub Issue: closes #24439
PR Type:
🔄 Refactoring (no functional changes, no api changes)
What changed? 🚀
Uno 7.0 removed the native (non-WinUI) renderers, but
Uno.UI.RuntimeTestsstill carried their conditions,#ifs, tests and fixtures. This removes them: 190 files, +237 / −5,874.The project now compiles in exactly two configurations — verified with
dotnet msbuild -getProperty:DefineConstants, not assumed:net10.0;net11.0)__SKIA__,HAS_UNO,HAS_UNO_WINUI,__CROSSRUNTIME__,UNO_REFERENCE_API,SUPPORTS_RTL,UNO_SUPPORTS_NATIVEHOST,UNO_HAS_ENHANCED_LIFECYCLE,UNO_HAS_MANAGED_POINTERS,UNO_HAS_MANAGED_SCROLL_PRESENTER,HAS_INPUT_INJECTOR,HAS_COMPOSITION_API,HAS_RENDER_TARGET_BITMAPWINAPPSDK,WINDOWS_WINUI,HAS_COMPOSITION_API,HAS_RENDER_TARGET_BITMAPThere is no third: the single Skia assembly is what the Skia Desktop, Skia-Android, Skia-iOS and Skia-WASM heads all load. So
__ANDROID__,__IOS__,__TVOS__,__APPLE_UIKIT__,__WASM__,HAS_NATIVE_VIEWS,NETFX_COREandWINDOWS_UWPwere dead in both configurations, andHAS_RENDER_TARGET_BITMAP/HAS_COMPOSITION_APItrue in both.1.
RuntimeTestPlatforms(commit 1)Removed
NativeWasm,NativeAndroid,NativeIOS,NativeTvOS,NativeUIKit,NativeMobile,Native, and theWasm/Android/IOSaliases.NativeWinUIand everySkia*member stay.Two things worth a reviewer's attention:
NativebecameNativeWinUI, not nothing.NativewasNativeWasm | NativeAndroid | NativeIOS | NativeTvOS | NativeWinUI— it included WinUI. Dropping it outright would have silently re-enabled ~30 tests on the WinUI head.Wasm/Android/IOSaliases could not be repointed at their Skia values.RuntimeTestsPlatformHelperenumerates single-bit members to detect the current platform and throws when it finds more than one; an alias sharing a bit withSkiaWasmwould make every run fail with "Multiple runtime platforms detected". They were rewritten at each call site instead.2. Dead
#ifs (commit 1)~770 directives removed: 274
__APPLE_UIKIT__, 258__ANDROID__, 197__WASM__, 38__IOS__, plus__TVOS__,NETFX_CORE,WINDOWS_UWP,HAS_NATIVE_VIEWS, and the two always-true symbols. Compound conditions were simplified rather than deleted —#if !__SKIA__ && !__WASM__becomes#if !__SKIA__(still live: it means "WinUI only"), and#elifarms were promoted where the leading arm died.Deliberately left alone:
USING_TAEF,MUX_PRERELEASE,#if false,FOCUS_IMPLEMENTED,LOOPING_SELECTOR_AVAILABLE,ARRANGE_DIRTY_PATH_AVAILABLE,NEED_CUSTOM_ADJUSTMENTS_FOR_CP_BYPASS. These are never-defined by design — MUX-port fidelity and parked ported code — as.claude/rules/code-style.mddocuments. Also left: the#if !IS_UNIT_TESTSguards inGiven_ListViewBase_Items.cs, which are live becauseUno.UI.UnitTestsCompile-links that file and the symbol is defined there.Not touched, on purpose:
OperatingSystem.IsAndroid()/IsIOS()/IsBrowser()runtime checks. On the single Skia assembly these mean Skia-on-Android / Skia-on-iOS / Skia-on-WASM — fully supported 7.0 targets.3. Native-only tests and fixtures (commit 2)
97
[TestMethod]s and 31 files removed. Twelve files were wrapped in a native#iffrom line 1 to EOF, so they compiled nowhere. The native-view fixture went wholesale (Given_Native_View,NativeViewPage/,NativeContainer, bothNative_View_Pagepages) — with native views gone,NativeViewhad collapsed to an emptyCheckBoxsubclass and the tests asserted that a CheckBox can live in aGrid.Given_JumpListItemcompiled only underWINAPPSDKyet carried a class-levelExclude, NativeWinUI, making it unreachable in both configurations.4. Coverage restored, not removed
Given_EmailMessageandGiven_EmailRecipientwere gated#if WINAPPSDK || __IOS__ || __ANDROID__with several methods excluded on WinUI — collapsing that would have left 16 tests compiling only on WinUI with half of them skipped there.EmailMessage/EmailRecipientare plain shared files inUno.WinRTwith no platform gate, so these were un-gated instead of deleted: 21 tests that previously ran on no live platform now run and pass on Skia.Known follow-ups (deliberately out of scope)
Given_NavigationViewstill has two#if HAS_UNO && !HAS_UNO_WINUIblocks. Provably dead, but that is Uno-UWP-flavor residue rather than native rendering — a separate sweep.Given_EmailManagerkeeps one unreachable test. Un-gating it would fail:EmailManager.ShowComposeNewEmailAsync(null)falls through to the mailto path and throwsNullReferenceExceptioninstead ofArgumentNullException. That is a real gap worth its own issue.Given_ApplicationView.When_StartupVisibleBounds_Has_Valueis[Ignore]d and fed by an#if __ANDROID__block inSamplesApp/App.Assertions.cs; whether to revive or delete it depends on the Skia-Android head's define set.Validation
Compile — four configurations, all clean:
Uno.UI.RuntimeTests.Skia(net10.0)Uno.UI.RuntimeTests.Windows(WinAppSDK)Uno.UI.UnitTests(net11.0)SamplesApp(net11.0-desktop)Runtime — Skia Desktop, filtered to the most-affected classes: 55 passed, 2 skipped, 0 failed. The two skips are pre-existing
[Ignore("#12183: …")], byte-identical tomaster.Given_EmailMessageGiven_EmailRecipientGiven_ListViewBase_ItemsGiven_VisualTreeHelperGiven_ClipboardStatic — scripted scans over the whole project: zero leftover dead-symbol
#ifs, zero removed enum members (including theusing static RuntimeTestPlatformsbare-identifier form), zeroAndroid.Views/UIKit.*references,#if/#endifbalanced in every file, and no BOM/CRLF damage across the 159 modified files.A full-suite run was not performed. By construction this change cannot alter behaviour on either live configuration — removed branches were dead in both, always-true unwraps are no-ops, and the enum rewrites preserve each predicate's value. The one class that could shift is an
[Ignore]gated by a now-always-false#ifsuddenly activating;Given_ProgressRing,Given_ToolTipandGiven_AutoSuggestBoxwere each checked and evaluate identically before and after.PR Checklist ✅
Screenshots Compare Test Runresults.Test-only change: no product code is touched, so no new tests or feature docs are warranted —
.claude/rules/runtime-tests.mdwas updated to reflect the survivingRuntimeTestPlatformsflags. Screenshot comparison is not applicable. Public API surface is unchanged;RuntimeTestPlatformslives in a test assembly.🤖 Generated with Claude Code
https://claude.ai/code/session_01E7YdbqsxwHyH4icDtYfUro