chore: integration coverage for SD card logging mode#561
Merged
Conversation
…witch modes The STREAM TO APP / LOG TO DEVICE segmented selector on the Devices pane drawer bound its mode switch to a RadioButton Command. A ButtonBase Command fires only on a real Click, which a background UI-automation host (the FlaUI integration harness) cannot raise — checking the button through the UIA SelectionItem/Toggle pattern raises Checked, not Click — so the device never switched logging mode under automation. Forward each RadioButton's Checked event to SetLoggingModeCommand via a Microsoft.Xaml.Behaviors EventTrigger/InvokeCommandAction. Interactive behavior is unchanged (a click still raises Checked); the SelectedLoggingMode setter is idempotent, so the redundant Checked raised when the OneWay IsChecked binding refreshes after a switch is a harmless no-op. Add literal AutomationIds to the two mode RadioButtons (LoggingModeStreamToApp, LoggingModeLogToDevice) and to the SD-card DATA FORMAT selector (SdCardFormatSelector), whose presence is the independent confirmation that the switch took effect. Surfaced while adding SD-card logging UI coverage (#553). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add SdCardLoggingTests (Ui + RequiresDevice): connect over USB, switch to Log to Device (SD card) mode, enable channels, run a brief logging session, stop, and assert the device logged to its SD card — the device's "Enabled/Disabled SD card logging" log lines plus a new file on the SD card (file count increased) — confirming SD-card mode was used rather than an in-app stream session. - DaqifiAppFixture: SetLoggingMode(bool) and GetSdCardFileCount() helpers, plus the device-drawer / DEVICE-LOGS-sub-tab / refresh plumbing they need. - AutomationIds on the touched SD surfaces: DeviceLogsView REFRESH button, SD status line, and file list; the Logged Data DEVICE LOGS sub-tab. - README: new scenario row, AutomationId map entries, and a gotcha documenting why a RadioButton/ToggleButton bound Command is not drivable from automation (drive it through IsChecked instead). Closes #553. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Review Summary by QodoAdd SD card logging integration test and fix automation-driven mode switching
WalkthroughsDescription• Add SD card logging integration test with device automation helpers • Fix logging-mode selector to work with UI automation via EventTrigger • Add AutomationIds to SD card and logging-mode UI controls • Document automation gotcha for RadioButton/ToggleButton Command binding Diagramflowchart LR
A["UI Automation Test"] -->|SetLoggingMode| B["Logging-Mode RadioButtons"]
B -->|EventTrigger on Checked| C["SetLoggingModeCommand"]
C -->|Updates| D["Shell.IsLogToDeviceMode"]
D -->|Visibility Binding| E["SdCardFormatSelector"]
A -->|GetSdCardFileCount| F["DeviceLogsView"]
F -->|Reads| G["SdCardStatusText"]
A -->|Asserts| H["SD File Count Increased"]
File Changes1. Daqifi.Desktop.UITest/SdCardLoggingTests.cs
|
Contributor
Code Review by Qodo
1. Public test method lacks XML
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+32
to
+36
| [TestMethod] | ||
| [TestCategory("Ui")] | ||
| [TestCategory("RequiresDevice")] | ||
| public void SdCardLogging_LogsToSdCard_NotStream() | ||
| { |
Contributor
There was a problem hiding this comment.
1. Public test method lacks xml 📘 Rule violation ✧ Quality
The new public test method SdCardLogging_LogsToSdCard_NotStream has no XML documentation comment, violating the requirement that all new/modified public members include /// <summary> docs.
Agent Prompt
## Issue description
A new public member (`SdCardLogging_LogsToSdCard_NotStream`) was added without an XML documentation comment containing at least a `<summary>` element.
## Issue Context
The file already uses XML docs for the class, so adding a method-level `<summary>` keeps documentation consistent and meets the public-API documentation rule.
## Fix Focus Areas
- Daqifi.Desktop.UITest/SdCardLoggingTests.cs[32-36]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
Fixed in eb41051 — added an XML <summary> to SdCardLogging_LogsToSdCard_NotStream describing the end-to-end SD-card scenario, consistent with the existing class-level docs.
- Add an XML <summary> to the SdCardLogging_LogsToSdCard_NotStream test method. - Replace the nested retry (an outer 30s loop wrapping GetSdCardFileCount, which itself waited up to 45s and threw on timeout) with WaitForSdCardFileCountAbove: a single overall timeout with a short, bounded per-attempt settle, so one slow attempt can no longer exceed the budget. Factored the shared refresh+parse into ReadSdCardFileCountAfterRefresh. - Reference Microsoft.Xaml.Behaviors.Wpf explicitly in Daqifi.Desktop.csproj (1.1.19, the version MahApps.Metro 2.4.11 resolves) so the XAML EventTrigger no longer relies on an untracked transitive dependency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 19.1%
Daqifi.Desktop.Common - 38.1%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
Contributor
Author
Responses to Qodo reviewThanks for the review. Addressed in 1. Public test method lacks XML doc — ✅ Fixed. Added a 4. Nested retry breaks timeout — ✅ Fixed. Good catch (I'd flagged the same in my own pass). Replaced the outer-30s-wrapping-inner-45s nesting with 5. Implicit XAML Behaviors dependency — ✅ Fixed. Added an explicit 2. Fixed 3. Asserts inside the Act section (AAA) — 🟡 Kept, by design. This is a sequential integration scenario (start → run → stop), where some state is only observable at a specific moment: Verification: build clean; unit gate 274/274; the SD scenario was validated end-to-end on a real device with an SD card. The |
Contributor
Author
|
Re-validated
So the count-wait refactor, the explicit |
tylerkron
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #553.
Adds the FlaUI hardware-in-the-loop scenario for SD card logging mode — one of the gaps surfaced during the Core 0.22.0 validation (#551) — plus the app change and AutomationIds needed to drive it.
What the test does
SdCardLoggingTests.SdCardLogging_LogsToSdCard_NotStream([Ui]+[RequiresDevice]) drives the real GUI out-of-process:Enabled/Disabled SD card logginglog lines and a new file on the SD card (file count increased) — confirming SD-card mode was used, not an in-app stream session.The app fix this surfaced (separate
fix(ui)commit)The logging-mode segmented selector bound its switch to a RadioButton
Command. AButtonBase.Commandfires only on a realClick, which a background UI-automation host cannot raise — checking the button through the UIASelectionItem/Togglepattern raisesChecked, notClick— so the device never switched mode under automation (same class of issue as the channel-tile gotcha).Fix: forward each RadioButton's
Checkedevent toSetLoggingModeCommandvia aMicrosoft.Xaml.BehaviorsEventTrigger/InvokeCommandAction. Interactive behavior is unchanged (a click still raisesChecked); theSelectedLoggingModesetter is idempotent, so the redundantCheckedraised when the OneWayIsCheckedbinding refreshes is a harmless no-op. Controls already driven byIsCheckeditself (StartLoggingToggle, theDeviceLogsTabsub-tab) were automatable as-is — now documented as README gotcha #12.AutomationIds added (touched controls only)
LoggingModeStreamToApp/LoggingModeLogToDevice/SdCardFormatSelector(Devices drawer),DeviceLogsTab(Logged Data), andRefreshSdCardFilesButton/SdCardStatusText/SdCardFileList(DeviceLogsView). All documented in the README AutomationId map.Verification
dotnet format-consistent.🤖 Generated with Claude Code