test: Disable three Skia-WASM flakes behind tracked issues - #24158
Conversation
Sorry, something went wrong.
|
Claude finished @MartinZikmund's task in 1m 11s —— View job Review: test: Disable two Skia-WASM flakes behind tracked issuesVerdict: ✅ ship (1 nit, no blockers) The mechanism is correct: [PlatformCondition(ConditionMode.Exclude, RuntimeTestPlatforms.SkiaWasm)] is the right attribute to skip on Skia-WASM only without touching other targets, per .claude/rules/runtime-tests.md. Both tests remain enabled on all Desktop Skia, Skia-Android, and Skia-iOS targets. The .skia.cs file suffix for the TextBox change is appropriate — it already narrows scope to Skia, and the attribute then excludes just the WASM variant. Issues by severity:
Cross-cutting: PR title follows Conventional Commits (test:), both issues are tracked (#24156, #24157), and the PR description includes solid reproduction evidence (assertion text, scroll offsets, CI build reference). Nothing else to flag. |
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR reduces CI noise by excluding two known-flaky runtime tests on Skia WASM only, while leaving them enabled on all other runtime-test targets. Each exclusion is annotated with a link to the tracking issue intended to re-enable the test once the underlying settle race is fixed.
Changes:
- Exclude Given_ListViewBase.When_Incremental_Load_Default on RuntimeTestPlatforms.SkiaWasm (tracked by #24156).
- Exclude Given_TextBox.When_OuterScrollViewer_BringIntoView_Scrolls_To_Caret on RuntimeTestPlatforms.SkiaWasm (tracked by #24157).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TextBox.skia.cs | Excludes a flaky BringIntoView/scroll-settle runtime test on Skia WASM with an issue link. |
| src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs | Excludes a flaky incremental-load/materialization runtime test on Skia WASM with an issue link. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
|
Claude finished @MartinZikmund's task in 2m 36s —— View job Review: test: Disable two Skia-WASM flakes behind tracked issuesVerdict: ✅ ship (1 nit on the new commit, consistent with prior review) The new commit (010c8cf1) extends the existing pattern to disable When_Incremental_Load_ShouldStop on Skia-WASM as well, by OR-ing RuntimeTestPlatforms.SkiaWasm into the existing Native exclusion. The mechanism is correct — ConditionMode.Exclude with a flags OR is the right way to accumulate excluded platforms on a single [PlatformCondition]. Issues by severity:
Cross-cutting: All three disabled tests follow the same shape: [PlatformCondition(ConditionMode.Exclude, RuntimeTestPlatforms.SkiaWasm)] with an inline comment explaining the failure mode and linking the tracking issue. No other concerns. |
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs:3876
- The trailing inline comment on the combined Native|SkiaWasm exclusion still says it was destabilized by PR #23269, but this PR is adding SkiaWasm exclusion for a different tracked flake (#24147). As written, it implies SkiaWasm is excluded due to #23269, which is misleading for future re-enablement work.
[PlatformCondition(ConditionMode.Exclude, RuntimeTestPlatforms.Native | RuntimeTestPlatforms.SkiaWasm)] // Destabilized by changes in https://github.com/unoplatform/uno/pull/23269
Sorry, something went wrong.
|
The build 229158 found UI Test snapshots differences: skia-linux-screenshots: 57, skia-windows-screenshots: 2348, wasm: 11 Details
|
Sorry, something went wrong.
Build 229119 failed WebAssembly Skia Runtime Tests 0 and 2 with exactly one failure each; every other runtime-test target (Desktop Skia Windows/Linux/ macOS, Skia-Android, Skia-iOS) ran both tests green in that same build, which is the signature of a slow-runtime settle race rather than a regression. - Given_ListViewBase.When_Incremental_Load_Default samples materialization after a fixed Task.Delay(500), and the last materialized index moved backwards between two identical scroll-to-bottom operations (27 then 11), so the second sample lands mid-recycle (#24156). This is the weaker sibling of When_Incremental_Load_ShouldStop (#24147), which polls and still flakes. - Given_TextBox.When_OuterScrollViewer_BringIntoView_Scrolls_To_Caret times out waiting for the outer ScrollViewer to scroll further; the split waits added in b085d50 show the TextBox does grow, so it is the scroll that never settles (#24157). It was measured failing 6 times in 10 master builds. Both excluded on Skia-WASM only, each citing the issue that must re-enable it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CtggTSug9EusdhVcNXXjnH
Measured failing 5 times in 10 master builds in the commit that instrumented it (b085d50) and tracked since as #24147, but never actually excluded -- only the Native exclusion from #23269 was in place. It passed in build 229119 by luck while its weaker sibling When_Incremental_Load_Default failed, so it was left one unlucky rerun away from reddening CI again. Polling is not the fix here: the test already waits for the condition, then waits for composition animations, then samples with nothing awaited in between, and the materialized index regresses during that settle. Joins the Native exclusion rather than replacing it, citing #24147 as the issue that must re-enable it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CtggTSug9EusdhVcNXXjnH
|
Claude finished @MartinZikmund's task in 3m 28s —— View job Review: test: Disable three Skia-WASM flakes behind tracked issuesVerdict: ✅ ship (3 nits, no blockers) The mechanism is correct throughout: [PlatformCondition(ConditionMode.Exclude, RuntimeTestPlatforms.SkiaWasm)] is the right attribute to skip on Skia-WASM only, all three remaining targets (Desktop Skia, Skia-Android, Skia-iOS) are unaffected, and the flags-OR pattern for accumulating excluded platforms on When_Incremental_Load_ShouldStop is correct. Issues by severity:
Cross-cutting: PR title follows Conventional Commits (test:), all three tracking issues (#24156, #24157, #24147) are cited, and the PR description includes solid reproduction evidence. No product code changed.
|
Sorry, something went wrong.
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24158/wasm-skia-net9/index.html |
Sorry, something went wrong.
|
The build 229387 found UI Test snapshots differences: skia-linux-screenshots: 57, skia-windows-screenshots: 2348, wasm: 11 Details
|
Sorry, something went wrong.
GitHub Issue: #24156, #24157, #24147
PR Type:
🏗️ Build or CI related changes
What changed? 🚀
Three runtime tests are excluded on Skia-WASM only, each citing the issue that must re-enable it.
Two of them were caught failing WebAssembly Skia Runtime Tests 0 and 2 (one failure per job) in CI build 229119. In that same build both ran green on every other runtime-test target — Desktop Skia Windows/Linux/macOS, Skia-Android (5 shards) and Skia-iOS (4 shards) — so this is a slow-runtime settle race on the WASM runtime, not a product regression. The third is a long-tracked flake that happened to pass in that build.
Given_ListViewBase.When_Incremental_Load_Default (#24156)
The last materialized index moved backwards between two identical scroll-to-bottom operations, so the second sample lands mid-recycle. The test samples after a fixed Task.Delay(500) rather than polling for a settled state.
Given_TextBox.When_OuterScrollViewer_BringIntoView_Scrolls_To_Caret (#24157)
The waits here were split by failure mode in b085d50c87b precisely because this test was measured failing 6 times in 10 master builds, always as a timeout. That instrumentation now pays off: the extent wait passes, so the TextBox does grow and it is the scroll that never settles.
Given_ListViewBase.When_Incremental_Load_ShouldStop (#24147)
Measured failing 5 times in 10 master builds in the same commit that instrumented it, and tracked as #24147 since, but never actually excluded — only the Native exclusion from #23269 was in place. It passed in build 229119 by luck while its weaker sibling When_Incremental_Load_Default failed, leaving it one unlucky rerun from reddening CI. Polling is not the fix: the test already waits for the condition, then waits for composition animations, then samples with nothing awaited in between, and the index regresses during that settle. Its exclusion joins the existing Native one rather than replacing it.
Same changes are going into feature/breakingchanges via the master-sync PR #24155, where the first two failures were surfaced.
PR Checklist ✅