Skip to content

Commit d5fe397

Browse files
khmyznikovLegendaryBlairCopilot
authored
Follow-up: UITest framework Next - stability improvements (#49242)
This pull request addresses several CI flakiness and reliability issues in the UI test automation harness, especially around foreground window handling and build artifact resolution. The main improvements ensure that UI interactions (like real mouse clicks) reliably target the correct window, even in complex CI and build environments, and that test code accurately locates module executables regardless of output layout. **Foreground window handling and input reliability:** * Added `Session.EnsureForeground()` and improved `WindowControl.TryBringToForeground()` to reliably raise the target window above others, defeating the Win32 foreground lock using `AttachThreadInput` and related APIs. This prevents coordinate-based clicks from landing on the wrong window, a common cause of CI test flakiness. (`src/common/UITestAutomation.Next/Session.cs`, `src/common/UITestAutomation.Next/WindowControl.cs`, `src/common/UITestAutomation.Next/Element/Element.cs`, `.github/skills/ui-tests-migration/references/ci-stability.md`, `src/modules/MeasureTool/Tests/ScreenRuler.UITests.Next/TestHelper.cs`) [[1]](diffhunk://#diff-713524f4ead9951000b3578248f2740088729981051b68c3fa3655ebb548f20fR91-R118) [[2]](diffhunk://#diff-60be6ec24a99f8974cff00414fc47ea1e60e3ab9aee706b5d6845eb8df19165fR59-R76) [[3]](diffhunk://#diff-60be6ec24a99f8974cff00414fc47ea1e60e3ab9aee706b5d6845eb8df19165fL247-R332) [[4]](diffhunk://#diff-3c00581cb1d2b6a4302b8378b87c27cdad2f87c2aefc37eec0bdc6c39af9291aR100-R103) [[5]](diffhunk://#diff-98e5aea12baaac99aba10e7dd341ed3f2efdd24cb80b29306bb500816de09e2fR96-R114) [[6]](diffhunk://#diff-d8766b48614fe4a99e3f100e69e611ca2464de6ba0be257586c12d51f9278ed0L498-R504) **Build artifact and executable path resolution:** * Refactored `ModuleInfo.GetDevelopmentPath()` to dynamically walk up from the test assembly and find the module executable in various build output layouts, including CI artifacts and local builds. This eliminates hardcoded path offsets and makes test launches robust to different build structures. (`src/common/UITestAutomation/ModuleInfo.cs`) * Updated `ModuleConfigData.GetModulePath()` to use the improved path resolution and fall back to installed builds if necessary, ensuring that test runs can always locate the correct executable. (`src/common/UITestAutomation/ModuleConfigData.cs`) * Simplified `SessionHelper` so that `locationPath` is always empty, leveraging the fact that module paths are now absolute. (`src/common/UITestAutomation/SessionHelper.cs`) [[1]](diffhunk://#diff-c001f0fd3432c8b6102c3120597fc48f24902eaa0d755f51adfa7646344b3833L7-L10) [[2]](diffhunk://#diff-c001f0fd3432c8b6102c3120597fc48f24902eaa0d755f51adfa7646344b3833L46-R54) These changes collectively improve test reliability, especially in CI environments, and make the harness more resilient to changes in build output structure. --------- Co-authored-by: Boliang Zhang (from Dev Box) <bozhang@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 18edd52 commit d5fe397

8 files changed

Lines changed: 205 additions & 23 deletions

File tree

.github/skills/ui-tests-migration/references/ci-stability.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,25 @@ navigating with `By.AccessibilityId(...).Click()`** (Recipe 1) — it's race-saf
9393
reach for a raw `MouseClick`/manual `MouseHelper` when the interaction genuinely needs real mouse input
9494
(and by then the window has settled).
9595

96+
**The second trap: a background-launched window comes up *behind* the foreground.** A physical click
97+
lands on whatever window is **topmost at those pixels** — not necessarily your target. When a module's
98+
overlay/toolbar is shown by a *background* process (the runner, reacting to a hotkey) while another
99+
window holds the foreground, Windows' **foreground lock** puts it *behind* that window — it's present,
100+
`IsWindowVisible`-true, and un-cloaked, yet occluded. A coordinate click then hits the covering window
101+
and looks **exactly** like the interactivity race, but it's occlusion. This is a prime "passes local,
102+
flakes on CI" cause: on CI the Settings window used to enable the module is still foreground when the
103+
overlay appears. The harness guards against it — `Element.Click()` calls `Session.EnsureForeground()`
104+
first, which raises the target with the foreground-lock-defeating `AttachThreadInput` dance
105+
(`WindowControl.TryBringToForeground`) before the real click, and still falls back to `Invoke()` if the
106+
raise doesn't take. Diagnose it via winappcli's `isForeground` flag on `list-windows`; UIA `invoke` is
107+
immune because it never touches coordinates.
108+
109+
**Elevation must match.** Injected input — a synthetic hotkey *or* a real click — from a process at a
110+
*different* integrity level than the PowerToys runner is blocked by UIPI: a non-elevated host can't
111+
drive an elevated runner, and an elevated host's foreground window blocks the non-elevated runner's
112+
hook. Run the test host at the **same** elevation as the runner (the `.Next` harness launches the
113+
runner non-elevated, so run the tests non-elevated too).
114+
96115
---
97116

98117
## Principle 3 — Screen-capture (WGC) modules: cold-start + don't disturb the session

src/common/UITestAutomation.Next/Element/Element.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ public virtual void Click(bool rightClick = false, int msPostAction = 200, int t
9797
{
9898
if (Width > 0 && Height > 0)
9999
{
100+
// Raise the target window first: a physical click lands on whatever window is topmost
101+
// at these pixels, and a background-launched overlay/toolbar is often placed behind the
102+
// window that held the foreground when it was triggered (the Win32 foreground lock).
103+
Owner!.EnsureForeground();
100104
MouseHelper.MoveTo(X + (Width / 2), Y + (Height / 2));
101105
Thread.Sleep(50);
102106
if (rightClick)

src/common/UITestAutomation.Next/Session.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,34 @@ private Session(PowerToysModule scope, string appNameOrPid, int pid, string proc
8888
ProcessName = processName;
8989
}
9090

91+
/// <summary>
92+
/// Best-effort: raise this session's target window to the foreground so a subsequent coordinate
93+
/// interaction (a real mouse click/drag) can't land on a window that happens to be covering it. A
94+
/// background-launched overlay/toolbar is often placed BEHIND whatever held the foreground when it
95+
/// was triggered (the Win32 foreground lock), so <see cref="Element.Click"/> calls this before its
96+
/// physical click. Window-scoped sessions raise their exact HWND; process-scoped sessions raise the
97+
/// app's first window. Never throws — a foreground failure must not fail the interaction (the click
98+
/// still has its coordinate-free <c>Invoke</c> fallback).
99+
/// </summary>
100+
public void EnsureForeground()
101+
{
102+
try
103+
{
104+
if (Scope == TargetScope.Window && WindowHandle != 0)
105+
{
106+
WindowControl.TryBringToForeground(new IntPtr(WindowHandle));
107+
}
108+
else if (Scope == TargetScope.Process)
109+
{
110+
WindowControl.TryFocusByApp(TargetValue);
111+
}
112+
}
113+
catch
114+
{
115+
// Best-effort foregrounding only.
116+
}
117+
}
118+
91119
/// <summary>
92120
/// Build a session scoped to a whole process via <c>winapp ... -a &lt;app&gt;</c>. Cheaper than
93121
/// resolving a HWND and ideal for the single-window-per-process case (e.g. Settings smoke

src/common/UITestAutomation.Next/WindowControl.cs

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ public static class WindowControl
5656
[return: MarshalAs(UnmanagedType.Bool)]
5757
private static extern bool GetWindowRect(IntPtr hWnd, out RECT lpRect);
5858

59+
[DllImport("user32.dll")]
60+
private static extern IntPtr GetForegroundWindow();
61+
62+
[DllImport("user32.dll", SetLastError = true)]
63+
[return: MarshalAs(UnmanagedType.Bool)]
64+
private static extern bool AttachThreadInput(uint idAttach, uint idAttachTo, [MarshalAs(UnmanagedType.Bool)] bool fAttach);
65+
66+
[DllImport("user32.dll", SetLastError = true)]
67+
[return: MarshalAs(UnmanagedType.Bool)]
68+
private static extern bool BringWindowToTop(IntPtr hWnd);
69+
70+
[DllImport("user32.dll")]
71+
[return: MarshalAs(UnmanagedType.Bool)]
72+
private static extern bool IsIconic(IntPtr hWnd);
73+
74+
[DllImport("kernel32.dll")]
75+
private static extern uint GetCurrentThreadId();
76+
5977
private delegate bool EnumWindowsProc(IntPtr hWnd, IntPtr lParam);
6078

6179
private const uint WM_CLOSE = 0x0010;
@@ -244,24 +262,74 @@ public static bool TryCloseByApp(string appNameOrPid, Func<WindowsFinder.WindowI
244262
/// Bring the first window owned by <paramref name="appNameOrPid"/> to the foreground.
245263
/// If the window is minimized it's first restored. Tolerant.
246264
/// </summary>
247-
public static bool TryFocusByApp(string appNameOrPid)
265+
/// <summary>
266+
/// Bring <paramref name="hwnd"/> to the foreground RELIABLY, defeating the Win32 foreground lock.
267+
/// A bare <c>SetForegroundWindow</c> from a process that isn't already the foreground is silently
268+
/// refused (SPI_SETFOREGROUNDLOCKTIMEOUT) — which is exactly what leaves a freshly-shown overlay /
269+
/// toolbar sitting BEHIND the window that held the foreground when it was triggered, so a coordinate
270+
/// click then lands on the covering window instead of the target. Briefly attaching our input queue
271+
/// to the current foreground thread lifts that restriction for the call. Best-effort; tolerant.
272+
/// </summary>
273+
public static bool TryBringToForeground(IntPtr hwnd)
248274
{
249275
try
250276
{
251-
var w = WindowsFinder.ListByApp(appNameOrPid).FirstOrDefault();
252-
if (w is null || w.Hwnd == 0)
277+
if (hwnd == IntPtr.Zero || !IsWindow(hwnd))
253278
{
254279
return false;
255280
}
256281

257-
var hwnd = new IntPtr(w.Hwnd);
258-
if (!IsWindow(hwnd))
282+
var foreground = GetForegroundWindow();
283+
if (foreground == hwnd)
284+
{
285+
// Already the foreground window — don't touch its show-state or the input queues.
286+
return true;
287+
}
288+
289+
var foregroundThread = foreground == IntPtr.Zero ? 0u : GetWindowThreadProcessId(foreground, out _);
290+
var currentThread = GetCurrentThreadId();
291+
292+
// Only un-MINIMIZE. Do NOT SW_RESTORE a maximized window — that un-maximizes it, resizing the
293+
// window and invalidating any element coordinates already resolved for the click (the cause of
294+
// the arm64 "Settings got resized then the toggle click missed" flakiness).
295+
if (IsIconic(hwnd))
296+
{
297+
ShowWindow(hwnd, SW_RESTORE);
298+
}
299+
300+
var attached = foregroundThread != 0 && foregroundThread != currentThread;
301+
if (attached)
302+
{
303+
AttachThreadInput(currentThread, foregroundThread, true);
304+
}
305+
306+
BringWindowToTop(hwnd);
307+
var ok = SetForegroundWindow(hwnd);
308+
309+
if (attached)
310+
{
311+
AttachThreadInput(currentThread, foregroundThread, false);
312+
}
313+
314+
return ok;
315+
}
316+
catch
317+
{
318+
return false;
319+
}
320+
}
321+
322+
public static bool TryFocusByApp(string appNameOrPid)
323+
{
324+
try
325+
{
326+
var w = WindowsFinder.ListByApp(appNameOrPid).FirstOrDefault();
327+
if (w is null || w.Hwnd == 0)
259328
{
260329
return false;
261330
}
262331

263-
ShowWindow(hwnd, SW_RESTORE);
264-
return SetForegroundWindow(hwnd);
332+
return TryBringToForeground(new IntPtr(w.Hwnd));
265333
}
266334
catch
267335
{

src/common/UITestAutomation/ModuleConfigData.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,17 @@ public string GetModulePath(PowerToysModule scope)
153153
}
154154
}
155155

156-
return moduleInfo.GetDevelopmentPath();
156+
var developmentPath = moduleInfo.GetDevelopmentPath();
157+
if (File.Exists(developmentPath))
158+
{
159+
return developmentPath;
160+
}
161+
162+
// Last resort: an installed build if one is present, so a slim/installed CI layout that didn't
163+
// set useInstallerForTest still resolves. Returns the development path either way so a launch
164+
// failure names a concrete location.
165+
var installedFallback = moduleInfo.GetInstalledPath(GetPowerToysInstallPath());
166+
return File.Exists(installedFallback) ? installedFallback : developmentPath;
157167
}
158168

159169
public string GetWindowsApplicationDriverUrl() => WindowsApplicationDriverUrl;

src/common/UITestAutomation/ModuleInfo.cs

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,69 @@ public ModuleInfo(string executableName, string windowName, string? subDirectory
2626
}
2727

2828
/// <summary>
29-
/// Gets the relative development path for this module
29+
/// Resolves the ABSOLUTE path to this module's executable in the build under test by walking up
30+
/// from the test assembly's output folder and returning the first ancestor that actually contains
31+
/// the exe. This adapts to any build-output nesting — the local flattened
32+
/// <c>&lt;root&gt;\&lt;plat&gt;\&lt;cfg&gt;\tests\&lt;project&gt;\&lt;tfm&gt;\</c> layout AND the deeper
33+
/// per-project CI artifact layout — instead of hard-coding the walk-up depth. The old fixed
34+
/// <c>\..\..\..</c> offset resolved to a non-existent path on CI, so every legacy test failed to
35+
/// launch PowerToys there; this mirrors the .Next harness's resolver. When no ancestor holds the
36+
/// exe, it returns the historical fixed-depth path (made absolute) so a launch failure still names
37+
/// a concrete location.
3038
/// </summary>
3139
public string GetDevelopmentPath()
3240
{
33-
// The test assembly normally lives in <buildRoot>\tests\<project>\<tfm>\, so the build
34-
// output root that holds the module exe is three levels above it. When a test project is
35-
// built with a RuntimeIdentifier (OutputType=Exe for the MTP runner) the output gains an
36-
// extra RID subfolder (<tfm>\win-x64\ or \win-arm64\), pushing the root one level further
37-
// up. Detect that case so the relative path stays correct in both layouts.
38-
string prefix = IsRuntimeIdentifierOutputFolder() ? @"\..\..\..\.." : @"\..\..\..";
41+
// 1) Walk up and return the first ancestor that DIRECTLY holds the exe. Covers the flattened
42+
// <root>\<plat>\<cfg>\tests\<project>\<tfm>\ layout and the CI artifact layout where the
43+
// product's output dir is itself an ancestor of the (deeply nested) test assembly.
44+
for (var dir = new DirectoryInfo(AppContext.BaseDirectory); dir is not null; dir = dir.Parent)
45+
{
46+
var candidate = ComposeUnder(dir.FullName);
47+
if (File.Exists(candidate))
48+
{
49+
return candidate;
50+
}
51+
}
3952

40-
if (string.IsNullOrEmpty(SubDirectory))
53+
// 2) Per-project NESTED layout: the test builds under <repoRoot>\src\...\<project>\<plat>\<cfg>\
54+
// while the product sits at <repoRoot>\<plat>\<cfg>\ — not a direct ancestor. Probe the
55+
// conventional <plat>\<cfg> output beneath each ancestor, preferring this run's own.
56+
foreach (var platCfg in BuildOutputSubdirs())
4157
{
42-
return $@"{prefix}\{ExecutableName}";
58+
for (var dir = new DirectoryInfo(AppContext.BaseDirectory); dir is not null; dir = dir.Parent)
59+
{
60+
var candidate = ComposeUnder(Path.Combine(dir.FullName, platCfg));
61+
if (File.Exists(candidate))
62+
{
63+
return candidate;
64+
}
65+
}
4366
}
4467

45-
return $@"{prefix}\{SubDirectory}\{ExecutableName}";
68+
// 3) Fallback: the historical fixed-depth walk-up, resolved to an absolute path so a launch
69+
// failure still names a concrete location instead of a bare relative fragment.
70+
var prefix = IsRuntimeIdentifierOutputFolder() ? @"..\..\..\.." : @"..\..\..";
71+
var baseDir = AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
72+
return Path.GetFullPath(Path.Combine(baseDir, prefix, RelativeExe()));
73+
}
74+
75+
// The exe (optionally under its module subdirectory) directly beneath a build-output root.
76+
private string ComposeUnder(string root) =>
77+
string.IsNullOrEmpty(SubDirectory)
78+
? Path.Combine(root, ExecutableName)
79+
: Path.Combine(root, SubDirectory, ExecutableName);
80+
81+
private string RelativeExe() =>
82+
string.IsNullOrEmpty(SubDirectory) ? ExecutableName : Path.Combine(SubDirectory, ExecutableName);
83+
84+
// Conventional <plat>\<cfg> build-output subdirs, ordered to prefer this test run's own so a
85+
// Debug run resolves the Debug product even when a stale Release build is also present.
86+
private static string[] BuildOutputSubdirs()
87+
{
88+
var all = new[] { @"x64\Debug", @"x64\Release", @"ARM64\Debug", @"ARM64\Release" };
89+
var baseDir = AppContext.BaseDirectory.Replace('/', '\\');
90+
var current = all.FirstOrDefault(pc => baseDir.Contains("\\" + pc + "\\", StringComparison.OrdinalIgnoreCase));
91+
return current is null ? all : new[] { current }.Concat(all.Where(pc => pc != current)).ToArray();
4692
}
4793

4894
// True when the executing assembly sits in a RID-specific output subfolder (e.g. ...\<tfm>\win-x64),

src/common/UITestAutomation/SessionHelper.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44

55
using System;
66
using System.Diagnostics;
7-
using System.Diagnostics.CodeAnalysis;
87
using System.IO;
98
using System.Linq;
10-
using System.Reflection;
119
using System.Threading.Tasks;
1210
using Microsoft.VisualStudio.TestTools.UnitTesting;
1311
using OpenQA.Selenium.Appium;
@@ -43,14 +41,17 @@ public class SessionHelper
4341
/// </summary>
4442
private bool UseInstallerForTest { get; }
4543

46-
[UnconditionalSuppressMessage("SingleFile", "IL3000:Avoid accessing Assembly file path when publishing as a single file", Justification = "<Pending>")]
4744
public SessionHelper(PowerToysModule scope, string[]? commandLineArgs = null)
4845
{
4946
this.scope = scope;
5047
this.commandLineArgs = commandLineArgs;
5148
this.sessionPath = ModuleConfigData.Instance.GetModulePath(scope);
5249
UseInstallerForTest = EnvironmentConfig.UseInstallerForTest;
53-
this.locationPath = UseInstallerForTest ? string.Empty : Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
50+
51+
// GetModulePath now returns an ABSOLUTE path (adaptive dev-build walk-up or the installed
52+
// path), so no per-assembly prefix is needed; locationPath stays empty and the launch-path
53+
// concatenations below resolve to that absolute path.
54+
this.locationPath = string.Empty;
5455

5556
CheckWinAppDriverAndRoot();
5657
}

src/modules/MeasureTool/Tests/ScreenRuler.UITests.Next/TestHelper.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,13 @@ private static void SelectToolAndVerify(Session ruler, string buttonId, string t
495495
{
496496
var btnX = button.X + (button.Width / 2);
497497
var btnY = button.Y + (button.Height / 2);
498-
Log($"SelectToolAndVerify[{testName}]: attempt {attempt}: located {buttonId} at ({button.X},{button.Y}) {button.Width}x{button.Height} (offscreen={button.IsOffscreen}, toggle={toggleState}); real mouse click at ({btnX},{btnY})");
498+
Log($"SelectToolAndVerify[{testName}]: attempt {attempt}: located {buttonId} at ({button.X},{button.Y}) {button.Width}x{button.Height} (offscreen={button.IsOffscreen}, toggle={toggleState}); raising toolbar to foreground then real mouse click at ({btnX},{btnY})");
499+
500+
// Raise the toolbar to the foreground first so the real click can't land on a window
501+
// occluding it (the Win32 foreground lock). Bounds selection uses a manual MouseHelper
502+
// click rather than Element.Click, so — unlike spacing — it needs this guard explicitly.
503+
// TryBringToForeground only un-minimizes (IsIconic), so a maximized toolbar isn't resized.
504+
ruler.EnsureForeground();
499505
MouseHelper.MoveTo(btnX, btnY);
500506
Thread.Sleep(200);
501507
MouseHelper.LeftClick();

0 commit comments

Comments
 (0)