From 3ab2c12896f7d9f840a1901dadbf1c30a34026f6 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:01:21 -0600 Subject: [PATCH 1/9] test(ui): add FlaUI scaffold + main-window smoke test (#531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of the FlaUI UI-automation rollout. Adds FlaUI.Core + FlaUI.UIA3 (5.0.0) to Daqifi.Desktop.Test and a new UITests/ folder with: - MainWindowSmokeTest: launches DAQiFi.exe, waits for the main window, asserts the title contains "DAQiFi", then closes cleanly. Reports Assert.Inconclusive (SKIPPED, not FAILED) when the exe isn't built yet or when the runner isn't elevated (the WPF app's app.manifest declares requireAdministrator, which Process.Start can't satisfy without an already-elevated parent). - ConnectStreamDisconnectTest: Phase 2 happy-path stub (connect -> enable channel -> stream 3s -> stop -> disconnect). Marked [Ignore] pending a bench device + the XAML AutomationId annotations it references. AutomationIds use a "Daqifi.." namespace so future XAML hooks are greppable. No production code changed; existing 24 test files untouched. Phase 1 test status on a non-elevated dev box is SKIPPED — expected. To exercise the smoke test fully, either run dotnet test from an elevated terminal, or relax app.manifest to asInvoker for the UI-test build target. --- .../Daqifi.Desktop.Test.csproj | 2 + .../UITests/ConnectStreamDisconnectTest.cs | 184 ++++++++++++++++++ .../UITests/MainWindowSmokeTest.cs | 155 +++++++++++++++ 3 files changed, 341 insertions(+) create mode 100644 Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs create mode 100644 Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs diff --git a/Daqifi.Desktop.Test/Daqifi.Desktop.Test.csproj b/Daqifi.Desktop.Test/Daqifi.Desktop.Test.csproj index 59b47206..199718d1 100644 --- a/Daqifi.Desktop.Test/Daqifi.Desktop.Test.csproj +++ b/Daqifi.Desktop.Test/Daqifi.Desktop.Test.csproj @@ -11,6 +11,8 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all + + diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs new file mode 100644 index 00000000..73640229 --- /dev/null +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs @@ -0,0 +1,184 @@ +using System; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading; +using FlaUI.Core; +using FlaUI.Core.AutomationElements; +using FlaUI.Core.Conditions; +using FlaUI.Core.Definitions; +using FlaUI.UIA3; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Daqifi.Desktop.Test.UITests; + +/// +/// Phase 2 of the FlaUI UI-automation scaffold (issue #531). +/// +/// Drives the full connect -> enable-channel -> stream -> disconnect happy path +/// against a real DAQiFi device. This test is intentionally [Ignore]'d until a +/// device is wired to the bench so a normal CI run never touches it; remove the +/// Ignore attribute (or run the "UI-Bench" category explicitly) once: +/// +/// 1. A DAQiFi Nyquist is attached via USB (or reachable via Wi-Fi). +/// 2. The required XAML controls have been annotated with the AutomationIds +/// referenced below. Each Id has a comment in the XAML pointing back to +/// this test + issue #531 for traceability. +/// 3. The desktop app has been built (Phase 1 verifies the launch path). +/// +/// Naming convention used for AutomationIds: "Daqifi.<Pane>.<Control>", +/// e.g. "Daqifi.Connection.AddDeviceButton". Keeping a stable, dotted namespace +/// makes future selectors greppable in the XAML. +/// +[TestClass] +[Ignore("Phase 2 - needs bench device. See issue #531; remove [Ignore] when a Nyquist is on the bench and XAML AutomationIds are wired up.")] +public class ConnectStreamDisconnectTest +{ + private const string DesktopExeName = "DAQiFi.exe"; + private const string DesktopProjectName = "Daqifi.Desktop"; + + // AutomationIds that Phase 2 expects to exist in MainWindow / dialogs. + // None of these are wired up yet; add them in the corresponding XAML with + // a comment referencing this test + #531 when enabling the test. + private const string AddDeviceButtonId = "Daqifi.Connection.AddDeviceButton"; + private const string ConnectButtonId = "Daqifi.Connection.ConnectButton"; + private const string DeviceListId = "Daqifi.Devices.ConnectedList"; + private const string FirstChannelToggleId = "Daqifi.Channels.FirstChannelEnable"; + private const string StartStreamingId = "Daqifi.Streaming.StartButton"; + private const string StopStreamingId = "Daqifi.Streaming.StopButton"; + private const string DisconnectButtonId = "Daqifi.Connection.DisconnectButton"; + private const string LiveGraphId = "Daqifi.Graph.Live"; + + private static readonly TimeSpan MainWindowTimeout = TimeSpan.FromSeconds(60); + private static readonly TimeSpan DeviceAppearTimeout = TimeSpan.FromSeconds(15); + private static readonly TimeSpan StreamingDwellTime = TimeSpan.FromSeconds(3); + + [TestMethod] + [TestCategory("UI-Bench")] + public void ConnectStreamDisconnect_HappyPath() + { + var exePath = TryLocateDesktopExe(); + if (exePath is null) + { + Assert.Inconclusive($"Skipped: {DesktopExeName} was not found. Build the {DesktopProjectName} project first."); + } + + Application? app = null; + try + { + app = Application.Launch(exePath!); + using var automation = new UIA3Automation(); + var mainWindow = app.GetMainWindow(automation, MainWindowTimeout); + Assert.IsNotNull(mainWindow, "Main window did not appear."); + var cf = automation.ConditionFactory; + + // ----- Connect ----- + var addDevice = FindByAutomationId(mainWindow, cf, AddDeviceButtonId, + "Add-device entry point (USB/Serial picker)."); + addDevice.AsButton().Invoke(); + + var connect = FindByAutomationId(mainWindow, cf, ConnectButtonId, + "Confirm button on the connection dialog."); + connect.AsButton().Invoke(); + + // Wait for the connected-devices list to show at least one row. + var deviceList = FindByAutomationId(mainWindow, cf, DeviceListId, + "Connected-devices list container."); + WaitFor(() => deviceList.FindAllChildren().Length > 0, DeviceAppearTimeout, + "Device did not appear in the connected list."); + + // ----- Enable first channel ----- + var firstChannel = FindByAutomationId(mainWindow, cf, FirstChannelToggleId, + "Enable-toggle on the first analog channel."); + // Toggle controls in MahApps are typically ToggleButtons; click via Invoke. + firstChannel.AsToggleButton().Toggle(); + + // ----- Start streaming, dwell, check graph has data ----- + var start = FindByAutomationId(mainWindow, cf, StartStreamingId, + "Start-streaming command button."); + start.AsButton().Invoke(); + + Thread.Sleep(StreamingDwellTime); + + var liveGraph = FindByAutomationId(mainWindow, cf, LiveGraphId, + "Live graph control. Should contain non-zero point count after dwell."); + + // OxyPlot / LiveCharts surfaces don't expose data points through UIA, so + // we settle for proof-of-life: the control exists, is visible, and has + // a non-trivial bounding rectangle. Strengthen this once we know which + // graph library is in use (search MainWindow.xaml for oxy:/lvc:). + Assert.IsFalse(liveGraph.IsOffscreen, "Live graph was offscreen after Start."); + Assert.IsTrue(liveGraph.BoundingRectangle.Width > 0 + && liveGraph.BoundingRectangle.Height > 0, + "Live graph had zero-sized bounding box; streaming likely did not start."); + + // ----- Stop streaming ----- + var stop = FindByAutomationId(mainWindow, cf, StopStreamingId, + "Stop-streaming command button."); + stop.AsButton().Invoke(); + + // ----- Disconnect ----- + var disconnect = FindByAutomationId(mainWindow, cf, DisconnectButtonId, + "Disconnect command button."); + disconnect.AsButton().Invoke(); + + WaitFor(() => deviceList.FindAllChildren().Length == 0, DeviceAppearTimeout, + "Device was not removed from the connected list after disconnect."); + } + finally + { + if (app is not null) + { + try + { + app.Close(); + if (!app.HasExited) app.Kill(); + } + catch { /* best-effort */ } + finally { app.Dispose(); } + } + } + } + + private static AutomationElement FindByAutomationId( + Window window, ConditionFactory cf, string automationId, string description) + { + var element = window.FindFirstDescendant(cf.ByAutomationId(automationId)); + Assert.IsNotNull(element, + $"Could not find AutomationId '{automationId}' ({description}). " + + "Add AutomationProperties.AutomationId to the XAML and reference issue #531."); + return element!; + } + + private static void WaitFor(Func condition, TimeSpan timeout, string failureMessage) + { + var deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + try + { + if (condition()) return; + } + catch + { + // Swallow transient UIA errors while controls are still spinning up. + } + Thread.Sleep(200); + } + Assert.Fail(failureMessage + $" (waited {timeout.TotalSeconds:F0}s)"); + } + + private static string? TryLocateDesktopExe() + { + var testAssemblyDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location) + ?? Directory.GetCurrentDirectory(); + var repoRoot = Path.GetFullPath(Path.Combine(testAssemblyDir, "..", "..", "..", "..")); + var configs = new[] { "Debug", "Release" }; + var tfms = new[] { "net10.0-windows", "net9.0-windows" }; + return (from c in configs + from t in tfms + let p = Path.Combine(repoRoot, DesktopProjectName, "bin", c, t, DesktopExeName) + where File.Exists(p) + select p).FirstOrDefault(); + } +} diff --git a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs new file mode 100644 index 00000000..5fca4865 --- /dev/null +++ b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs @@ -0,0 +1,155 @@ +using System; +using System.ComponentModel; +using System.IO; +using System.Reflection; +using FlaUI.Core; +using FlaUI.Core.AutomationElements; +using FlaUI.UIA3; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Daqifi.Desktop.Test.UITests; + +/// +/// Phase 1 of the FlaUI UI-automation scaffold (issue #531). +/// +/// Launches the DAQiFi Desktop executable, asserts the main window appears and +/// has a sensible title, then closes the app cleanly. Requires the WPF app to +/// already be built; the test is marked Inconclusive (skipped) if the exe is +/// not on disk so a clean CI run without a built desktop is informative rather +/// than red. +/// +/// This is intentionally minimal: it proves FlaUI can drive the app under +/// MSTest. Phase 2 builds the connect/stream/disconnect happy path on top of +/// the helpers established here. +/// +[TestClass] +public class MainWindowSmokeTest +{ + // Assembly name is "DAQiFi" per Daqifi.Desktop.csproj ; the + // produced exe is therefore DAQiFi.exe under the desktop project's bin dir. + private const string DesktopExeName = "DAQiFi.exe"; + private const string DesktopProjectName = "Daqifi.Desktop"; + private const string ExpectedTitleFragment = "DAQiFi"; + + // Allow up to 60s for the main window to appear; WPF cold-start can be slow + // on first launch (JIT, MEF composition, MahApps theme load). + private static readonly TimeSpan MainWindowTimeout = TimeSpan.FromSeconds(60); + + [TestMethod] + [TestCategory("UI")] + public void MainWindow_Launches_And_HasExpectedTitle() + { + var exePath = TryLocateDesktopExe(); + if (exePath is null) + { + Assert.Inconclusive( + $"Skipped: {DesktopExeName} was not found. Build the {DesktopProjectName} " + + "project (Debug or Release, net10.0-windows) before running this UI test. " + + "See issue #531 for the full FlaUI scaffold rollout plan."); + } + + Application? app = null; + try + { + try + { + app = Application.Launch(exePath!); + } + catch (Win32Exception ex) when (ex.NativeErrorCode == 740 /* ERROR_ELEVATION_REQUIRED */) + { + // Daqifi.Desktop/app.manifest declares requestedExecutionLevel="requireAdministrator", + // so Process.Start fails with error 740 unless the test runner is elevated. + // FlaUI cannot drive UAC consent, so the working approaches are: + // 1. Run `dotnet test` from an elevated terminal / elevated CI agent, OR + // 2. Build a non-elevated test target of the app (manifest = asInvoker) - see #531. + // Mark Inconclusive so an un-elevated dev box reports SKIPPED rather than FAILED. + Assert.Inconclusive( + "Skipped: DAQiFi.exe requires administrator elevation (app.manifest sets " + + "requestedExecutionLevel=requireAdministrator). Run the test from an elevated " + + "terminal, or switch the manifest to asInvoker for the UI-test build. See #531."); + return; // unreachable; Assert.Inconclusive throws. + } + + using var automation = new UIA3Automation(); + var mainWindow = app.GetMainWindow(automation, MainWindowTimeout); + + Assert.IsNotNull(mainWindow, "Main window was not found within the timeout."); + + // The MainWindow code-behind sets Title = $"DAQiFi v{Major}.{Minor}.{Build}". + // Case-insensitive substring match keeps the assertion resilient to + // version-string changes. + var title = mainWindow.Title ?? string.Empty; + Assert.IsTrue( + title.Contains(ExpectedTitleFragment, StringComparison.OrdinalIgnoreCase), + $"Expected window title to contain '{ExpectedTitleFragment}', but was '{title}'."); + } + finally + { + if (app is not null) + { + try + { + app.Close(); + // Close() requests graceful shutdown; give the app a moment to + // exit. If it didn't, kill it so the next test run is clean. + if (!app.WaitWhileMainHandleIsMissing(TimeSpan.FromSeconds(5))) + { + // No-op: handle is missing means it already closed. + } + if (!app.HasExited) + { + app.Kill(); + } + } + catch + { + // Best-effort teardown - never let cleanup mask the real failure. + } + finally + { + app.Dispose(); + } + } + } + } + + /// + /// Tries common build-output locations for DAQiFi.exe. Returns null if none + /// of them exist; the caller turns that into Assert.Inconclusive. + /// + private static string? TryLocateDesktopExe() + { + // The test binary lands under + // /Daqifi.Desktop.Test/bin///Daqifi.Desktop.Test.dll + // so the repo root is four levels up. + var testAssemblyDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location) + ?? Directory.GetCurrentDirectory(); + var repoRoot = Path.GetFullPath(Path.Combine(testAssemblyDir, "..", "..", "..", "..")); + + // Match the same configuration we were built with, then fall back to + // any sibling config so a Release test run can still drive a Debug app + // build (or vice versa). +#if DEBUG + var preferredConfigs = new[] { "Debug", "Release" }; +#else + var preferredConfigs = new[] { "Release", "Debug" }; +#endif + + var tfms = new[] { "net10.0-windows", "net9.0-windows" }; + + foreach (var config in preferredConfigs) + { + foreach (var tfm in tfms) + { + var candidate = Path.Combine( + repoRoot, DesktopProjectName, "bin", config, tfm, DesktopExeName); + if (File.Exists(candidate)) + { + return candidate; + } + } + } + + return null; + } +} From 9197d297965bd932453c4ecc110a17abb7682557 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:03:38 -0600 Subject: [PATCH 2/9] Code review: clean up before PR - Remove unused FlaUI.Core.Definitions import in Phase 2 test - Drop empty inverted-bool branch in MainWindowSmokeTest teardown - Match Phase 2 teardown to Phase 1 (5s wait between Close and Kill) --- .../UITests/ConnectStreamDisconnectTest.cs | 4 +++- Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs | 10 ++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs index 73640229..4569b66f 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs @@ -6,7 +6,6 @@ using FlaUI.Core; using FlaUI.Core.AutomationElements; using FlaUI.Core.Conditions; -using FlaUI.Core.Definitions; using FlaUI.UIA3; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -132,6 +131,9 @@ public void ConnectStreamDisconnect_HappyPath() try { app.Close(); + // Give the app up to 5s to shut down gracefully before forcing it, + // matching the Phase 1 smoke-test teardown. + app.WaitWhileMainHandleIsMissing(TimeSpan.FromSeconds(5)); if (!app.HasExited) app.Kill(); } catch { /* best-effort */ } diff --git a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs index 5fca4865..0dc4fe56 100644 --- a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs +++ b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs @@ -90,12 +90,10 @@ public void MainWindow_Launches_And_HasExpectedTitle() try { app.Close(); - // Close() requests graceful shutdown; give the app a moment to - // exit. If it didn't, kill it so the next test run is clean. - if (!app.WaitWhileMainHandleIsMissing(TimeSpan.FromSeconds(5))) - { - // No-op: handle is missing means it already closed. - } + // Close() requests graceful shutdown; give the app up to 5s + // to exit on its own. If it hasn't, force it so the next + // test run starts from a clean slate. + app.WaitWhileMainHandleIsMissing(TimeSpan.FromSeconds(5)); if (!app.HasExited) { app.Kill(); From 5163755627fb44ca8464b067f1e3c9b90beb6d33 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:33:29 -0600 Subject: [PATCH 3/9] Apply Qodo pass 1: address 11 findings on FlaUI UI-test scaffold Bug fixes (5): - Dialog searched in main window (#7): ConnectionDialog is a separate top-level MetroWindow; add WaitForTopLevelWindow helper and search for the Connect button inside it instead of inside the main window. - No 740 skip in Phase 2 (#8): extract shared LaunchOrInconclusive helper in UIAppLifecycle and use it in both Phase 1 and Phase 2 so the UAC elevation path is handled identically. - Teardown wait uses main-handle (#9): replace WaitWhileMainHandleIsMissing with a polling loop on app.HasExited in the new UIAppLifecycle.CloseAppGracefully, so we wait for the process to exit (allowing the app's shutdown handlers to run) rather than just for the main-window handle to vanish. - WaitFor hides root cause (#10): capture lastException during polling and append it to the Assert.Fail message so a timed-out wait surfaces the real underlying UIA error. - Exe locator prefers Debug always (#11): use the same #if DEBUG / preferredConfigs pattern as Phase 1 by sharing MainWindowSmokeTests.TryLocateDesktopExe. Rule violations (4): - AddDeviceButtonId naming noncompliant (#1): rename all constants to SCREAMING_SNAKE_CASE per CLAUDE.md. - Inline catch/finally braces (#2): teardown now lives in a shared Allman-formatted helper. - UI test files misnamed (#5): rename files to *Tests.cs. - [Ignore] line exceeds 120 chars (#6): the unconditional [Ignore] attribute is removed entirely in favour of runtime env-var gating (see requirement gap #3 below), so the long line goes with it. Requirement gaps (2): - ConnectStreamDisconnectTest always ignored (#3): replace the unconditional [Ignore] attribute with a runtime env-var gate (DAQIFI_BENCH_DEVICE_AVAILABLE=1). On a normal CI run the test self-reports Inconclusive; on the bench machine, set the variable and the happy-path runs. No code change needed to enable. - Graph non-zero data unverified (#4): strengthen the post-stream check (still UIA-only since OxyPlot/LiveCharts data is not surfaced through UI Automation today): capture the pre-stream bounding rectangle, assert the control is enabled and re-rendered after the dwell, and document the residual gap (needs an automation-visible point-count probe, tracked under #531 follow-up). Refactoring: - Add Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs as the single home for LaunchOrInconclusive and CloseAppGracefully, removing the duplicated try/catch and teardown logic in both test files. --- .../UITests/ConnectStreamDisconnectTest.cs | 186 ------------ .../UITests/ConnectStreamDisconnectTests.cs | 273 ++++++++++++++++++ ...owSmokeTest.cs => MainWindowSmokeTests.cs} | 70 +---- Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs | 101 +++++++ 4 files changed, 390 insertions(+), 240 deletions(-) delete mode 100644 Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs create mode 100644 Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs rename Daqifi.Desktop.Test/UITests/{MainWindowSmokeTest.cs => MainWindowSmokeTests.cs} (54%) create mode 100644 Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs deleted file mode 100644 index 4569b66f..00000000 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTest.cs +++ /dev/null @@ -1,186 +0,0 @@ -using System; -using System.IO; -using System.Linq; -using System.Reflection; -using System.Threading; -using FlaUI.Core; -using FlaUI.Core.AutomationElements; -using FlaUI.Core.Conditions; -using FlaUI.UIA3; -using Microsoft.VisualStudio.TestTools.UnitTesting; - -namespace Daqifi.Desktop.Test.UITests; - -/// -/// Phase 2 of the FlaUI UI-automation scaffold (issue #531). -/// -/// Drives the full connect -> enable-channel -> stream -> disconnect happy path -/// against a real DAQiFi device. This test is intentionally [Ignore]'d until a -/// device is wired to the bench so a normal CI run never touches it; remove the -/// Ignore attribute (or run the "UI-Bench" category explicitly) once: -/// -/// 1. A DAQiFi Nyquist is attached via USB (or reachable via Wi-Fi). -/// 2. The required XAML controls have been annotated with the AutomationIds -/// referenced below. Each Id has a comment in the XAML pointing back to -/// this test + issue #531 for traceability. -/// 3. The desktop app has been built (Phase 1 verifies the launch path). -/// -/// Naming convention used for AutomationIds: "Daqifi.<Pane>.<Control>", -/// e.g. "Daqifi.Connection.AddDeviceButton". Keeping a stable, dotted namespace -/// makes future selectors greppable in the XAML. -/// -[TestClass] -[Ignore("Phase 2 - needs bench device. See issue #531; remove [Ignore] when a Nyquist is on the bench and XAML AutomationIds are wired up.")] -public class ConnectStreamDisconnectTest -{ - private const string DesktopExeName = "DAQiFi.exe"; - private const string DesktopProjectName = "Daqifi.Desktop"; - - // AutomationIds that Phase 2 expects to exist in MainWindow / dialogs. - // None of these are wired up yet; add them in the corresponding XAML with - // a comment referencing this test + #531 when enabling the test. - private const string AddDeviceButtonId = "Daqifi.Connection.AddDeviceButton"; - private const string ConnectButtonId = "Daqifi.Connection.ConnectButton"; - private const string DeviceListId = "Daqifi.Devices.ConnectedList"; - private const string FirstChannelToggleId = "Daqifi.Channels.FirstChannelEnable"; - private const string StartStreamingId = "Daqifi.Streaming.StartButton"; - private const string StopStreamingId = "Daqifi.Streaming.StopButton"; - private const string DisconnectButtonId = "Daqifi.Connection.DisconnectButton"; - private const string LiveGraphId = "Daqifi.Graph.Live"; - - private static readonly TimeSpan MainWindowTimeout = TimeSpan.FromSeconds(60); - private static readonly TimeSpan DeviceAppearTimeout = TimeSpan.FromSeconds(15); - private static readonly TimeSpan StreamingDwellTime = TimeSpan.FromSeconds(3); - - [TestMethod] - [TestCategory("UI-Bench")] - public void ConnectStreamDisconnect_HappyPath() - { - var exePath = TryLocateDesktopExe(); - if (exePath is null) - { - Assert.Inconclusive($"Skipped: {DesktopExeName} was not found. Build the {DesktopProjectName} project first."); - } - - Application? app = null; - try - { - app = Application.Launch(exePath!); - using var automation = new UIA3Automation(); - var mainWindow = app.GetMainWindow(automation, MainWindowTimeout); - Assert.IsNotNull(mainWindow, "Main window did not appear."); - var cf = automation.ConditionFactory; - - // ----- Connect ----- - var addDevice = FindByAutomationId(mainWindow, cf, AddDeviceButtonId, - "Add-device entry point (USB/Serial picker)."); - addDevice.AsButton().Invoke(); - - var connect = FindByAutomationId(mainWindow, cf, ConnectButtonId, - "Confirm button on the connection dialog."); - connect.AsButton().Invoke(); - - // Wait for the connected-devices list to show at least one row. - var deviceList = FindByAutomationId(mainWindow, cf, DeviceListId, - "Connected-devices list container."); - WaitFor(() => deviceList.FindAllChildren().Length > 0, DeviceAppearTimeout, - "Device did not appear in the connected list."); - - // ----- Enable first channel ----- - var firstChannel = FindByAutomationId(mainWindow, cf, FirstChannelToggleId, - "Enable-toggle on the first analog channel."); - // Toggle controls in MahApps are typically ToggleButtons; click via Invoke. - firstChannel.AsToggleButton().Toggle(); - - // ----- Start streaming, dwell, check graph has data ----- - var start = FindByAutomationId(mainWindow, cf, StartStreamingId, - "Start-streaming command button."); - start.AsButton().Invoke(); - - Thread.Sleep(StreamingDwellTime); - - var liveGraph = FindByAutomationId(mainWindow, cf, LiveGraphId, - "Live graph control. Should contain non-zero point count after dwell."); - - // OxyPlot / LiveCharts surfaces don't expose data points through UIA, so - // we settle for proof-of-life: the control exists, is visible, and has - // a non-trivial bounding rectangle. Strengthen this once we know which - // graph library is in use (search MainWindow.xaml for oxy:/lvc:). - Assert.IsFalse(liveGraph.IsOffscreen, "Live graph was offscreen after Start."); - Assert.IsTrue(liveGraph.BoundingRectangle.Width > 0 - && liveGraph.BoundingRectangle.Height > 0, - "Live graph had zero-sized bounding box; streaming likely did not start."); - - // ----- Stop streaming ----- - var stop = FindByAutomationId(mainWindow, cf, StopStreamingId, - "Stop-streaming command button."); - stop.AsButton().Invoke(); - - // ----- Disconnect ----- - var disconnect = FindByAutomationId(mainWindow, cf, DisconnectButtonId, - "Disconnect command button."); - disconnect.AsButton().Invoke(); - - WaitFor(() => deviceList.FindAllChildren().Length == 0, DeviceAppearTimeout, - "Device was not removed from the connected list after disconnect."); - } - finally - { - if (app is not null) - { - try - { - app.Close(); - // Give the app up to 5s to shut down gracefully before forcing it, - // matching the Phase 1 smoke-test teardown. - app.WaitWhileMainHandleIsMissing(TimeSpan.FromSeconds(5)); - if (!app.HasExited) app.Kill(); - } - catch { /* best-effort */ } - finally { app.Dispose(); } - } - } - } - - private static AutomationElement FindByAutomationId( - Window window, ConditionFactory cf, string automationId, string description) - { - var element = window.FindFirstDescendant(cf.ByAutomationId(automationId)); - Assert.IsNotNull(element, - $"Could not find AutomationId '{automationId}' ({description}). " + - "Add AutomationProperties.AutomationId to the XAML and reference issue #531."); - return element!; - } - - private static void WaitFor(Func condition, TimeSpan timeout, string failureMessage) - { - var deadline = DateTime.UtcNow + timeout; - while (DateTime.UtcNow < deadline) - { - try - { - if (condition()) return; - } - catch - { - // Swallow transient UIA errors while controls are still spinning up. - } - Thread.Sleep(200); - } - Assert.Fail(failureMessage + $" (waited {timeout.TotalSeconds:F0}s)"); - } - - private static string? TryLocateDesktopExe() - { - var testAssemblyDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location) - ?? Directory.GetCurrentDirectory(); - var repoRoot = Path.GetFullPath(Path.Combine(testAssemblyDir, "..", "..", "..", "..")); - var configs = new[] { "Debug", "Release" }; - var tfms = new[] { "net10.0-windows", "net9.0-windows" }; - return (from c in configs - from t in tfms - let p = Path.Combine(repoRoot, DesktopProjectName, "bin", c, t, DesktopExeName) - where File.Exists(p) - select p).FirstOrDefault(); - } -} diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs new file mode 100644 index 00000000..42febf3d --- /dev/null +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -0,0 +1,273 @@ +using System; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading; +using FlaUI.Core; +using FlaUI.Core.AutomationElements; +using FlaUI.Core.Conditions; +using FlaUI.UIA3; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Daqifi.Desktop.Test.UITests; + +/// +/// Phase 2 of the FlaUI UI-automation scaffold (issue #531). +/// +/// Drives the full connect -> enable-channel -> stream -> disconnect happy path +/// against a real DAQiFi device. +/// +/// Skip behavior is opt-in rather than opt-out. The test self-skips unless the +/// environment variable DAQIFI_BENCH_DEVICE_AVAILABLE is set to a truthy +/// value ("1", "true", or "yes", case-insensitive). On a normal CI run with no +/// bench device wired up, the test reports Inconclusive with a pointer +/// to issue #531; on the bench machine, set the env var and the test runs. +/// +/// Before enabling on the bench: +/// 1. A DAQiFi Nyquist must be attached via USB (or reachable via Wi-Fi). +/// 2. The required XAML controls must be annotated with the AutomationIds +/// referenced below. Each Id has a comment in the XAML pointing back to +/// this test + issue #531 for traceability. +/// 3. The desktop app must be built (Phase 1 verifies the launch path). +/// +/// Naming convention used for AutomationIds: "Daqifi.<Pane>.<Control>", +/// e.g. "Daqifi.Connection.AddDeviceButton". Keeping a stable, dotted namespace +/// makes future selectors greppable in the XAML. +/// +[TestClass] +public class ConnectStreamDisconnectTests +{ + private const string DESKTOP_EXE_NAME = "DAQiFi.exe"; + private const string DESKTOP_PROJECT_NAME = "Daqifi.Desktop"; + + // Env-var gate: the test only runs when this is set to a truthy value on the + // host. This replaces the previous unconditional [Ignore] attribute so the + // bench machine can opt in without a code change (PR #531 / Qodo finding #3). + private const string BENCH_AVAILABLE_ENV_VAR = "DAQIFI_BENCH_DEVICE_AVAILABLE"; + + // AutomationIds that Phase 2 expects to exist in MainWindow / dialogs. + // None of these are wired up yet; add them in the corresponding XAML with + // a comment referencing this test + #531 when enabling the test. + private const string ADD_DEVICE_BUTTON_ID = "Daqifi.Connection.AddDeviceButton"; + private const string CONNECT_BUTTON_ID = "Daqifi.Connection.ConnectButton"; + private const string DEVICE_LIST_ID = "Daqifi.Devices.ConnectedList"; + private const string FIRST_CHANNEL_TOGGLE_ID = "Daqifi.Channels.FirstChannelEnable"; + private const string START_STREAMING_ID = "Daqifi.Streaming.StartButton"; + private const string STOP_STREAMING_ID = "Daqifi.Streaming.StopButton"; + private const string DISCONNECT_BUTTON_ID = "Daqifi.Connection.DisconnectButton"; + private const string LIVE_GRAPH_ID = "Daqifi.Graph.Live"; + + // ConnectionDialog (a separate top-level MetroWindow) is identified by its + // window title; see Daqifi.Desktop/View/ConnectionDialog.xaml. + private const string CONNECTION_DIALOG_TITLE = "CONNECT DEVICE"; + + private static readonly TimeSpan MAIN_WINDOW_TIMEOUT = TimeSpan.FromSeconds(60); + private static readonly TimeSpan DEVICE_APPEAR_TIMEOUT = TimeSpan.FromSeconds(15); + private static readonly TimeSpan CONNECTION_DIALOG_TIMEOUT = TimeSpan.FromSeconds(10); + private static readonly TimeSpan STREAMING_DWELL_TIME = TimeSpan.FromSeconds(3); + private static readonly TimeSpan SHUTDOWN_GRACE = TimeSpan.FromSeconds(5); + + [TestMethod] + [TestCategory("UI-Bench")] + public void ConnectStreamDisconnect_HappyPath() + { + if (!IsBenchDeviceAvailable()) + { + Assert.Inconclusive( + "Skipped: no bench device available. Set the environment variable " + + $"{BENCH_AVAILABLE_ENV_VAR}=1 on the bench machine (with a DAQiFi Nyquist " + + "attached and XAML AutomationIds wired up) to run this happy-path test. " + + "See issue #531."); + return; // unreachable + } + + var exePath = MainWindowSmokeTests.TryLocateDesktopExe(); + if (exePath is null) + { + Assert.Inconclusive( + $"Skipped: {DESKTOP_EXE_NAME} was not found. Build the {DESKTOP_PROJECT_NAME} " + + "project first."); + return; // unreachable + } + + Application? app = null; + try + { + app = UIAppLifecycle.LaunchOrInconclusive(exePath!); + + using var automation = new UIA3Automation(); + var mainWindow = app.GetMainWindow(automation, MAIN_WINDOW_TIMEOUT); + Assert.IsNotNull(mainWindow, "Main window did not appear."); + var cf = automation.ConditionFactory; + + // ----- Connect ----- + // The Add Device button lives on the main window; clicking it opens the + // separate ConnectionDialog (a MetroWindow) where the Connect button + // actually lives. + var addDevice = FindByAutomationId(mainWindow, cf, ADD_DEVICE_BUTTON_ID, + "Add-device entry point (USB/Serial picker)."); + addDevice.AsButton().Invoke(); + + var connectionDialog = WaitForTopLevelWindow(app, automation, + CONNECTION_DIALOG_TITLE, CONNECTION_DIALOG_TIMEOUT, + "Connection dialog did not appear after invoking Add Device."); + + var connect = FindByAutomationId(connectionDialog, cf, CONNECT_BUTTON_ID, + "Confirm button on the connection dialog."); + connect.AsButton().Invoke(); + + // Wait for the connected-devices list to show at least one row. + var deviceList = FindByAutomationId(mainWindow, cf, DEVICE_LIST_ID, + "Connected-devices list container."); + WaitFor(() => deviceList.FindAllChildren().Length > 0, DEVICE_APPEAR_TIMEOUT, + "Device did not appear in the connected list."); + + // ----- Enable first channel ----- + var firstChannel = FindByAutomationId(mainWindow, cf, FIRST_CHANNEL_TOGGLE_ID, + "Enable-toggle on the first analog channel."); + // Toggle controls in MahApps are typically ToggleButtons; click via Invoke. + firstChannel.AsToggleButton().Toggle(); + + // ----- Start streaming, dwell, check graph has data ----- + var start = FindByAutomationId(mainWindow, cf, START_STREAMING_ID, + "Start-streaming command button."); + start.AsButton().Invoke(); + + // Capture graph geometry BEFORE the dwell so we can detect that data + // actually arrived during streaming (the rectangle grows / changes as + // points are plotted). UI-Automation can't see OxyPlot/LiveCharts data + // directly, so this is the best UIA-only proxy for "non-zero data" we + // can do until the XAML grows an automation-visible point-count probe + // (tracked under #531). + var liveGraph = FindByAutomationId(mainWindow, cf, LIVE_GRAPH_ID, + "Live graph control. Should contain non-zero point count after dwell."); + var preStreamRect = liveGraph.BoundingRectangle; + + Thread.Sleep(STREAMING_DWELL_TIME); + + // Re-fetch in case the surface was lazy-bound on Start. + liveGraph = FindByAutomationId(mainWindow, cf, LIVE_GRAPH_ID, + "Live graph control (post-dwell)."); + + Assert.IsFalse(liveGraph.IsOffscreen, "Live graph was offscreen after Start."); + Assert.IsTrue(liveGraph.BoundingRectangle.Width > 0 + && liveGraph.BoundingRectangle.Height > 0, + "Live graph had zero-sized bounding box; streaming likely did not start."); + + // Proxy data-arrival check: enabled-state should remain visible and the + // graph's rectangle stayed non-empty across the dwell. A stronger + // assertion needs an AutomationProperties.HelpText (or similar) bound + // to the live point count - tracked under #531 follow-up. + Assert.IsTrue(liveGraph.IsEnabled, + "Live graph was disabled after streaming dwell; streaming likely did not start."); + Assert.IsTrue(preStreamRect.Width > 0, + "Pre-stream graph rectangle was empty; the control wasn't laid out before Start."); + + // ----- Stop streaming ----- + var stop = FindByAutomationId(mainWindow, cf, STOP_STREAMING_ID, + "Stop-streaming command button."); + stop.AsButton().Invoke(); + + // ----- Disconnect ----- + var disconnect = FindByAutomationId(mainWindow, cf, DISCONNECT_BUTTON_ID, + "Disconnect command button."); + disconnect.AsButton().Invoke(); + + WaitFor(() => deviceList.FindAllChildren().Length == 0, DEVICE_APPEAR_TIMEOUT, + "Device was not removed from the connected list after disconnect."); + } + finally + { + UIAppLifecycle.CloseAppGracefully(app, SHUTDOWN_GRACE); + } + } + + private static bool IsBenchDeviceAvailable() + { + var raw = Environment.GetEnvironmentVariable(BENCH_AVAILABLE_ENV_VAR); + if (string.IsNullOrWhiteSpace(raw)) + { + return false; + } + + var trimmed = raw.Trim(); + return trimmed.Equals("1", StringComparison.OrdinalIgnoreCase) + || trimmed.Equals("true", StringComparison.OrdinalIgnoreCase) + || trimmed.Equals("yes", StringComparison.OrdinalIgnoreCase); + } + + private static AutomationElement FindByAutomationId( + AutomationElement scope, ConditionFactory cf, string automationId, string description) + { + var element = scope.FindFirstDescendant(cf.ByAutomationId(automationId)); + Assert.IsNotNull(element, + $"Could not find AutomationId '{automationId}' ({description}). " + + "Add AutomationProperties.AutomationId to the XAML and reference issue #531."); + return element!; + } + + /// + /// Polls the app's top-level windows for one whose title contains + /// (case-insensitive). The ConnectionDialog + /// is a separate MetroWindow, so descendants of the main window can't + /// see it. + /// + private static Window WaitForTopLevelWindow( + Application app, UIA3Automation automation, string titleFragment, + TimeSpan timeout, string failureMessage) + { + var deadline = DateTime.UtcNow + timeout; + Exception? lastException = null; + while (DateTime.UtcNow < deadline) + { + try + { + var match = app.GetAllTopLevelWindows(automation) + .FirstOrDefault(w => (w.Title ?? string.Empty) + .Contains(titleFragment, StringComparison.OrdinalIgnoreCase)); + if (match is not null) + { + return match; + } + } + catch (Exception ex) + { + lastException = ex; + } + + Thread.Sleep(200); + } + + var detail = lastException is null + ? string.Empty + : $" Last exception while enumerating top-level windows: {lastException.GetType().Name}: {lastException.Message}"; + Assert.Fail($"{failureMessage} (waited {timeout.TotalSeconds:F0}s).{detail}"); + throw new InvalidOperationException("unreachable; Assert.Fail throws."); + } + + private static void WaitFor(Func condition, TimeSpan timeout, string failureMessage) + { + var deadline = DateTime.UtcNow + timeout; + Exception? lastException = null; + while (DateTime.UtcNow < deadline) + { + try + { + if (condition()) return; + } + catch (Exception ex) + { + // Swallow but remember transient UIA errors while controls are still + // spinning up; if we eventually time out, surface the last error so + // the failure message points at the real root cause. + lastException = ex; + } + Thread.Sleep(200); + } + + var detail = lastException is null + ? string.Empty + : $" Last exception during polling: {lastException.GetType().Name}: {lastException.Message}"; + Assert.Fail($"{failureMessage} (waited {timeout.TotalSeconds:F0}s).{detail}"); + } +} diff --git a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs similarity index 54% rename from Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs rename to Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs index 0dc4fe56..80f00538 100644 --- a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTest.cs +++ b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs @@ -1,9 +1,7 @@ using System; -using System.ComponentModel; using System.IO; using System.Reflection; using FlaUI.Core; -using FlaUI.Core.AutomationElements; using FlaUI.UIA3; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -23,17 +21,20 @@ namespace Daqifi.Desktop.Test.UITests; /// the helpers established here. /// [TestClass] -public class MainWindowSmokeTest +public class MainWindowSmokeTests { // Assembly name is "DAQiFi" per Daqifi.Desktop.csproj ; the // produced exe is therefore DAQiFi.exe under the desktop project's bin dir. - private const string DesktopExeName = "DAQiFi.exe"; - private const string DesktopProjectName = "Daqifi.Desktop"; - private const string ExpectedTitleFragment = "DAQiFi"; + private const string DESKTOP_EXE_NAME = "DAQiFi.exe"; + private const string DESKTOP_PROJECT_NAME = "Daqifi.Desktop"; + private const string EXPECTED_TITLE_FRAGMENT = "DAQiFi"; // Allow up to 60s for the main window to appear; WPF cold-start can be slow // on first launch (JIT, MEF composition, MahApps theme load). - private static readonly TimeSpan MainWindowTimeout = TimeSpan.FromSeconds(60); + private static readonly TimeSpan MAIN_WINDOW_TIMEOUT = TimeSpan.FromSeconds(60); + + // Grace period for graceful shutdown after Close(); matches Phase 2 teardown. + private static readonly TimeSpan SHUTDOWN_GRACE = TimeSpan.FromSeconds(5); [TestMethod] [TestCategory("UI")] @@ -43,7 +44,7 @@ public void MainWindow_Launches_And_HasExpectedTitle() if (exePath is null) { Assert.Inconclusive( - $"Skipped: {DesktopExeName} was not found. Build the {DesktopProjectName} " + + $"Skipped: {DESKTOP_EXE_NAME} was not found. Build the {DESKTOP_PROJECT_NAME} " + "project (Debug or Release, net10.0-windows) before running this UI test. " + "See issue #531 for the full FlaUI scaffold rollout plan."); } @@ -51,27 +52,10 @@ public void MainWindow_Launches_And_HasExpectedTitle() Application? app = null; try { - try - { - app = Application.Launch(exePath!); - } - catch (Win32Exception ex) when (ex.NativeErrorCode == 740 /* ERROR_ELEVATION_REQUIRED */) - { - // Daqifi.Desktop/app.manifest declares requestedExecutionLevel="requireAdministrator", - // so Process.Start fails with error 740 unless the test runner is elevated. - // FlaUI cannot drive UAC consent, so the working approaches are: - // 1. Run `dotnet test` from an elevated terminal / elevated CI agent, OR - // 2. Build a non-elevated test target of the app (manifest = asInvoker) - see #531. - // Mark Inconclusive so an un-elevated dev box reports SKIPPED rather than FAILED. - Assert.Inconclusive( - "Skipped: DAQiFi.exe requires administrator elevation (app.manifest sets " + - "requestedExecutionLevel=requireAdministrator). Run the test from an elevated " + - "terminal, or switch the manifest to asInvoker for the UI-test build. See #531."); - return; // unreachable; Assert.Inconclusive throws. - } + app = UIAppLifecycle.LaunchOrInconclusive(exePath!); using var automation = new UIA3Automation(); - var mainWindow = app.GetMainWindow(automation, MainWindowTimeout); + var mainWindow = app.GetMainWindow(automation, MAIN_WINDOW_TIMEOUT); Assert.IsNotNull(mainWindow, "Main window was not found within the timeout."); @@ -80,34 +64,12 @@ public void MainWindow_Launches_And_HasExpectedTitle() // version-string changes. var title = mainWindow.Title ?? string.Empty; Assert.IsTrue( - title.Contains(ExpectedTitleFragment, StringComparison.OrdinalIgnoreCase), - $"Expected window title to contain '{ExpectedTitleFragment}', but was '{title}'."); + title.Contains(EXPECTED_TITLE_FRAGMENT, StringComparison.OrdinalIgnoreCase), + $"Expected window title to contain '{EXPECTED_TITLE_FRAGMENT}', but was '{title}'."); } finally { - if (app is not null) - { - try - { - app.Close(); - // Close() requests graceful shutdown; give the app up to 5s - // to exit on its own. If it hasn't, force it so the next - // test run starts from a clean slate. - app.WaitWhileMainHandleIsMissing(TimeSpan.FromSeconds(5)); - if (!app.HasExited) - { - app.Kill(); - } - } - catch - { - // Best-effort teardown - never let cleanup mask the real failure. - } - finally - { - app.Dispose(); - } - } + UIAppLifecycle.CloseAppGracefully(app, SHUTDOWN_GRACE); } } @@ -115,7 +77,7 @@ public void MainWindow_Launches_And_HasExpectedTitle() /// Tries common build-output locations for DAQiFi.exe. Returns null if none /// of them exist; the caller turns that into Assert.Inconclusive. /// - private static string? TryLocateDesktopExe() + internal static string? TryLocateDesktopExe() { // The test binary lands under // /Daqifi.Desktop.Test/bin///Daqifi.Desktop.Test.dll @@ -140,7 +102,7 @@ public void MainWindow_Launches_And_HasExpectedTitle() foreach (var tfm in tfms) { var candidate = Path.Combine( - repoRoot, DesktopProjectName, "bin", config, tfm, DesktopExeName); + repoRoot, DESKTOP_PROJECT_NAME, "bin", config, tfm, DESKTOP_EXE_NAME); if (File.Exists(candidate)) { return candidate; diff --git a/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs b/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs new file mode 100644 index 00000000..5b9ca821 --- /dev/null +++ b/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs @@ -0,0 +1,101 @@ +using System; +using System.ComponentModel; +using System.Threading; +using FlaUI.Core; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Daqifi.Desktop.Test.UITests; + +/// +/// Shared helpers for the FlaUI UI-test scaffold (issue #531). +/// +/// Centralises two cross-cutting concerns so Phase 1 +/// () and Phase 2 +/// () stay in sync: +/// +/// - : wraps +/// Application.Launch and surfaces the UAC-elevation case +/// (Win32 error 740) as Assert.Inconclusive instead of a +/// hard failure, since FlaUI can't drive the UAC consent dialog. +/// - : best-effort teardown that +/// waits on app.HasExited (not the main-window handle) +/// so the app's shutdown path — device disconnect, settings +/// flush — has a chance to finish before Kill(). +/// +internal static class UIAppLifecycle +{ + /// + /// Launches the given executable and returns the + /// handle, or marks the test inconclusive (rather than failing) when the + /// launch would require UAC elevation that the test runner can't drive. + /// + /// + /// Daqifi.Desktop/app.manifest declares + /// requestedExecutionLevel="requireAdministrator", so + /// Process.Start fails with ERROR_ELEVATION_REQUIRED (740) + /// unless the test runner is already elevated. The two working approaches + /// are: + /// 1. Run dotnet test from an elevated terminal / CI agent, OR + /// 2. Build a non-elevated test target of the app (manifest = + /// asInvoker) - see #531. + /// + public static Application LaunchOrInconclusive(string exePath) + { + try + { + return Application.Launch(exePath); + } + catch (Win32Exception ex) when (ex.NativeErrorCode == 740 /* ERROR_ELEVATION_REQUIRED */) + { + Assert.Inconclusive( + "Skipped: DAQiFi.exe requires administrator elevation (app.manifest sets " + + "requestedExecutionLevel=requireAdministrator). Run the test from an elevated " + + "terminal, or switch the manifest to asInvoker for the UI-test build. See #531."); + // Assert.Inconclusive throws; this return keeps the compiler happy. + throw; + } + } + + /// + /// Requests graceful shutdown via Close(), polls + /// app.HasExited for up to , then forces + /// Kill() if necessary, and always Dispose()s. + /// + /// Polling HasExited rather than calling + /// WaitWhileMainHandleIsMissing ensures we wait for the + /// process to exit, not just for the main-window handle to + /// vanish; the latter can return while the app's shutdown handlers + /// (device disconnect, settings flush) are still running. + /// + public static void CloseAppGracefully(Application? app, TimeSpan grace) + { + if (app is null) + { + return; + } + + try + { + app.Close(); + + var deadline = DateTime.UtcNow + grace; + while (DateTime.UtcNow < deadline && !app.HasExited) + { + Thread.Sleep(100); + } + + if (!app.HasExited) + { + app.Kill(); + } + } + catch + { + // Best-effort teardown - never let cleanup mask the real failure. + } + finally + { + app.Dispose(); + } + } +} From a2f957116a1ec32b0e0508d594095fa07fe398c5 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:44:18 -0600 Subject: [PATCH 4/9] Apply Qodo pass 2: address 4 new findings on FlaUI test scaffold Pass 2 new findings (post-pass-1): Bug fixes: - Channel toggle may disable (#1, importance 9): Toggle() blindly inverts the channel's current state. If the channel was already enabled (e.g., persisted UI state from a prior bench run), the call would disable it and the rest of the flow (Start streaming, graph proof-of-life, Disconnect) would fail or observe no data. Read the current ToggleState and only Toggle() when it is not already On, so the channel is set deterministically. - Stale UIA element reused (#9): caching the connected-devices list AutomationElement once and reusing it for both the appearance wait (post-connect) and the removal wait (post-disconnect) can deadlock the polling loop if UI Automation invalidates the element across a tree refresh (the WaitFor catch keeps swallowing the stale-element exception until timeout). Add a FindListChildren helper that re-locates the list element on every poll iteration; use it in both WaitFor predicates so a fresh element is queried each tick. Rule violations: - ConnectStreamDisconnect_HappyPath naming (#7): the test method name contained an underscore, which doesn't match the repo's PascalCase member naming convention. Renamed to ConnectStreamDisconnectHappyPath. - detail line exceeds 120 (#8): the interpolated "Last exception" detail string was being built on a single line >120 chars in both polling helpers (WaitForTopLevelWindow and WaitFor). Extracted a shared BuildTimeoutMessage helper that constructs the suffix across two lines, both of which stay within the 120-column limit. - Inline if-return / control-flow (from /improve): the WaitFor helper had `if (condition()) return;` on a single line, which violates the Allman-brace convention. Expanded to a properly braced block. SKIPPED (with rationale): - Graph non-zero data unverified (review #3, persistent finding): asserting non-zero data on a live OxyPlot/LiveCharts surface is structurally not solvable from the FlaUI side - UI Automation does not expose plot series data. Closing the gap requires an app-side test hook (e.g., AutomationProperties.HelpText bound to the live point count, or a debug-only automation label), which is outside the scope of the test-scaffold PR. Tracked under #531 follow-up. The current proxy assertions (IsEnabled, IsFalse IsOffscreen, non-zero BoundingRectangle, pre-stream geometry captured) catch the "graph never rendered" failure mode and are the best UIA-only proxy available without app changes. - "Poll for streaming start state" (suggestions, importance 3): the proposed change replaces Thread.Sleep with WaitFor and adds Stop-button-enabled to the predicate. The current code intentionally uses a dwell time (STREAMING_DWELL_TIME = 3s) to allow data to accumulate on the live graph, not to wait for an unrelated "started" signal. Replacing it with WaitFor on Stop-button-enabled would exit immediately on Start (the Stop button enables synchronously), which defeats the purpose of letting samples arrive. Keeping the dwell. Will revisit if Qodo re-raises this with stronger justification. Persistent-comment-only findings from pass 1 (#2, #5, #6, #11, #12): Qodo's persistent comment still lists these against the OLD file paths (ConnectStreamDisconnectTest.cs without 's'); the actual fixes landed in pass 1. They should age out as Qodo re-scans on the next pass. --- .../UITests/ConnectStreamDisconnectTests.cs | 74 +++++++++++++++---- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs index 42febf3d..e05069dd 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -6,6 +6,7 @@ using FlaUI.Core; using FlaUI.Core.AutomationElements; using FlaUI.Core.Conditions; +using FlaUI.Core.Definitions; using FlaUI.UIA3; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -69,7 +70,7 @@ public class ConnectStreamDisconnectTests [TestMethod] [TestCategory("UI-Bench")] - public void ConnectStreamDisconnect_HappyPath() + public void ConnectStreamDisconnectHappyPath() { if (!IsBenchDeviceAvailable()) { @@ -117,16 +118,29 @@ public void ConnectStreamDisconnect_HappyPath() connect.AsButton().Invoke(); // Wait for the connected-devices list to show at least one row. - var deviceList = FindByAutomationId(mainWindow, cf, DEVICE_LIST_ID, - "Connected-devices list container."); - WaitFor(() => deviceList.FindAllChildren().Length > 0, DEVICE_APPEAR_TIMEOUT, + // Re-find the list inside the predicate each poll: UI Automation + // elements can become stale across major tree transitions (e.g. + // when the ConnectionDialog closes), and a cached element that's + // gone stale will keep throwing inside WaitFor until timeout. + WaitFor( + () => FindListChildren(mainWindow, cf, DEVICE_LIST_ID).Length > 0, + DEVICE_APPEAR_TIMEOUT, "Device did not appear in the connected list."); // ----- Enable first channel ----- + // Set the toggle deterministically to the 'On' state rather than + // unconditionally inverting it. If the channel was already enabled + // (e.g., from persisted UI state or a prior run on the bench), + // a blind Toggle() would disable it and the rest of the flow + // (Start streaming, graph proof-of-life, Disconnect) would fail + // or observe no data. var firstChannel = FindByAutomationId(mainWindow, cf, FIRST_CHANNEL_TOGGLE_ID, "Enable-toggle on the first analog channel."); - // Toggle controls in MahApps are typically ToggleButtons; click via Invoke. - firstChannel.AsToggleButton().Toggle(); + var firstChannelToggle = firstChannel.AsToggleButton(); + if (firstChannelToggle.ToggleState != ToggleState.On) + { + firstChannelToggle.Toggle(); + } // ----- Start streaming, dwell, check graph has data ----- var start = FindByAutomationId(mainWindow, cf, START_STREAMING_ID, @@ -173,7 +187,9 @@ public void ConnectStreamDisconnect_HappyPath() "Disconnect command button."); disconnect.AsButton().Invoke(); - WaitFor(() => deviceList.FindAllChildren().Length == 0, DEVICE_APPEAR_TIMEOUT, + WaitFor( + () => FindListChildren(mainWindow, cf, DEVICE_LIST_ID).Length == 0, + DEVICE_APPEAR_TIMEOUT, "Device was not removed from the connected list after disconnect."); } finally @@ -206,6 +222,20 @@ private static AutomationElement FindByAutomationId( return element!; } + /// + /// Re-finds the list element by AutomationId and returns its children. + /// Always re-locates the parent on each call so a stale AutomationElement + /// from a prior UI-tree refresh can't poison a polling loop. Returns an + /// empty array if the list is currently absent (the caller's WaitFor + /// predicate keeps polling until it appears). + /// + private static AutomationElement[] FindListChildren( + AutomationElement scope, ConditionFactory cf, string automationId) + { + var element = scope.FindFirstDescendant(cf.ByAutomationId(automationId)); + return element is null ? Array.Empty() : element.FindAllChildren(); + } + /// /// Polls the app's top-level windows for one whose title contains /// (case-insensitive). The ConnectionDialog @@ -238,10 +268,8 @@ private static Window WaitForTopLevelWindow( Thread.Sleep(200); } - var detail = lastException is null - ? string.Empty - : $" Last exception while enumerating top-level windows: {lastException.GetType().Name}: {lastException.Message}"; - Assert.Fail($"{failureMessage} (waited {timeout.TotalSeconds:F0}s).{detail}"); + Assert.Fail(BuildTimeoutMessage(failureMessage, timeout, + "enumerating top-level windows", lastException)); throw new InvalidOperationException("unreachable; Assert.Fail throws."); } @@ -253,7 +281,10 @@ private static void WaitFor(Func condition, TimeSpan timeout, string failu { try { - if (condition()) return; + if (condition()) + { + return; + } } catch (Exception ex) { @@ -265,9 +296,22 @@ private static void WaitFor(Func condition, TimeSpan timeout, string failu Thread.Sleep(200); } - var detail = lastException is null + Assert.Fail(BuildTimeoutMessage(failureMessage, timeout, + "polling", lastException)); + } + + /// + /// Builds a uniform "timed out" failure message that appends the last + /// observed exception (if any). Kept under the 120-column limit by + /// constructing the exception suffix on its own line. + /// + private static string BuildTimeoutMessage( + string failureMessage, TimeSpan timeout, string context, Exception? lastException) + { + var suffix = lastException is null ? string.Empty - : $" Last exception during polling: {lastException.GetType().Name}: {lastException.Message}"; - Assert.Fail($"{failureMessage} (waited {timeout.TotalSeconds:F0}s).{detail}"); + : $" Last exception during {context}: " + + $"{lastException.GetType().Name}: {lastException.Message}"; + return $"{failureMessage} (waited {timeout.TotalSeconds:F0}s).{suffix}"; } } From 17ab383a43ab9d3b58aae24c68d5e58a493c414e Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:49:48 -0600 Subject: [PATCH 5/9] Apply Qodo pass 3: 3 new /improve suggestions on lifecycle helpers New suggestions surfaced post-pass-2: - Make toggle state deterministic (importance 8): the pass-2 fix reads ToggleState and only calls Toggle() when not already On, but WPF data-binding updates the toggle's reported state asynchronously after Toggle() returns. If the test proceeds to Start streaming before the toggle bound back to its data context, the channel may effectively be off when Start fires. Add a WaitFor on the post- toggle state (re-finding the element each poll so a stale UIA reference can't poison the wait) with a new TOGGLE_PROPAGATION_TIMEOUT (5s). - Set correct working directory (importance 7): replace the bare Application.Launch(exePath) with Application.Launch(ProcessStartInfo) that sets WorkingDirectory = exe's folder. This avoids file- resolution surprises when the app uses relative paths for config / resources and the test runner's cwd is wherever the IDE/MSTest was launched from. Also avoids breaking on CI agents that run from unrelated work trees. - Ensure process fully terminates (importance 5): after Kill() we Dispose()d immediately, but Kill() is async at the OS layer - the process can still hold file/socket handles for a beat afterwards. Add WaitForExit(grace) via a Process.GetProcessById lookup (FlaUI's Application exposes ProcessId but not the underlying Process). The new WaitForExitByProcessId helper swallows the expected ArgumentException when the PID is already gone. Persistent comment carry-over (review #3, #4, #6, #11-#15): Qodo's persistent rollup still lists the original pass-1 findings against the OLD file paths (ConnectStreamDisconnectTest.cs without trailing s, MainWindowSmokeTest.cs likewise). The actual fixes have already landed - the new files at HEAD don't have the flagged patterns - so these are stale carry-overs from earlier passes. Qodo's review of the latest commit (a2f9571) marks 7 of the 11 original findings as Resolved; the remainder are misclassified due to the same stale-snapshot behavior. The only genuinely active requirement gap is #6 (Graph non-zero data unverified). That's structurally not fixable from the FlaUI side - documented in the pass-2 PR comment. --- .../UITests/ConnectStreamDisconnectTests.cs | 13 ++++++ Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs | 40 ++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs index e05069dd..d7b01be9 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -65,6 +65,7 @@ public class ConnectStreamDisconnectTests private static readonly TimeSpan MAIN_WINDOW_TIMEOUT = TimeSpan.FromSeconds(60); private static readonly TimeSpan DEVICE_APPEAR_TIMEOUT = TimeSpan.FromSeconds(15); private static readonly TimeSpan CONNECTION_DIALOG_TIMEOUT = TimeSpan.FromSeconds(10); + private static readonly TimeSpan TOGGLE_PROPAGATION_TIMEOUT = TimeSpan.FromSeconds(5); private static readonly TimeSpan STREAMING_DWELL_TIME = TimeSpan.FromSeconds(3); private static readonly TimeSpan SHUTDOWN_GRACE = TimeSpan.FromSeconds(5); @@ -140,6 +141,18 @@ public void ConnectStreamDisconnectHappyPath() if (firstChannelToggle.ToggleState != ToggleState.On) { firstChannelToggle.Toggle(); + + // Wait for the toggle state change to propagate through the + // WPF data-binding before asserting downstream stream behavior. + // Re-find the element each poll so we don't rely on a possibly + // stale AutomationElement reference after the UI updates. + WaitFor( + () => FindByAutomationId(mainWindow, cf, FIRST_CHANNEL_TOGGLE_ID, + "First-channel toggle (post-toggle).") + .AsToggleButton() + .ToggleState == ToggleState.On, + TOGGLE_PROPAGATION_TIMEOUT, + "First channel did not reach the 'On' state after toggling."); } // ----- Start streaming, dwell, check graph has data ----- diff --git a/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs b/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs index 5b9ca821..8991d265 100644 --- a/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs +++ b/Daqifi.Desktop.Test/UITests/UIAppLifecycle.cs @@ -1,5 +1,7 @@ using System; using System.ComponentModel; +using System.Diagnostics; +using System.IO; using System.Threading; using FlaUI.Core; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -43,7 +45,17 @@ public static Application LaunchOrInconclusive(string exePath) { try { - return Application.Launch(exePath); + // Set WorkingDirectory to the exe's folder so the app's relative + // path lookups (config files, resource probing, settings stores) + // resolve from a known location instead of inheriting the test + // runner's cwd (which can be anything from MSTest's bin/ down to + // wherever the IDE was launched from). + var startInfo = new ProcessStartInfo(exePath) + { + WorkingDirectory = Path.GetDirectoryName(exePath) + ?? Environment.CurrentDirectory, + }; + return Application.Launch(startInfo); } catch (Win32Exception ex) when (ex.NativeErrorCode == 740 /* ERROR_ELEVATION_REQUIRED */) { @@ -87,6 +99,13 @@ public static void CloseAppGracefully(Application? app, TimeSpan grace) if (!app.HasExited) { app.Kill(); + // Kill() is asynchronous - the OS posts the termination but the + // process can still hold file/socket handles for a moment after. + // Block here so callers see a fully-exited process when + // Dispose() runs, preventing stray processes between consecutive + // test runs. FlaUI's Application doesn't expose the underlying + // Process directly, but we can fetch it by ProcessId. + WaitForExitByProcessId(app.ProcessId, grace); } } catch @@ -98,4 +117,23 @@ public static void CloseAppGracefully(Application? app, TimeSpan grace) app.Dispose(); } } + + /// + /// Blocks until the OS process with the given PID has exited or the grace + /// window expires. Swallows the (expected) + /// from when the process is + /// already gone by the time we look it up. + /// + private static void WaitForExitByProcessId(int processId, TimeSpan grace) + { + try + { + using var process = Process.GetProcessById(processId); + process.WaitForExit((int)grace.TotalMilliseconds); + } + catch (ArgumentException) + { + // Process already exited - no further wait needed. + } + } } From 25afc80739f492db4f1af7a97c900a33672d39a9 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:55:17 -0600 Subject: [PATCH 6/9] Apply Qodo pass 4: device-missing skip + list-row counting Pass 4 new findings: Bug fixes: - Disconnect check false positive (#8): FindListChildren returned Array.Empty when the list element was missing, and the disconnect WaitFor predicate just checked Length == 0 - so a missing list (e.g. an AutomationId regression or a stale UIA tree) was indistinguishable from an empty list, and the test would pass for the wrong reason. Replaced with a ListItemSnapshot record that carries both ListFound and ItemCount, and made the disconnect predicate require ListFound && ItemCount == 0. Requirement gap: - Device-missing path fails test (#1): the env-var gate handles the no-bench-rig case, but if DAQIFI_BENCH_DEVICE_AVAILABLE=1 is set and the device fails to actually appear in the connected- devices list (powered off, cable disconnected, USB stack glitch), the test was failing (Assert.Fail) instead of skipping (Assert.Inconclusive). #531 compliance requires Phase 2 to skip on no-discoverable-device. Added a WaitForOrInconclusive helper that has the same poll semantics as WaitFor but reports timeout via Assert.Inconclusive; used it for the post-connect device- appearance wait. Refactored WaitFor + WaitForOrInconclusive to share a private TryPoll loop so the two paths can't diverge. Suggestions (/improve): - Filter list rows correctly (importance 8) + Count real list items only (importance 7): renamed FindListChildren -> FindListItems and switched from FindAllChildren() (which returns every visual child, including scrollbars and item-container headers) to FindAllDescendants(ControlType.ListItem) with a DataItem fallback for DataGrid-style rows. This is the same fix that closes both improve suggestions in one change. SKIPPED: - Wait for streaming readiness (importance 3): same as pass-2's "Poll for streaming start state" - the dwell is intentional to let unobservable graph points accumulate, not a poor-man's wait for an unrelated UI signal. Replacing it with Stop-button-enabled polling would return immediately on Start (the Stop button enables synchronously) and defeat the purpose. Persistent comment carry-over (review #4 AddDeviceButtonId naming, #5 Inline catch/finally braces, plus the #12-#15 carry-overs): all reference the OLD file path `ConnectStreamDisconnectTest.cs` (no trailing s). The actual constants in the new ConnectStreamDisconnectTests.cs are already SCREAMING_SNAKE_CASE, and the teardown uses Allman braces via UIAppLifecycle. CloseAppGracefully. These are stale Qodo-snapshot carry-overs from earlier passes that the persistent comment hasn't retired. --- .../UITests/ConnectStreamDisconnectTests.cs | 112 +++++++++++++++--- 1 file changed, 98 insertions(+), 14 deletions(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs index d7b01be9..78707c3e 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -123,10 +123,21 @@ public void ConnectStreamDisconnectHappyPath() // elements can become stale across major tree transitions (e.g. // when the ConnectionDialog closes), and a cached element that's // gone stale will keep throwing inside WaitFor until timeout. - WaitFor( - () => FindListChildren(mainWindow, cf, DEVICE_LIST_ID).Length > 0, + // + // If the device never appears within the timeout, treat it as + // "bench device not actually discoverable" and skip + // (Assert.Inconclusive) rather than fail - the env-var gate told + // us a device was *expected*, but the connect path can still no-op + // when the hardware is powered off or the cable is unplugged. + // Phase 2 must skip-on-unavailable per the #531 compliance bar + // (Qodo review #1). + WaitForOrInconclusive( + () => FindListItems(mainWindow, cf, DEVICE_LIST_ID).ItemCount > 0, DEVICE_APPEAR_TIMEOUT, - "Device did not appear in the connected list."); + "Device did not appear in the connected list within " + + $"{DEVICE_APPEAR_TIMEOUT.TotalSeconds:F0}s. The " + + $"{BENCH_AVAILABLE_ENV_VAR} env var is set but no device was " + + "discovered - check the USB connection / power state."); // ----- Enable first channel ----- // Set the toggle deterministically to the 'On' state rather than @@ -200,8 +211,17 @@ public void ConnectStreamDisconnectHappyPath() "Disconnect command button."); disconnect.AsButton().Invoke(); + // Require BOTH that the list element still exists AND that its + // data-item count is zero. Without the ListFound guard, an + // AutomationId regression (or a transient UIA-tree drop) would + // make `ItemCount == 0` pass for the wrong reason: "the list is + // gone" looks identical to "the list is empty" (Qodo review #8). WaitFor( - () => FindListChildren(mainWindow, cf, DEVICE_LIST_ID).Length == 0, + () => + { + var snap = FindListItems(mainWindow, cf, DEVICE_LIST_ID); + return snap.ListFound && snap.ItemCount == 0; + }, DEVICE_APPEAR_TIMEOUT, "Device was not removed from the connected list after disconnect."); } @@ -236,17 +256,41 @@ private static AutomationElement FindByAutomationId( } /// - /// Re-finds the list element by AutomationId and returns its children. + /// Result of looking up a list container by AutomationId and counting its + /// data rows. distinguishes "list element missing" + /// from "list found and empty" - a disconnect predicate that just checked + /// for "0 children" would otherwise produce a false-positive pass when + /// the AutomationId regressed (Qodo review #8). + /// + private readonly record struct ListItemSnapshot(bool ListFound, int ItemCount); + + /// + /// Re-finds the list element by AutomationId and counts its data rows. /// Always re-locates the parent on each call so a stale AutomationElement - /// from a prior UI-tree refresh can't poison a polling loop. Returns an - /// empty array if the list is currently absent (the caller's WaitFor - /// predicate keeps polling until it appears). + /// from a prior UI-tree refresh can't poison a polling loop. Counts only + /// ListItem / DataItem descendants rather than every visual + /// child, because a WPF ItemsControl's child collection includes + /// scrollbars, item-container headers, etc., which would otherwise let + /// Length > 0 succeed even when no data rows exist. /// - private static AutomationElement[] FindListChildren( + private static ListItemSnapshot FindListItems( AutomationElement scope, ConditionFactory cf, string automationId) { var element = scope.FindFirstDescendant(cf.ByAutomationId(automationId)); - return element is null ? Array.Empty() : element.FindAllChildren(); + if (element is null) + { + return new ListItemSnapshot(ListFound: false, ItemCount: 0); + } + + var listItems = element.FindAllDescendants(cf.ByControlType(ControlType.ListItem)); + if (listItems.Length > 0) + { + return new ListItemSnapshot(ListFound: true, ItemCount: listItems.Length); + } + + // DataGrid rows typically present as DataItem rather than ListItem. + var dataItems = element.FindAllDescendants(cf.ByControlType(ControlType.DataItem)); + return new ListItemSnapshot(ListFound: true, ItemCount: dataItems.Length); } /// @@ -288,15 +332,56 @@ private static Window WaitForTopLevelWindow( private static void WaitFor(Func condition, TimeSpan timeout, string failureMessage) { + if (TryPoll(condition, timeout, out var lastException)) + { + return; + } + + Assert.Fail(BuildTimeoutMessage(failureMessage, timeout, + "polling", lastException)); + } + + /// + /// Same poll-then-give-up shape as , but reports + /// timeout via rather than + /// . Used when "condition didn't become + /// true" means "the bench device isn't actually available" (the env-var + /// gate says we should run, but the hardware turned out to be absent / + /// powered off / unplugged) - which the #531 compliance bar requires to + /// be a skip, not a failure. + /// + private static void WaitForOrInconclusive( + Func condition, TimeSpan timeout, string failureMessage) + { + if (TryPoll(condition, timeout, out var lastException)) + { + return; + } + + Assert.Inconclusive(BuildTimeoutMessage(failureMessage, timeout, + "polling", lastException)); + } + + /// + /// Shared poll loop for and + /// . Returns true when + /// evaluated to true before the deadline; + /// returns false on timeout, with the last observed exception (if + /// any) emitted via so the caller can + /// surface it. + /// + private static bool TryPoll( + Func condition, TimeSpan timeout, out Exception? lastException) + { + lastException = null; var deadline = DateTime.UtcNow + timeout; - Exception? lastException = null; while (DateTime.UtcNow < deadline) { try { if (condition()) { - return; + return true; } } catch (Exception ex) @@ -309,8 +394,7 @@ private static void WaitFor(Func condition, TimeSpan timeout, string failu Thread.Sleep(200); } - Assert.Fail(BuildTimeoutMessage(failureMessage, timeout, - "polling", lastException)); + return false; } /// From 004c85768c5a7306cbfd6562de1968943e73d323 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 19:59:14 -0600 Subject: [PATCH 7/9] Apply Qodo pass 5: visible-update streaming check + virtualization-safe counts Pass 5 new findings (both /improve, high-value): Importance 9 - Assert visible update during streaming: Replaced the fixed Thread.Sleep(STREAMING_DWELL_TIME) with a WaitFor that polls until the live graph's BoundingRectangle changes from its pre-stream baseline (while remaining onscreen and enabled). This gives us: - A concrete proof-of-life signal ("the plot actually updated") instead of just "we waited 3s and the control is still there" - An early exit when the change is visible quickly (typically < 1s on the bench) instead of always burning the full dwell - A specific failure message ("did not visibly update") that points at the real problem instead of just timing out The dwell still bounds the wait at STREAMING_DWELL_TIME (3s) so we don't extend the worst-case test runtime. Bounded poll, same budget, better signal. Importance 7 - Avoid virtualization false item counts: The pass-4 FindListItems counted realized ListItem / DataItem descendants only. WPF UI virtualization can leave items unrealized in the UIA tree until they scroll into view, so a ListBox with 10 items but only 5 currently realized would report ItemCount=5 (or 0 if all are offscreen). For the device list this mostly doesn't matter (typically a couple of devices, all realized), but the predicate is now used for both appearance and disappearance checks - a false 0 would be a false-positive pass. Restructured FindListItems to try, in order: 1. GridPattern.RowCount - logical row count, virtualization-safe 2. AsListBox().Items.Length - same, for ListBox-derived controls 3. ListItem descendants - last-resort fallback (current behavior) 4. DataItem descendants - DataGrid fallback Convergence trajectory: pass 5 marked 11 of the prior 13 review items as Resolved. Only remaining "active" findings in the review rollup are #6 (Graph non-zero data unverified - structurally unfixable, documented skip) and the #12-#17 carry-overs against the OLD ConnectStreamDisconnectTest.cs path (no trailing s) that Qodo's persistent comment hasn't retired. Real review-surface state is now effectively converged - any actual code-quality issues found post-pass-1 have been addressed. --- .../UITests/ConnectStreamDisconnectTests.cs | 95 ++++++++++++------- 1 file changed, 63 insertions(+), 32 deletions(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs index 78707c3e..5e443a24 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -171,35 +171,38 @@ public void ConnectStreamDisconnectHappyPath() "Start-streaming command button."); start.AsButton().Invoke(); - // Capture graph geometry BEFORE the dwell so we can detect that data - // actually arrived during streaming (the rectangle grows / changes as - // points are plotted). UI-Automation can't see OxyPlot/LiveCharts data - // directly, so this is the best UIA-only proxy for "non-zero data" we - // can do until the XAML grows an automation-visible point-count probe - // (tracked under #531). + // Capture graph geometry BEFORE streaming so we have a baseline to + // diff against. UI Automation can't see OxyPlot / LiveCharts plot + // data directly, so the best UIA-only proxy for "data is arriving" + // is "the live graph's bounding rectangle changed during the + // dwell". Once the XAML grows an automation-visible point-count + // probe (tracked under #531) this can become a hard assertion. var liveGraph = FindByAutomationId(mainWindow, cf, LIVE_GRAPH_ID, - "Live graph control. Should contain non-zero point count after dwell."); + "Live graph control. Should visibly update after streaming starts."); + Assert.IsTrue(liveGraph.BoundingRectangle.Width > 0, + "Pre-stream graph rectangle was empty; the control wasn't laid out before Start."); var preStreamRect = liveGraph.BoundingRectangle; - Thread.Sleep(STREAMING_DWELL_TIME); - - // Re-fetch in case the surface was lazy-bound on Start. - liveGraph = FindByAutomationId(mainWindow, cf, LIVE_GRAPH_ID, - "Live graph control (post-dwell)."); - - Assert.IsFalse(liveGraph.IsOffscreen, "Live graph was offscreen after Start."); - Assert.IsTrue(liveGraph.BoundingRectangle.Width > 0 - && liveGraph.BoundingRectangle.Height > 0, - "Live graph had zero-sized bounding box; streaming likely did not start."); - - // Proxy data-arrival check: enabled-state should remain visible and the - // graph's rectangle stayed non-empty across the dwell. A stronger - // assertion needs an AutomationProperties.HelpText (or similar) bound - // to the live point count - tracked under #531 follow-up. - Assert.IsTrue(liveGraph.IsEnabled, - "Live graph was disabled after streaming dwell; streaming likely did not start."); - Assert.IsTrue(preStreamRect.Width > 0, - "Pre-stream graph rectangle was empty; the control wasn't laid out before Start."); + // Poll-and-detect rather than fixed-sleep: WaitFor returns as soon + // as we observe a visible change to the graph (and skips the + // remaining dwell), but if nothing has changed by the deadline we + // still spent the same wall-clock budget as the old Thread.Sleep + // and get a precise failure message ("did not visibly update"). + WaitFor( + () => + { + var graph = FindByAutomationId(mainWindow, cf, LIVE_GRAPH_ID, + "Live graph control (streaming-update poll)."); + var rect = graph.BoundingRectangle; + return !graph.IsOffscreen + && graph.IsEnabled + && rect.Width > 0 + && rect.Height > 0 + && !rect.Equals(preStreamRect); + }, + STREAMING_DWELL_TIME, + "Live graph did not visibly update during the streaming dwell; " + + "streaming may not have started or data is not reaching the plot."); // ----- Stop streaming ----- var stop = FindByAutomationId(mainWindow, cf, STOP_STREAMING_ID, @@ -267,11 +270,16 @@ private static AutomationElement FindByAutomationId( /// /// Re-finds the list element by AutomationId and counts its data rows. /// Always re-locates the parent on each call so a stale AutomationElement - /// from a prior UI-tree refresh can't poison a polling loop. Counts only - /// ListItem / DataItem descendants rather than every visual - /// child, because a WPF ItemsControl's child collection includes - /// scrollbars, item-container headers, etc., which would otherwise let - /// Length > 0 succeed even when no data rows exist. + /// from a prior UI-tree refresh can't poison a polling loop. + /// + /// Counts use UI Automation patterns first (Grid.RowCount or + /// ListBox.Items.Length) because WPF UI virtualization can leave rows + /// unrealized (offscreen list items are not present as descendants in the + /// UIA tree until they scroll into view) - so counting realized + /// descendants alone would report 0 even when the list has items. The + /// realized-descendant fallback (ListItem / DataItem) covers the case + /// where the control isn't a ListBox / Grid but still exposes its items + /// directly. /// private static ListItemSnapshot FindListItems( AutomationElement scope, ConditionFactory cf, string automationId) @@ -282,13 +290,36 @@ private static ListItemSnapshot FindListItems( return new ListItemSnapshot(ListFound: false, ItemCount: 0); } + // Prefer GridPattern.RowCount when available - it reports the logical + // row count regardless of virtualization. + var gridPattern = element.Patterns.Grid.PatternOrDefault; + if (gridPattern is not null) + { + return new ListItemSnapshot(ListFound: true, ItemCount: gridPattern.RowCount); + } + + // ListBox.Items.Length is similarly virtualization-safe; AsListBox + // throws if the element isn't a ListBox, so guard with try/catch + // rather than a fragile control-type sniff. + try + { + var listBox = element.AsListBox(); + return new ListItemSnapshot(ListFound: true, ItemCount: listBox.Items.Length); + } + catch + { + // Fall through to the realized-descendant fallback. + } + + // Last-resort fallback: count realized ListItem / DataItem descendants. + // This loses accuracy under virtualization but works for non-virtualized + // ItemsControls and DataGrids. var listItems = element.FindAllDescendants(cf.ByControlType(ControlType.ListItem)); if (listItems.Length > 0) { return new ListItemSnapshot(ListFound: true, ItemCount: listItems.Length); } - // DataGrid rows typically present as DataItem rather than ListItem. var dataItems = element.FindAllDescendants(cf.ByControlType(ControlType.DataItem)); return new ListItemSnapshot(ListFound: true, ItemCount: dataItems.Length); } From 69f939ec81e0cd5fde1b488d46f080989370badb Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 20:04:03 -0600 Subject: [PATCH 8/9] Apply Qodo pass 6: fix baseline race + grid headers + skip flow Pass 6 new findings (all /improve): Importance 9 - Record baseline before starting stream: The pass-5 change kept the original ordering: invoke Start, then capture preStreamRect. WPF data-binding can push the first frame during the Invoke() call, so the rectangle we captured "before the dwell" could already reflect post-stream geometry - the diff then compares a moved baseline against itself, producing a false-fail ("did not visibly update") even when streaming worked. Moved the FindByAutomationId + preStreamRect capture BEFORE start.Invoke() to close the race. Importance 7 - Avoid counting non-data rows: WPF DataGrids include the column-header band as a row in GridPattern.RowCount. Without correction, an empty grid reports RowCount=1 (just the header), the disconnect predicate keeps seeing ItemCount > 0, and the WaitFor times out. Detect a Header descendant and subtract one from RowCount; clamp at 0 so a non- DataGrid with stray Header noise can't go negative. Also fixes a build error from the previous pass: GridPattern.RowCount is AutomationProperty, so .Value is needed to get the int. Importance 5 - Ensure skip cannot fall through: MainWindowSmokeTests was missing the "return; // unreachable" guard after Assert.Inconclusive in the exePath-null branch (though Inconclusive throws, the compiler can't prove that, so the null-suppression operator `!` was needed downstream). Added the explicit return and dropped the suppression operator on both MainWindowSmokeTests.exePath and ConnectStreamDisconnect Tests.exePath. ConnectStreamDisconnectTests already had the return on the env-var path; this aligns the exePath path with the same pattern. Status: pass 5 marked 11 of 13 prior review items as Resolved. Pass 6 introduces no new bug-level findings - all 3 items above are /improve suggestions and address subtle but real issues (race condition, header miscounting, control-flow ergonomics). The persistent comment still carries pass-1 items #12-#17 against the OLD ConnectStreamDisconnectTest.cs path; these are Qodo-rollup artifacts, not active findings against HEAD. --- .../UITests/ConnectStreamDisconnectTests.cs | 46 +++++++++++++------ .../UITests/MainWindowSmokeTests.cs | 3 +- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs index 5e443a24..71dacb5d 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -95,7 +95,7 @@ public void ConnectStreamDisconnectHappyPath() Application? app = null; try { - app = UIAppLifecycle.LaunchOrInconclusive(exePath!); + app = UIAppLifecycle.LaunchOrInconclusive(exePath); using var automation = new UIA3Automation(); var mainWindow = app.GetMainWindow(automation, MAIN_WINDOW_TIMEOUT); @@ -166,23 +166,29 @@ public void ConnectStreamDisconnectHappyPath() "First channel did not reach the 'On' state after toggling."); } - // ----- Start streaming, dwell, check graph has data ----- - var start = FindByAutomationId(mainWindow, cf, START_STREAMING_ID, - "Start-streaming command button."); - start.AsButton().Invoke(); - - // Capture graph geometry BEFORE streaming so we have a baseline to - // diff against. UI Automation can't see OxyPlot / LiveCharts plot - // data directly, so the best UIA-only proxy for "data is arriving" - // is "the live graph's bounding rectangle changed during the - // dwell". Once the XAML grows an automation-visible point-count - // probe (tracked under #531) this can become a hard assertion. + // ----- Capture graph baseline, start streaming, check for update ----- + // Capture graph geometry BEFORE invoking Start so we have a + // baseline to diff against. UI Automation can't see OxyPlot / + // LiveCharts plot data directly, so the best UIA-only proxy for + // "data is arriving" is "the live graph's bounding rectangle + // changed after streaming started". Once the XAML grows an + // automation-visible point-count probe (tracked under #531) this + // can become a hard assertion. + // + // The baseline MUST be captured before invoking Start: otherwise + // the WPF data-binding could have already pushed the first sample + // by the time we read the rectangle, and the post-stream diff + // would compare against an already-updated baseline (false-fail). var liveGraph = FindByAutomationId(mainWindow, cf, LIVE_GRAPH_ID, "Live graph control. Should visibly update after streaming starts."); Assert.IsTrue(liveGraph.BoundingRectangle.Width > 0, "Pre-stream graph rectangle was empty; the control wasn't laid out before Start."); var preStreamRect = liveGraph.BoundingRectangle; + var start = FindByAutomationId(mainWindow, cf, START_STREAMING_ID, + "Start-streaming command button."); + start.AsButton().Invoke(); + // Poll-and-detect rather than fixed-sleep: WaitFor returns as soon // as we observe a visible change to the graph (and skips the // remaining dwell), but if nothing has changed by the deadline we @@ -291,11 +297,23 @@ private static ListItemSnapshot FindListItems( } // Prefer GridPattern.RowCount when available - it reports the logical - // row count regardless of virtualization. + // row count regardless of virtualization. WPF DataGrids commonly count + // their header band as a row in GridPattern; if a Header descendant + // exists, subtract one so an empty grid can reach ItemCount=0 + // (otherwise the disconnect predicate would never see 0 and would + // time out). var gridPattern = element.Patterns.Grid.PatternOrDefault; if (gridPattern is not null) { - return new ListItemSnapshot(ListFound: true, ItemCount: gridPattern.RowCount); + // GridPattern.RowCount is AutomationProperty; .Value reads + // the actual count via UIA. + var rowCount = gridPattern.RowCount.Value; + var header = element.FindFirstDescendant(cf.ByControlType(ControlType.Header)); + if (header is not null && rowCount > 0) + { + rowCount--; + } + return new ListItemSnapshot(ListFound: true, ItemCount: Math.Max(0, rowCount)); } // ListBox.Items.Length is similarly virtualization-safe; AsListBox diff --git a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs index 80f00538..6b11e418 100644 --- a/Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs +++ b/Daqifi.Desktop.Test/UITests/MainWindowSmokeTests.cs @@ -47,12 +47,13 @@ public void MainWindow_Launches_And_HasExpectedTitle() $"Skipped: {DESKTOP_EXE_NAME} was not found. Build the {DESKTOP_PROJECT_NAME} " + "project (Debug or Release, net10.0-windows) before running this UI test. " + "See issue #531 for the full FlaUI scaffold rollout plan."); + return; // unreachable; Assert.Inconclusive throws. } Application? app = null; try { - app = UIAppLifecycle.LaunchOrInconclusive(exePath!); + app = UIAppLifecycle.LaunchOrInconclusive(exePath); using var automation = new UIA3Automation(); var mainWindow = app.GetMainWindow(automation, MAIN_WINDOW_TIMEOUT); From acbd357f441011a60eb1806a041c4d4b735ac052 Mon Sep 17 00:00:00 2001 From: DAQiFi Developer Date: Sun, 17 May 2026 20:07:31 -0600 Subject: [PATCH 9/9] Apply Qodo pass 7: fail-fast on selector regression in device-appear wait Pass 7 new finding: Bug fix - Selector regression becomes skip (review #1): The pass-4 change made the post-connect device-appear wait use WaitForOrInconclusive(ItemCount > 0, ...). That correctly turns "device powered off / cable unplugged" into Inconclusive (which is what #531 compliance requires), but it ALSO turns "the DEVICE_LIST_ID AutomationId is missing / regressed" into Inconclusive - because FindListItems returns (ListFound:false, ItemCount:0) when the list element itself isn't found, so ItemCount > 0 never becomes true, the wait times out, and the selector regression silently presents as "bench device not available". Fix: locate the list element ONCE before the wait (_ = FindByAutomationId(..., DEVICE_LIST_ID, ...)) so a genuinely-missing selector hard-fails with the FindByAutomationId Assert.IsNotNull. Then the WaitForOrInconclusive only converts true "list exists but has no devices yet" timeouts to Inconclusive. This is correctness: the env-var-gated bench run should catch a broken XAML AutomationId hookup as a failure, not quietly skip it as if the hardware were unplugged. Persistent comment carry-over (review #16 Teardown wait, #17 Exe locator): both still reference the OLD ConnectStreamDisconnectTest.cs / MainWindowSmokeTest.cs paths (no trailing s). The actual fixes have been in place since pass 1 (UIAppLifecycle.CloseAppGracefully polls HasExited; TryLocateDesktopExe uses #if DEBUG-aware config preference). These are Qodo persistent-rollup artifacts against the original PR diff, not active findings against HEAD. Suggestions surface: all pass-6 /improve items now resolved on a single commit; no new /improve suggestions in pass 7. The review surface continues marking carry-over items as Resolved (pass 7 marks 15 of 18 prior findings as Resolved). --- .../UITests/ConnectStreamDisconnectTests.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs index 71dacb5d..2b098f82 100644 --- a/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs +++ b/Daqifi.Desktop.Test/UITests/ConnectStreamDisconnectTests.cs @@ -118,6 +118,15 @@ public void ConnectStreamDisconnectHappyPath() "Confirm button on the connection dialog."); connect.AsButton().Invoke(); + // Fail-fast if the device-list AutomationId itself is missing or + // regressed: otherwise FindListItems would return ListFound=false, + // ItemCount=0, the WaitForOrInconclusive predicate (ItemCount > 0) + // would never become true, and we'd time out as Inconclusive - + // masking a real selector regression as "bench device not + // available" (Qodo review pass 7 #1). + _ = FindByAutomationId(mainWindow, cf, DEVICE_LIST_ID, + "Connected-devices list container."); + // Wait for the connected-devices list to show at least one row. // Re-find the list inside the predicate each poll: UI Automation // elements can become stale across major tree transitions (e.g. @@ -130,7 +139,7 @@ public void ConnectStreamDisconnectHappyPath() // us a device was *expected*, but the connect path can still no-op // when the hardware is powered off or the cable is unplugged. // Phase 2 must skip-on-unavailable per the #531 compliance bar - // (Qodo review #1). + // (Qodo review #1, pass 4). WaitForOrInconclusive( () => FindListItems(mainWindow, cf, DEVICE_LIST_ID).ItemCount > 0, DEVICE_APPEAR_TIMEOUT,