[ci-fix-net11] Needs review: de-flake ActiveWindowTracker Windows device tests — bounded condition wait instead of fixed 100 ms delay (refs #36 [Content truncated due to length] - #36092
Conversation
The ActiveWindowTracker Windows device tests post an asynchronous window message (PostMessage) and then assert it was received after a fixed 100 ms delay (PostTestMessageAsync). Under CI load the message pump can take longer than 100 ms to dispatch the posted message, so the positive assertions intermittently fail with an empty collection - e.g. SwitchingWindowsPostsToTheNewWindow (#36043), seen in 2 of the last 10 net11.0 maui-pr-devicetests builds. Replace the fixed-delay-then-assert pattern with a bounded condition wait (WaitForMessageAsync) for the three positive-assertion tests. The wait returns as soon as the expected message is observed and otherwise times out, so a genuine "message never delivered" defect still fails the unchanged assertions. Negative-assertion tests keep the fixed delay, since absence cannot be polled for. Refs: #36043 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@github-actions[bot] — new AI review results are available based on this last commit:
eee09e3. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ⚠️ INCONCLUSIVE
Platform: ANDROID
⚠️ verify-tests-fail.ps1exited before writing a verification report. Diagnostics below.
Exit code: 3
Auto-detected:
- Test filter:
Windows_ActiveWindowTracker_Tests
Likely cause:
- Device/emulator setup failed (env error class).
Artifacts written before exit:
test-failure-BaseWindowMessageManager_Tests.log(287 KB)test-failure-BaseWindowMessageManager_Tests.log.attempt2(449.5 KB)test-failure-Windows_ActiveWindowTracker_Tests.log(400 KB)verification-log.txt(0.2 KB)
Gate output log (last 60 lines)
Starting android Device/Emulator
═══════════════════════════════════════════════════════════
✓ Device ready: emulator-5554
═══════════════════════════════════════════════════════════
Running Tests
═══════════════════════════════════════════════════════════
Running: dotnet xharness android test --app /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net11.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk --package-name com.microsoft.maui.essentials.devicetests --device-id emulator-5554 -o artifacts/log --timeout 01:00:00 -v --arg TestFilter=BaseWindowMessageManager_Tests
Target: android-emulator-64
Device: emulator-5554
═══════════════════════════════════════════════════════════
Test Results
═══════════════════════════════════════════════════════════
═══════════════════════════════════════════════════════════
═══════════════════════════════════════════════════════════
─────────────────────────────────────────────────
📱 Test 2/2: [DeviceTest] Windows_ActiveWindowTracker_Tests
🧪 Running device tests: Essentials on android
Filter: Windows_ActiveWindowTracker_Tests
═══════════════════════════════════════════════════════════
MAUI Device Tests Runner
═══════════════════════════════════════════════════════════
✓ xharness found: local dotnet tool
✓ dotnet found: /home/vsts/work/1/s/.dotnet/dotnet
Project: Essentials
Project Path: src/Essentials/test/DeviceTests/Essentials.DeviceTests.csproj
Platform: android
Configuration: Release
Test Filter: Windows_ActiveWindowTracker_Tests
═══════════════════════════════════════════════════════════
Building Essentials Device Tests for android
═══════════════════════════════════════════════════════════
Running: dotnet build src/Essentials/test/DeviceTests/Essentials.DeviceTests.csproj -c Release -f net11.0-android /p:TreatWarningsAsErrors=false /p:AndroidPackageFormat=apk
✓ Build succeeded
✓ App found: /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net11.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk
═══════════════════════════════════════════════════════════
Starting android Device/Emulator
═══════════════════════════════════════════════════════════
✓ Device ready: emulator-5554
═══════════════════════════════════════════════════════════
Running Tests
═══════════════════════════════════════════════════════════
Running: dotnet xharness android test --app /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net11.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk --package-name com.microsoft.maui.essentials.devicetests --device-id emulator-5554 -o artifacts/log --timeout 01:00:00 -v --arg TestFilter=Windows_ActiveWindowTracker_Tests
Target: android-emulator-64
Device: emulator-5554
═══════════════════════════════════════════════════════════
Test Results
═══════════════════════════════════════════════════════════
═══════════════════════════════════════════════════════════
═══════════════════════════════════════════════════════════
==========================================
VERIFICATION RESULTS
==========================================
📱 [DeviceTest] BaseWindowMessageManager_Tests: PASSED ❌ (should fail!)
📱 [DeviceTest] Windows_ActiveWindowTracker_Tests: PASSED ❌ (should fail!)
╔═══════════════════════════════════════════════════════════╗
║ VERIFICATION FAILED ❌ ║
╠═══════════════════════════════════════════════════════════╣
║ 2/2 test(s) PASSED but should FAIL! ║
║ Those tests don't reproduce the bug. Revise them! ║
╚═══════════════════════════════════════════════════════════╝
📱 UI Tests — Essentials
Detected UI test categories: Essentials
⏭️ Deep UI tests — 0 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above). 1 category reported 0 tests.
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Essentials |
0 tests | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
📋 Pre-Flight — Context & Validation
Issue: #36043 - [ci-scan-net11] Essentials.DeviceTests: SwitchingWindowsPostsToTheNewWindow fails on Windows — Assert.Contains collection empty (net11.0)
PR: #36092 - [ci-fix] Needs review: de-flake ActiveWindowTracker Windows device tests — bounded condition wait instead of fixed 100 ms delay
Platforms Affected: Windows device tests. Requested try-fix test platform: android, which cannot execute these Windows-only tests.
Files Changed: 0 implementation, 2 test
Key Findings
- The reported failure is in
SwitchingWindowsPostsToTheNewWindow, where a positive assertion sometimes observes an empty collection afterPostMessageplus a fixed 100 ms delay. - PR #36092 keeps assertions intact and replaces fixed-delay positive checks with a bounded polling helper in the three positive
ActiveWindowTrackertests. - The changed tests are Windows-only (
Tests/Windows, WinUIWindow,user32.dll), so Android execution is not a meaningful regression gate. - No prior PR review comments or inline review findings were available from public GitHub API data.
- GitHub CLI is unauthenticated in this environment; required-check and Azure DevOps test status could not be fully classified.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- No concrete code-level issues found in the PR diff.
- CI/gate status is undetermined because
gh pr checks --requiredis unavailable and the prior gate result was already inconclusive. - Blast radius is limited to Windows Essentials device tests; no product runtime or public API changes.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36092 | Add WaitForMessageAsync(Func<bool>) polling and await expected message before positive assertions. |
WARNING INCONCLUSIVE (prior gate could not build/run) | ActiveWindowTracker_Tests.cs, BaseWindowMessageManager_Tests.cs |
Original PR fix; code review found no concrete issues. |
🔬 Code Review — Deep Analysis
Code Review — PR #36092
Independent Assessment
What this changes: Adds a bounded polling helper for Windows PostMessage delivery and uses it in three positive ActiveWindowTracker device tests before asserting.
Inferred motivation: De-flake tests that assert before the Win32 message pump dispatches the posted message.
Reconciliation with PR Narrative
Author claims: Fixes intermittent Windows device-test failures from fixed 100 ms delay after async PostMessage.
Agreement/disagreement: Agrees. The code preserves assertions and waits only for positive delivery cases.
Prior Review Reconciliation
No prior error findings found.
Blast Radius Assessment
- Runs for all instances: no — test-only helper.
- Startup impact: no.
- Static/shared state: no.
CI Status
- Required-check result:
gh pr checks --requiredunavailable: gh is unauthenticated. - Public check-run result:
maui-prfailed; failures include macOS AOT and Windows Helix Unit Tests Debug. - Classification: undetermined.
- Action taken:
ci-analysisunavailable; confidence capped low.
Findings
No Error, Warning, or Suggestion findings. Expert reviewer also reported no important findings.
Failure-Mode Probing
- Message arrives after 100 ms: helper yields and keeps polling until observed.
- Message never arrives: final
Assert.Containsstill fails after timeout. - Negative tests: unchanged; absence assertions still use existing fixed delay.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code change looks sound and localized, but CI is red/undetermined and required checks could not be verified with gh auth unavailable. Per skill rules, this cannot be LGTM until CI is classified or rerun.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | TaskCompletionSource event signal from tracker.WindowMessage. |
BLOCKED | 2 files | More precise than polling, but more boilerplate and cleanup. Android cannot run Windows-only regression. |
| 2 | try-fix | Replace test PostMessage with synchronous SendMessage. |
BLOCKED | 2 files | Most deterministic and fastest, but lowers fidelity to queued message delivery. Android cannot run Windows-only regression. |
| 3 | try-fix | Await DispatcherQueue barrier turns after queued PostMessage. |
BLOCKED | 2 files | Preserves queued delivery but relies on Win32/DispatcherQueue ordering; not clearly stronger than polling. Android cannot run Windows-only regression. |
| 4 | try-fix | Add internal synthetic dispatch seam and test tracker state directly. | BLOCKED | 3 files | Deterministic abstraction-boundary fix, but adds production test seam and reduces end-to-end coverage. Android cannot run Windows-only regression. |
| PR | PR #36092 | Bounded condition wait after PostMessage for positive assertions. |
WARNING INCONCLUSIVE (prior gate) | 2 files | Original PR; simplest test-only change and code review found no concrete issue. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| gpt-5.5 / maui-expert-reviewer | 2 | Yes | Proposed a synthetic ActiveWindowTracker dispatch seam; captured as try-fix-4. |
| final assessment | 2 | No | Remaining options are trivial variations of polling, event signaling, synchronous dispatch, or test-seam abstraction split. |
Exhausted: Yes
Selected Fix: PR's fix — The alternatives are plausible but none could be validated on the requested Android platform, and none is demonstrably better on code merits alone. Candidate 2 is the most deterministic but changes test fidelity; candidate 4 changes production internals for a test seam. The PR's bounded wait is the smallest localized test-only de-flake and preserves existing assertions.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current description is detailed and accurate, but the title has CI-agent noise and does not match the [Platform] Component: What changed title formula.
Recommended title
[Windows] ActiveWindowTracker tests: Wait for posted messages before asserting
Recommended description
**Target branch:** net11.0
**Refs:** dotnet/maui#36043
**Attempt:** 1/5
## Summary
The `ActiveWindowTracker` Windows device tests intermittently fail with an empty-collection assertion — e.g. `SwitchingWindowsPostsToTheNewWindow` failing as `Assert.Contains() Failure ... Collection: []`. This is a **test-quality flake** (Step 4.7 bucket **b**), not a product defect.
## Root cause
`BaseWindowMessageManager_Tests.PostTestMessageAsync` posts an **asynchronous** window message via `PostMessage` and then waits a **fixed `await Task.Delay(100)`** before the test asserts the message was received. `PostMessage` only delivers once the message pump dispatches it; under CI load that can exceed 100 ms, so the positive assertions occasionally run before the message arrives and observe an empty collection.
Observed intermittently (green-on-retry) in 2 of the last ~10 `net11.0` `maui-pr-devicetests` builds (1473153, 1455596) — the classic flaky signal.
## Fix (de-flake — not a mute)
Replace the fixed-delay-then-assert pattern with a **bounded condition wait** (`WaitForMessageAsync`) for the three positive-assertion tests:
- `ActivatedBeforeStartRecievesMessage`
- `StartBeforeActivatedRecievesMessage`
- `SwitchingWindowsPostsToTheNewWindow`
The helper polls (yielding to the message pump) until the expected message is observed, up to a generous 5 s ceiling. It returns **as soon as** the message arrives, so there is no artificial slowdown, and if the message is **never** delivered (a genuine regression) the wait times out and the **unchanged** `Assert.Contains` still fails. Negative-assertion tests (`StoppedTrackerDoesNotTrack`, `SwitchingWindowsDoesNotPostToTheOldWindow`) intentionally keep the fixed delay, since absence cannot be polled for.
### Why this is not a mute
- No assertions removed or weakened — every original `Assert.Contains` / `Assert.DoesNotContain` is intact.
- No `[Retry]` / `[Repeat]` / `[ActiveIssue]` / `[Ignore]` / category exclusion / timeout-hiding change added.
- A real "message never delivered" regression still fails the test after the ceiling, so the change cannot hide a product bug.
## Validation
**Not runner-validated.** These are Windows-only device tests (`net11.0-windows`, `user32.dll` `PostMessage` + `Microsoft.UI.Xaml.Window`) and this fixer runs on a Linux runner with no Windows workload, so the `Essentials.DeviceTests` Windows path cannot be built or executed here. Please validate via a `maui-pr-devicetests` run on Windows (or run the Windows `ActiveWindowTracker` device tests locally) before merging.
## Files changed
- `src/Essentials/test/DeviceTests/Tests/Windows/BaseWindowMessageManager_Tests.cs` — add the `WaitForMessageAsync` condition-wait helper.
- `src/Essentials/test/DeviceTests/Tests/Windows/ActiveWindowTracker_Tests.cs` — await the condition before the three positive assertions.
🏁 Report — Final Recommendation
Comparative Candidate Report — PR #36092
Candidates Compared
| Candidate | Approach | Regression status | Code-merit assessment | Rank |
|---|---|---|---|---|
pr |
Keep PostMessage; add bounded condition wait before positive Assert.Contains checks. |
Best balance: minimal test-only change, preserves queued message fidelity, preserves assertions, and directly waits for the assertion condition. | 1 | |
pr-plus-reviewer |
Same as pr; expert reviewer produced no actionable changes. |
Equivalent to pr; no reviewer patch to distinguish it from the submitted PR. |
1 (tie, no-op variant) | |
try-fix-1 |
Add a temporary TaskCompletionSource event signal completed by tracker.WindowMessage. |
BLOCKED on platform; not regression-validated. | Precise and plausible, but adds per-test ceremony and cleanup risk without demonstrating a meaningful advantage over direct condition polling. | 3 |
try-fix-2 |
Replace async PostMessage with synchronous SendMessage. |
BLOCKED on platform; not regression-validated. | Most deterministic, but changes test fidelity by removing queued Win32 message delivery from the test path. | 2 |
try-fix-3 |
Drain DispatcherQueue barrier turns after PostMessage. |
BLOCKED on platform; not regression-validated. | Preserves queued delivery, but depends on ordering between Win32 posted messages and dispatcher queue callbacks; not clearly stronger than waiting for the observed condition. | 4 |
try-fix-4 |
Add an internal synthetic dispatch seam and test tracker routing directly. | BLOCKED on platform; not regression-validated. | Deterministic at the abstraction boundary, but adds production test seam and reduces end-to-end coverage in these tests. | 5 |
Regression-Test Ordering
No candidate has a validated passing Windows regression result in this environment. The gate is explicitly inconclusive, and the try-fix candidates are blocked because the requested platform was Android while the affected tests are Windows-only. No candidate is ranked as "passing"; no candidate is ranked lower due to a confirmed regression-test failure.
Winner
Winner: pr
The raw PR fix wins because it is the smallest localized de-flake that preserves the intended end-to-end PostMessage coverage and keeps all existing assertions intact. pr-plus-reviewer is code-identical because the expert reviewer produced no actionable findings, while the try-fix alternatives either add complexity, reduce fidelity, or introduce production test seams without validation evidence that they are better.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
|
/azp run maui-pr-uitests |
|
/azp run maui-pr-devicetests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
🎯 Target de-flake validated green — marking ready for review
The remaining red legs are unrelated pre-existing iOS/MacCatalyst Essentials FileSystem failures ( Automated by the CI Failure Fixer (net11.0). A maintainer should confirm before merge.
|
|
Target test SwitchingWindowsPostsToTheNewWindow validated green on eee09e3 on the only platform it runs on (Windows). Fresh maui-pr-devicetests build 1511744 Helix testResults.xml shows result="Pass" for both packaged and unpackaged runs, and all five ActiveWindowTracker tests pass. Remaining red legs are unrelated pre-existing iOS/MacCatalyst Essentials FileSystem failures, orthogonal to this Windows-only de-flake. Marking ready for maintainer review.
|
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
kubaflo
left a comment
There was a problem hiding this comment.
🔍 AI-generated review — automated multi-model consensus (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), independent reviews cross-pollinated and synthesized. Posted on behalf of @kubaflo.
Verdict: LGTM (code) — 3-model consensus, high confidence
What this changes: Adds a non-throwing poll helper WaitForMessageAsync(Func<bool>, timeoutMs = 5000, pollMs = 50) to BaseWindowMessageManager_Tests, and awaits it before the Assert.Contains(TEST_MESSAGE, …) checks in three ActiveWindowTracker_Tests cases (replacing reliance on a single fixed post-message delay).
Why it's a sound de-flake (verified):
- Failure detection preserved. The helper never throws on timeout — it just returns; the caller's
Assert.Containsis still the correctness anchor and fails genuine non-delivery (after polling the full 5 s). - No weakened assertion. In
SwitchingWindowsPostsToTheNewWindow,TEST_MESSAGEis only ever posted towindow2, and dispatch is per-HWND, so waiting onwindow2cannot mask theDoesNotContain(window1…)check — which now runs later, giving strictly more time to catch a spurious entry. - No data race.
messagesis a plainList<…>, but both the WndProc append (OnWindowMessage) and theContainspoll execute on the single WinUI UI thread (the window HWND is owned by it;await Task.Delayresumes on the sameDispatcherQueueSynchronizationContext).Containscompletes synchronously betweenawaitpoints, so no concurrent enumeration/mutation. - Budget: ≤ 100 iterations; happy path resolves in ~100–150 ms, only genuine failures pay the 5 s.
CI status
maui-pr-devicetests● red,maui-pr-uitests● red,maui-pr● pending.- Classification: failures confirmed PR-unrelated. The red device tests are
CheckFileResultOpenReadAsyncMultipleTimes/OpenAppPackageFileAsync_Can_Load_Filefailing withSystem.NullReferenceException : Arg_NullReferenceException— the known dotnet/runtime#129813 net11 runtime-async miscompile inStream.ReadAsync, notActiveWindowTracker. This PR is a Windows-only test change and cannot affect those FileSystem tests. - ℹ Final merge gating (green
maui-pr) remains a human call.
Note for @kubaflo: LGTM — correct, race-free de-flake. Not formally approving (automated-loop policy).
|
♻️ Attempt 1/10 — red is an unrelated CI flake, not caused by this PR (head The failing leg is A maintainer re-run (
|
kubaflo
left a comment
There was a problem hiding this comment.
LGTM — approving.
This PR replaces a fixed 100 ms delay with a bounded condition-wait in the Windows Essentials device tests (ActiveWindowTracker_Tests.cs, BaseWindowMessageManager_Tests.cs). The standing 3-model review (Opus 4.8 / GPT-5.5 / Gemini 3.1 Pro) reached LGTM.
CI-gate: the changed surface — maui-pr-devicetests (net11.0 Windows Helix Tests → Run DeviceTests Windows) passes, along with every Windows build/pack/unit/integration leg. The red legs are all unrelated:
Build macOS (Release)fails insideMaui.Controls.Sample.csprojbuilding for net11.0-ios (CS0246 'Android' not found,CS0115 OnAppearing/OnNavigatedTo no suitable method to override) — an orthogonal, transient net11.0 sample-app build breakage that a Windows-only Essentials-test change cannot cause.MacCatalyst/iOSCoreCLR device-test reds are the known net11 CoreCLR flake set (dotnet/runtime#129813); the remaining reds are non-Windows UI-test flakes.
🤖 AI-generated review (GitHub Copilot CLI, on behalf of @kubaflo).
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Target branch: net11.0
Refs: #36043
Attempt: 1/5
Summary
The
ActiveWindowTrackerWindows device tests intermittently fail with an empty-collection assertion — e.g.SwitchingWindowsPostsToTheNewWindowfailing asAssert.Contains() Failure ... Collection: []. This is a test-quality flake (Step 4.7 bucket b), not a product defect.Root cause
BaseWindowMessageManager_Tests.PostTestMessageAsyncposts an asynchronous window message viaPostMessageand then waits a fixedawait Task.Delay(100)before the test asserts the message was received.PostMessageonly delivers once the message pump dispatches it; under CI load that can exceed 100 ms, so the positive assertions occasionally run before the message arrives and observe an empty collection.Observed intermittently (green-on-retry) in 2 of the last ~10
net11.0maui-pr-devicetestsbuilds (1473153, 1455596) — the classic flaky signal.Fix (de-flake — not a mute)
Replace the fixed-delay-then-assert pattern with a bounded condition wait (
WaitForMessageAsync) for the three positive-assertion tests:ActivatedBeforeStartRecievesMessageStartBeforeActivatedRecievesMessageSwitchingWindowsPostsToTheNewWindowThe helper polls (yielding to the message pump) until the expected message is observed, up to a generous 5 s ceiling. It returns as soon as the message arrives, so there is no artificial slowdown, and if the message is never delivered (a genuine regression) the wait times out and the unchanged
Assert.Containsstill fails. Negative-assertion tests (StoppedTrackerDoesNotTrack,SwitchingWindowsDoesNotPostToTheOldWindow) intentionally keep the fixed delay, since absence cannot be polled for.Why this is not a mute
Assert.Contains/Assert.DoesNotContainis intact.[Retry]/[Repeat]/[ActiveIssue]/[Ignore]/ category exclusion / timeout-hiding change added.Validation
Not runner-validated. These are Windows-only device tests (
net11.0-windows,user32.dllPostMessage+Microsoft.UI.Xaml.Window) and this fixer runs on a Linux runner with no Windows workload, so theEssentials.DeviceTestsproject cannot be built or executed here. Please validate via amaui-pr-devicetestsrun on Windows (or run the WindowsActiveWindowTrackerdevice tests locally) before merging.Files changed
src/Essentials/test/DeviceTests/Tests/Windows/BaseWindowMessageManager_Tests.cs— add theWaitForMessageAsynccondition-wait helper.src/Essentials/test/DeviceTests/Tests/Windows/ActiveWindowTracker_Tests.cs— await the condition before the three positive assertions.Generated by the CI Failure Fixer (net11.0-only). Target branch: net11.0 · Refs: #36043 · Attempt: 1/5