Skip to content

Add Microsoft.Maui.Client tool for cross-platform device and SDK management#34498

Open
rmarinho wants to merge 94 commits intomainfrom
maui-client-tool-android
Open

Add Microsoft.Maui.Client tool for cross-platform device and SDK management#34498
rmarinho wants to merge 94 commits intomainfrom
maui-client-tool-android

Conversation

@rmarinho
Copy link
Member

@rmarinho rmarinho commented Mar 16, 2026

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Summary

Add the maui CLI tool (Microsoft.Maui.Client) — a cross-platform command-line tool for managing Android development environments, devices, and emulators. This tool powers the VS Code MAUI extension acquisition experience and can also be used standalone.

Built with Spectre.Console for rich terminal UI including interactive prompts, progress bars, colored tables, and tree views. All commands support --json for machine-readable output (consumed by the VS Code extension).

Important

Apple/macOS support (Xcode management, iOS simulators, runtime management) has been intentionally removed from this PR to keep the scope focused on Android. Apple support will be added in a follow-up PR once the macios-devtools package APIs are finalized.

Screenshots

maui --help

maui --help

maui doctor — Environment diagnostics

maui doctor

maui device list — Cross-platform device listing

maui device list

maui android install — Interactive SDK setup

maui android install

Spec PRs

Key Features

Command Description
maui doctor Check system readiness (.NET SDK, MAUI workload, Android SDK/JDK). Supports --fix to auto-resolve issues and --platform to scope checks.
maui android sdk check Detect Android SDK, JDK, build-tools, platforms, emulator
maui android install Bootstrap Android SDK + JDK from scratch with interactive multi-select package picker
maui android jdk check/install Manage OpenJDK 17/21 installation
maui android avd list/create/delete Manage Android Virtual Devices
maui android emulator start/stop Start/stop emulators with cold-boot and wipe-data options
maui device list Cross-platform device/emulator listing with table view
maui version Show tool version, .NET SDK version, and runtime info
--json Machine-readable JSON output on all commands
--dry-run Preview what a command would do without executing
--verbose Show detailed output including process commands

Architecture

src/Tools/
├── Microsoft.Maui.Client/
│   ├── Commands/           # CLI commands (System.CommandLine 2.0.5)
│   │   ├── AndroidCommands.cs          # android command group (partial class)
│   │   ├── AndroidCommands.Install.cs  # android sdk install (interactive + CI)
│   │   ├── AndroidCommands.Jdk.cs      # android jdk check/install
│   │   ├── AndroidCommands.Sdk.cs      # android sdk check/install/uninstall
│   │   ├── AndroidCommands.Emulator.cs # android avd/emulator management
│   │   ├── DeviceCommand.cs            # device list (cross-platform)
│   │   ├── DoctorCommand.cs            # environment diagnostics with --fix
│   │   ├── VersionCommand.cs           # tool version info
│   │   └── CommandExtensions.cs        # shared option parsing helpers
│   ├── Providers/
│   │   └── Android/        # Wraps android-tools NuGet (Xamarin.Android.Tools.AndroidSdk)
│   │       ├── AndroidProvider.cs      # Main provider (SDK/JDK/package management)
│   │       ├── Adb.cs                  # ADB device enumeration
│   │       ├── AvdManager.cs           # AVD create/delete/list
│   │       ├── SdkManager.cs           # SDK package management + license acceptance
│   │       ├── JdkManager.cs           # OpenJDK download/install
│   │       └── AndroidEnvironment.cs   # SDK/JDK path detection
│   ├── Services/           # Cross-platform services
│   │   ├── DeviceManager.cs            # Aggregates devices from all providers
│   │   └── DoctorService.cs            # Health checks (.NET, Android, Windows)
│   ├── Output/             # Dual-mode output (Spectre rich UI + JSON)
│   │   ├── IOutputFormatter.cs         # Output abstraction
│   │   ├── SpectreOutputFormatter.cs   # Rich terminal UI (tables, trees, progress)
│   │   ├── SpectreHelpBuilder.cs       # Custom branded --help
│   │   └── JsonOutputFormatter.cs      # Machine-readable JSON
│   ├── Models/             # Shared data models
│   │   ├── Device.cs                   # Cross-platform device record
│   │   ├── DoctorReport.cs             # Health check results
│   │   └── ErrorResult.cs              # Structured error output
│   ├── Errors/             # Structured error codes
│   │   ├── ErrorCodes.cs               # All error codes (MAUI0001–MAUI0099)
│   │   └── MauiToolException.cs        # Typed exception with error codes
│   ├── Utils/
│   │   ├── ProcessRunner.cs            # Safe process execution with ArgumentList
│   │   └── PlatformDetector.cs         # OS detection and default paths
│   └── Program.cs          # Entry point with service configuration
└── tests/
    └── Microsoft.Maui.Client.UnitTests/  # 118 xUnit tests

Security

  • Input sanitization: All user-provided arguments passed to external processes use ArgumentList (not shell-interpolated strings) to prevent command injection
  • Path validation: JdkManager.ValidateInstallPath() prevents arbitrary directory operations
  • Command parsing: DoctorService.ParseCommand() safely splits command strings into fileName + arguments
  • No shell execution: ProcessRunner uses UseShellExecute = false with direct process invocation

Testing

  • 118 unit tests (xUnit + Spectre.Console.Testing)
  • Tests run on Helix (Windows + macOS) via eng/helix.proj
  • No Moq — hand-written FakeAndroidProvider for full control
  • Tests cover: command parsing, provider logic, service orchestration, output formatting, error codes, platform detection
dotnet test src/Tools/tests/Microsoft.Maui.Client.UnitTests

CI Integration

  • Tool project added to Microsoft.Maui.sln (Any CPU only — no x64/x86)
  • Pack solution filters updated (eng/Microsoft.Maui.Packages.slnf, eng/Microsoft.Maui.Packages-mac.slnf)
  • Unit tests run on Helix via eng/helix.proj (Windows.10.Amd64.Open + osx.15.arm64.maui.open)
  • Dependencies managed via eng/Versions.props and eng/Version.Details.xml

How to Test

# Restore and build
dotnet build src/Tools/Microsoft.Maui.Client -c Release

# Run unit tests
dotnet test src/Tools/tests/Microsoft.Maui.Client.UnitTests -c Release

# Pack and install as global tool
dotnet pack src/Tools/Microsoft.Maui.Client -c Release -o artifacts/nupkg
dotnet tool install --global Microsoft.Maui.Client --version <version> --add-source artifacts/nupkg

# Try it
maui --help
maui doctor
maui android sdk check --json
maui device list

Dependencies Added

Package Version Purpose
System.CommandLine 2.0.5 CLI parsing framework (stable release)
Spectre.Console 0.49.1 Rich terminal UI (tables, trees, progress)
Xamarin.Android.Tools.AndroidSdk 1.0.143-preview.8 Android SDK/JDK management

Future Improvements

See Issue #34619 for multi-model API design ideas including:

  • Split IAndroidProvider into focused interfaces (SDK, JDK, Device, Emulator)
  • Add maui setup unified bootstrap command
  • Add maui config for persistent settings
  • Apple/macOS support: Xcode management, iOS simulators, runtime management (follow-up PR)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34498

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34498"

@rmarinho rmarinho changed the title Add Microsoft.Maui.DevTools CLI tool for cross-platform device and SDK management Add Microsoft.Maui.Client tool for cross-platform device and SDK management Mar 16, 2026
@rmarinho rmarinho force-pushed the maui-client-tool-android branch from a9c263a to 56f2265 Compare March 16, 2026 20:21
@rmarinho
Copy link
Member Author

rmarinho commented Mar 23, 2026

🤖 AI Summary

📊 Expand Full Review
📋 Report — Final Recommendation
📝 Review SessionUpdate sdk · b387d81

📋 Report: Principal Engineer Review Complete

Status: Review and implementation complete

Review Summary

14 commits pushed across multiple review rounds by 3 AI models (Claude Opus 4.6, Gemini 3 Pro, GPT-5.1).

Commits Applied

# Commit Description
1 131e905 Clean up dead code, duplicates, and boilerplate
2 0e8e0ef Split AndroidCommands.cs (1,438→187 lines) into partials
3 3e35aa2 Consolidate env var setup + 656 format fixes
4 18b7e24 Fix tests broken by dead code cleanup
5 ced7e2f Remove CS1591 suppression, clean NuGet.config
6 12c9a5e Fix security (command injection), resource leaks, error handling
7 dd89798 Log exceptions in all 23 catch blocks via Trace.WriteLine
8 7014f20 Fix thread safety, null guard, extract magic numbers
9 2bde4f5 Add SanitizeArg tests, fix unused options, shell injection
10 091fb64 Fix DoctorService command parsing + JdkManager directory safety
11 d966b4f Remove redundant private modifiers, document coding conventions
12 dbc8e71 Add tool + tests to solution and pack filters
13 645d725 Add tests to Helix pipeline

Key Fixes by Category

🔴 Security (3 fixes)

  • Command injection in LogsCommand via unsanitized filter/device params → Added SanitizeArg()
  • Shell injection in XcodeSelect path → Sanitized
  • JdkManager arbitrary directory deletion → Added ValidateInstallPath()

🟡 Correctness (3 fixes)

  • DoctorService.TryFixAsync passing full command string as filename → Added ParseCommand()
  • Program.cs Services setter race condition → Added lock
  • ProcessRunner obsolete method with broken semantics → Removed

🔵 Resource Management (2 fixes)

  • HttpClient created per-call in JdkManager → Made static
  • Process handle leak in AvdManager → Added Dispose()

🟢 Infrastructure (3 fixes)

  • Tool not packaged in CI → Added to sln + pack solution filters
  • Tests not running on Helix → Added to eng/helix.proj
  • 656 formatting violations → Fixed via dotnet format

Test Coverage

  • 118 total tests (31 new: 13 SanitizeArg, 7 ParseCommand, 4 ValidateInstallPath, 7 others)
  • 117 pass, 1 pre-existing failure (unrelated)
  • Tests now run on Helix (Windows + macOS)

Multi-Model Review

  • Gemini 3 Pro + GPT-5.1 + Claude Sonnet 4.5 independently reviewed the API surface
  • All 3 agreed on 5 critical findings (command hierarchy, God Interface, missing setup command)
  • Ideas filed as Issue #34619

Build Status

  • ✅ 0 warnings, 0 errors
  • ✅ Format clean
  • ✅ Pack produces Microsoft.Maui.Client.*.nupkg

@rmarinho rmarinho marked this pull request as ready for review March 24, 2026 18:07
Copilot AI review requested due to automatic review settings March 24, 2026 18:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new cross-platform .NET global tool, Microsoft.Maui.Client (command name: maui), intended to provide a unified CLI for MAUI environment diagnostics (“doctor”), device enumeration, log streaming, and Android/Apple environment management. It also adds a dedicated unit test project and wires it into Helix.

Changes:

  • Add src/Tools/Microsoft.Maui.Client implementation (commands, providers, services, models, output formatters, utilities).
  • Add src/Tools/tests/Microsoft.Maui.Client.UnitTests with fakes and coverage for core helpers and DI setup.
  • Integrate the new test project into Helix and update build metadata/versioning inputs for tool dependencies.

Reviewed changes

Copilot reviewed 70 out of 75 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Tools/Microsoft.Maui.Client/Microsoft.Maui.Client.csproj New global tool project definition (packable tool, dependencies).
src/Tools/Microsoft.Maui.Client/Program.cs CLI entry point, command registration, global options, exception handling.
src/Tools/Microsoft.Maui.Client/ServiceConfiguration.cs DI registration and test service provider helpers.
src/Tools/Microsoft.Maui.Client/Commands/AndroidCommands.cs Android command group + interactive install package selection helpers.
src/Tools/Microsoft.Maui.Client/Commands/AndroidCommands.Install.cs Android environment install workflow (interactive + CI/dry-run).
src/Tools/Microsoft.Maui.Client/Commands/AndroidCommands.Jdk.cs JDK check/install/list commands.
src/Tools/Microsoft.Maui.Client/Commands/AndroidCommandsTests.cs Unit tests for Android command shape/option parsing.
src/Tools/Microsoft.Maui.Client/Commands/CommandExtensions.cs Helper for retrieving command-local option values.
src/Tools/Microsoft.Maui.Client/Commands/DeviceCommand.cs maui device list/logs subcommands.
src/Tools/Microsoft.Maui.Client/Commands/DoctorCommand.cs maui doctor checks + optional --fix.
src/Tools/Microsoft.Maui.Client/Commands/LogsCommand.cs Top-level maui logs streaming implementation and colorization.
src/Tools/Microsoft.Maui.Client/Commands/VersionCommand.cs maui version output in JSON or formatted form.
src/Tools/Microsoft.Maui.Client/Errors/ErrorCodes.cs Structured error code taxonomy for the tool.
src/Tools/Microsoft.Maui.Client/Errors/MauiToolException.cs Tool exception type with remediation metadata.
src/Tools/Microsoft.Maui.Client/Models/Device.cs Cross-platform device schema and related enums/constants.
src/Tools/Microsoft.Maui.Client/Models/DoctorReport.cs Doctor report schema and check/fix metadata.
src/Tools/Microsoft.Maui.Client/Models/ErrorResult.cs JSON error contract and exception-to-error conversion.
src/Tools/Microsoft.Maui.Client/Models/Platforms.cs Platform string constants + normalization/validation helpers.
src/Tools/Microsoft.Maui.Client/Output/IOutputFormatter.cs Formatter interface for JSON vs console output.
src/Tools/Microsoft.Maui.Client/Output/JsonOutputFormatter.cs JSON formatting and serialization options.
src/Tools/Microsoft.Maui.Client/Providers/Android/Adb.cs Wrapper around android-tools ADB device enumeration and stop.
src/Tools/Microsoft.Maui.Client/Providers/Android/AndroidEnvironment.cs ABI/API mapping + env var composition helpers.
src/Tools/Microsoft.Maui.Client/Providers/Android/AvdManager.cs AVD management wrapper around android-tools runners.
src/Tools/Microsoft.Maui.Client/Providers/Android/IAndroidProvider.cs Android provider abstraction + related records.
src/Tools/Microsoft.Maui.Client/Providers/Android/IJdkManager.cs JDK manager abstraction.
src/Tools/Microsoft.Maui.Client/Providers/Android/SdkManager.cs SDK package management wrapper around android-tools.
src/Tools/Microsoft.Maui.Client/Providers/Apple/AppleProvider.cs Apple provider built on xcrun simctl + xcode-select.
src/Tools/Microsoft.Maui.Client/Providers/Apple/IAppleProvider.cs Apple provider abstraction + runtime model.
src/Tools/Microsoft.Maui.Client/Providers/Apple/Simctl.cs simctl process wrapper + JSON parsing to device/runtime models.
src/Tools/Microsoft.Maui.Client/Providers/Apple/XcodeSelect.cs xcode-select and Xcode discovery/switching helper.
src/Tools/Microsoft.Maui.Client/Services/DeviceManager.cs Cross-platform device aggregation + AVD/emulator metadata merging.
src/Tools/Microsoft.Maui.Client/Services/IDeviceManager.cs Device manager abstraction.
src/Tools/Microsoft.Maui.Client/Services/DoctorService.cs Doctor checks, categorization, and --fix command execution helper.
src/Tools/Microsoft.Maui.Client/Services/IDoctorService.cs Doctor service abstraction.
src/Tools/Microsoft.Maui.Client/Utils/PlatformDetector.cs OS/arch detection and default tool path discovery.
src/Tools/Microsoft.Maui.Client/Utils/ProcessRunner.cs Process execution helper (sync/async), sanitization, elevation helpers.
src/Tools/Microsoft.Maui.Client/Scripts/demo-all.sh Scripted demo runner for CLI scenarios.
src/Tools/Microsoft.Maui.Client/Scripts/demo-android-emulator.sh Android emulator demo script.
src/Tools/Microsoft.Maui.Client/Scripts/demo-android-sdk.sh Android SDK demo script.
src/Tools/Microsoft.Maui.Client/Scripts/demo-apple.sh Apple/macOS demo script.
src/Tools/Microsoft.Maui.Client/Scripts/demo-interactive-install.sh Demonstrates interactive UX flows.
src/Tools/Microsoft.Maui.Client/Scripts/demo-json-scripting.sh Demonstrates --json usage for scripting.
src/Tools/Microsoft.Maui.Client/Scripts/demo-overview.sh Quick “version/doctor/device” overview script.
src/Tools/Microsoft.Maui.Client/Scripts/demo-simulator-boot.sh Boots/shuts down an iOS simulator demo.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/Microsoft.Maui.Client.UnitTests.csproj New unit test project for the tool.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/AndroidProviderTests.cs Tests for Android provider fakes and behaviors.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/DeviceManagerTests.cs Tests for device aggregation/merge behavior.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/DoctorServiceTests.cs Tests for doctor check aggregation/summary.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/ErrorCodesTests.cs Tests for error code formatting/grouping.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/MauiToolExceptionTests.cs Tests for exception/remediation shaping.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/OutputFormatterTests.cs Tests for JSON + Spectre formatter behaviors.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/PlatformsTests.cs Tests for platform normalization/validation.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/ProcessRunnerTests.cs Tests for argument sanitization + command parsing + JDK path validation.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/ServiceConfigurationTests.cs Tests for DI registrations and override behavior.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/Fakes/FakeAndroidProvider.cs Test fake Android provider with call tracking.
src/Tools/tests/Microsoft.Maui.Client.UnitTests/Fakes/FakeAppleProvider.cs Test fake Apple provider with call tracking.
eng/helix.proj Add new tool unit tests to Helix test list.
eng/Versions.props Add versions for new tool dependencies.
eng/Version.Details.xml Add dependency tracking entries for new tool dependencies.
eng/Microsoft.Maui.Packages.slnf Include tool project in packaging solution filter.
eng/Microsoft.Maui.Packages-mac.slnf Include tool project in mac packaging solution filter.
NuGet.config Add an additional restore feed needed for new tool dependencies.
.github/copilot-instructions.md Document C# repo conventions for Copilot guidance.

@simonrozsival
Copy link
Member

  • How does this compare to xharness? Is there any overlap or reuse between these two tools?
  • Can it be used with dnx? Would dnx maui doctor work even when the nuget tool name is Microsoft.Maui.Client?

@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The 79 unit tests are well-structured and cover the most critical paths (security, Android commands, core services), but Apple commands and several CLI command handlers have zero test coverage despite a FakeAppleProvider being available.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34498 — Add Microsoft.Maui.Client tool for cross-platform device and SDK management
Test files evaluated: 10 (+ 2 fake helpers)
Fix files: 40 implementation files


Overall Verdict

⚠️ Tests need improvement

The unit tests are well-structured and appropriately cover security-sensitive code, Android commands, core services, and output formatting. However, AppleCommands (4 subcommand groups: install, simulator, runtime, xcode), DeviceCommand, DoctorCommand, and VersionCommand have zero test coverage — despite a FakeAppleProvider already being available for Apple testing.


1. Fix Coverage — ⚠️

The tested code paths are covered well, but a significant portion of the codebase is entirely untested:

Covered:

  • ProcessRunner.SanitizeArg() — all injection vectors tested
  • JdkManager.ValidateInstallPath() — home dir, system dirs, shallow paths
  • AndroidCommands — all subcommands and option parsing
  • DoctorService.RunAllChecksAsync() — happy path with fakes
  • DeviceManager — device listing, filtering, emulator merging
  • OutputFormatter (JSON + Spectre) — key output scenarios
  • MauiToolException — all factory methods
  • ServiceConfiguration — DI wiring
  • Platforms — normalization and validation

Not covered:

  • AppleCommands — zero tests (simulator CRUD, runtime install/check, Xcode select, all --json paths)
  • DeviceCommand — cross-platform device listing and --platform filter
  • DoctorCommand — command handler (not just DoctorService)
  • VersionCommand — version output
  • DoctorService.RunCategoryChecksAsync() — only "dotnet" category tested; "android", "apple", "windows" branches untested
  • Error paths in DoctorService — no tests for when checks throw exceptions

2. Edge Cases & Gaps — ⚠️

Covered:

  • Null input to SanitizeArgArgumentNullException
  • All 8 shell metacharacters rejected by SanitizeArg
  • Spaces in arguments → quoted output
  • ValidateInstallPath home/system/shallow-path guards
  • DeviceManager merging running emulators with AVDs (two scenarios: via Details dict and via EmulatorId fallback)
  • Null/empty/invalid platform strings in Platforms.IsValid

Missing:

  • DeviceManager.GetAllDevicesAsync() when both providers throw — no exception propagation test
  • DoctorService.RunAllChecksAsync() when dotnet isn't on PATH — unhappy path not tested
  • DoctorService.RunCategoryChecksAsync("android"), "apple", "windows" categories — only "dotnet" is implicitly tested
  • Apple command option parsing (e.g., apple simulator create --name X --device-type Y --runtime Z) — no equivalent of AndroidCommandsTests for Apple
  • DeviceCommand --platform filtering (android/ios/all)
  • --json flag on Apple/Device commands producing valid JSON
  • SanitizeArg with Unicode or very long strings — minor

3. Test Type Appropriateness — ✅

Current: Unit Tests (xUnit + Spectre.Console.Testing)
Recommendation: Same — unit tests are the correct and most efficient choice for a CLI tool. No platform rendering, device interaction, or visual UI is involved. The use of hand-written fakes over Moq is a good pattern for control and readability.

The FakeAndroidProvider and FakeAppleProvider are well-designed and fully implement their interfaces with configurable return values and call-tracking lists. They are ready to be used for Apple command tests.


4. Convention Compliance — ✅

Automated checks found zero convention issues:

  • All tests use [Fact] / [Theory] (xUnit conventions)
  • Test classes are in the correct namespace (Microsoft.Maui.Client.UnitTests)
  • [InlineData] used correctly on Theory methods
  • Arrange/Act/Assert pattern consistently followed
  • No Thread.Sleep or async anti-patterns detected

5. Flakiness Risk — ✅ Low

All tests are pure unit tests using hand-written fakes — no process spawning, no file system I/O, no network calls in the test code itself. Risk is minimal.


6. Duplicate Coverage — ✅ No duplicates

This is a brand-new tool with no pre-existing tests. No duplication concerns.


7. Platform Scope — ⚠️

The PR description states tests run on Helix (Windows + macOS) via eng/helix.proj. Since these are pure unit tests with no platform-specific logic in the test code, they should pass on both platforms.

However, the Apple-specific features (simulators, Xcode, runtimes) are macOS-only and have zero tests, meaning the macOS-specific code paths in AppleCommands are entirely untested in CI.


8. Assertion Quality — ✅

Assertions are specific and meaningful throughout:

  • Assert.Equal(expectedValue, actualValue) — used consistently
  • Assert.Contains(collection, predicate) — used for command structure checks
  • Assert.Single(collection) — verifies exact counts
  • Assert.Throws(T)() — used for exception assertions
  • Assert.Same() — used to verify DI singleton behavior
  • No magic-number assertions or Assert.True(true) anti-patterns found

9. Fix-Test Alignment — ⚠️

The most significant misalignment is the Apple platform coverage:

  • AppleCommands.cs adds 4 command groups (install, simulator, runtime, xcode) with ~15 subcommands
  • FakeAppleProvider exists and implements the full IAppleProvider interface
  • Zero tests for AppleCommands — the fake is defined but never used by any test class for command-level testing

Similarly, DeviceCommand.cs (the cross-platform device list command highlighted in PR screenshots) is untested at the command level, even though DeviceManager is tested at the service level.


Recommendations

  1. Add AppleCommandsTests.cs — Mirror the structure of AndroidCommandsTests.cs. Test command structure (subcommands exist, options have correct names and defaults), option parsing, and the --dry-run path. The FakeAppleProvider is already available.

  2. Add DeviceCommandTests.cs — Test device list command structure, --platform option parsing, and that it wires up to IDeviceManager.

  3. Add category coverage for DoctorService.RunCategoryChecksAsync() — Test "android", "apple" (gated on platform), and "windows" category branches with fakes to verify correct delegation.

  4. Test DoctorService error paths — What happens if CheckHealthAsync() throws? Does the service propagate or swallow the exception? Add a test with a throwing fake.

  5. Consider --json output shape tests for Apple/Device commands — The PR description highlights --json as a key feature for VS Code extension integration. A test verifying JSON output structure for device list --json would guard against regressions.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The unit tests provide solid coverage of the new CLI tool's core logic, but DoctorService.RunCategoryChecksAsync is entirely untested and a flakiness risk exists in the progress test.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34498 — MAUI CLI Client Tool (Android)
Test files evaluated: 11 unit test files
Fix files: 35 (new CLI tool: src/Tools/Microsoft.Maui.Client/)


Overall Verdict

⚠️ Tests need improvement

The unit tests are well-structured and use a good fake/mock pattern (FakeAndroidProvider). Coverage is solid for the happy paths. However, DoctorService.RunCategoryChecksAsync is completely untested, a Task.Delay(100) creates a flakiness risk, and ProcessRunner's actual process execution is not tested at all.


1. Fix Coverage — ⚠️

The tests cover most major components: command parsing (AndroidCommandsTests), device management (DeviceManagerTests), Android provider interactions (AndroidProviderTests), doctor service health checks (DoctorServiceTests), output formatting (OutputFormatterTests), and process argument sanitization (ProcessRunnerTests).

Gap: DoctorService.RunCategoryChecksAsync (50+ lines of logic including category branching and error throwing) has zero test coverage. This is a meaningful public API on the service.

Gap: ProcessRunner.RunSync and RunAsync (the actual process execution methods) are not tested. Only the internal SanitizeArg helper is tested. The process execution infrastructure handles timeouts, stdin piping, and cancellation — none of this is verified.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Valid and invalid SanitizeArg inputs including command-injection characters
  • JdkManager.ValidateInstallPath for home, system, and shallow directories
  • Device merging logic (emulator + AVD merging by avd key and EmulatorId fallback)
  • Platforms.IsValid / Normalize with case variants and aliases
  • Error code format and category grouping

Missing:

  • DoctorService.RunCategoryChecksAsync — the method routes by "dotnet", "android", "windows", and throws MauiToolException(ErrorCodes.InvalidArgument, ...) on unknown categories. None of these branches are tested.
  • DoctorService.RunCategoryChecksAsync("windows") on non-Windows — returns a Skipped check; this branch is untested.
  • ProcessRunner actual execution — timeout behavior, cancellation, non-zero exit codes, and stderr capture are not covered.
  • PlatformDetector.Paths.GetAndroidSdkPath() — the ANDROID_HOME / ANDROID_SDK_ROOT environment variable resolution logic is not tested.
  • Multiple FakeAndroidProvider capabilities set up in the fake but never exercised in tests: StoppedEmulators, UninstalledPackageSets, AcceptLicensesCalled, InstallSdkToolsCalls.

3. Test Type Appropriateness — ✅

Current: Unit Tests (xUnit) — appropriate
Recommendation: Same. This is a CLI tool with no visual UI; unit tests with fakes are exactly the right approach. No device tests or UI tests are needed.


4. Convention Compliance — ✅

All 11 test files pass convention checks. Correct xUnit [Fact]/[Theory] attributes, proper [InlineData] usage, clear Arrange/Act/Assert structure, and no obsolete APIs.


5. Flakiness Risk — ⚠️ Medium

AndroidProviderTests.InstallAsync_ReportsProgress uses await Task.Delay(100) to "allow progress callbacks to complete":

await provider.InstallAsync(progress: progress);
// Allow progress callbacks to complete
await Task.Delay(100);
Assert.Contains(progressMessages, m => m.Contains("Step 1/4"));

The FakeAndroidProvider invokes the InstallCallback synchronously — the delay is unnecessary and creates a timing dependency. The test could remove the delay entirely, or use TaskCompletionSource if the callback were genuinely async.


6. Duplicate Coverage — ✅ No duplicates

This is a new tool with no prior test suite. No duplicate coverage issues.


7. Platform Scope — ✅

The fix files include 13 Android-specific files and 22 cross-platform files. Unit tests are platform-agnostic by nature and run on all CI platforms. JdkManagerValidateInstallPathTests appropriately skips Unix-path cases on Windows with a guard (if (OperatingSystem.IsWindows()) return;).


8. Assertion Quality — ⚠️

Most assertions are specific and meaningful. One minor concern:

// DoctorServiceTests.cs line 25
Assert.True(report.Checks.Any(c => c.Category == "dotnet"));

This is less readable and produces a worse failure message than:

Assert.Contains(report.Checks, c => c.Category == "dotnet");

The DeviceManagerTests.GetAllDevicesAsync_MergesRunningEmulatorWithAvd assertions are excellent — very specific about what the merged result should contain.


9. Fix-Test Alignment — ✅

Test files map clearly to the fix files they exercise. The FakeAndroidProvider is well-designed and covers the full IAndroidProvider interface, making it easy to exercise all provider behaviors in isolation.


Recommendations

  1. Add tests for DoctorService.RunCategoryChecksAsync — at minimum: valid "dotnet" category, valid "android" category, "windows" on non-Windows platform (returns Skipped), and invalid category (throws MauiToolException).

  2. Remove Task.Delay(100) in InstallAsync_ReportsProgress — the FakeAndroidProvider calls the InstallCallback synchronously, so the delay is not needed and risks test flakiness in slow CI environments.

  3. Add basic ProcessRunner execution tests — at minimum: verify a well-known command (e.g., echo hello) succeeds with exit code 0 and captures stdout. This validates the core process-running infrastructure.

  4. Replace Assert.True(report.Checks.Any(...)) with Assert.Contains(...) — better failure messages.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@PureWeen
Copy link
Member

🟡 NEW Findings Not Yet on the PR

M1 — sudo call sites in XcodeSelect.cs need specific migration

File: src/Tools/Microsoft.Maui.Client/Providers/Apple/XcodeSelect.cs:64

rolfbjarne's comment covers ProcessRunner.cs generically, but fixing ProcessRunner doesn't auto-fix callers. Two specific sudo call sites need migration:

// SwitchAsync — user-controlled path
await ProcessRunner.RunAsync("sudo", $"xcode-select --switch {ProcessRunner.SanitizeArg(xcodePath)}", ...);

// AcceptLicenseAsync — hardcoded (safe today, wrong pattern)
await ProcessRunner.RunAsync("sudo", "xcodebuild -license accept", ...);

SwitchAsync is the higher-risk one: sudo doesn't invoke a shell, so the entire argument string is one token. Paths with spaces (e.g., /Applications/Xcode 16.app/...) break silently.

Fix: These callers need to pass ["xcode-select", "--switch", xcodePath] as discrete args once ProcessRunner supports ArgumentList.


M2 — JdkManager.ExtractArchiveAsync tar invocation uses string interpolation

File: src/Tools/Microsoft.Maui.Client/Providers/Android/JdkManager.cs

$"-xzf \"{archivePath}\" -C \"{targetPath}\" --strip-components=1"

Same class of bug as rolfbjarne's finding but in a different file. archivePath is temp-dir-based (safe in practice), but the pattern is fragile — if GetTempPath() returns a path with ", it silently corrupts.


M3 — Versioning scheme bypasses arcade

File: src/Tools/Microsoft.Maui.Client/Microsoft.Maui.Client.csproj

Uses a custom SetGitCommitHash MSBuild target to embed git commit SHA as version suffix (0.0.1-alpha.{sha}). This is non-standard for the repo — versioning should be handled by the arcade/darc pipeline. The package cannot be version-bumped via normal darc update flows. Nobody has raised this yet — needs a team decision on shipping strategy.


🟢 Minor (NEW)

N1 — continuousInput parameter in ProcessRunner.RunAsync appears unused

No callers use this parameter. License acceptance goes through Xamarin.Android.Tools.SdkManager instead. Dead code.

N2 — SpectreHelpBuilder hardcodes System.CommandLine help aliases

New in latest commit. Hardcodes ["-h", "--help", "-?"] — fragile if System.CommandLine changes its alias set in a future version.

N3 — No tests for SpectreHelpBuilder

New code with no test coverage.


@rmarinho
Copy link
Member Author

@simonrozsival great questions:

xharness comparison: xharness is focused on running tests on devices/simulators — it orchestrates test execution, collects results, and handles device allocation for CI pipelines. This tool (maui) is focused on development environment setup and management — SDK installation, JDK provisioning, device listing, doctor checks. They are complementary: xharness runs your tests, maui ensures your dev machine has the right SDKs/JDKs/emulators configured. No code is shared today but there may be opportunities in the future (e.g., device enumeration logic).

dnx / tool name: The tool command name is maui (set via <ToolCommandName>maui</ToolCommandName> in the csproj), so dotnet maui doctor would work when installed as a dotnet tool. The NuGet package ID is Microsoft.Maui.Client but that is just the package name — the actual CLI binary is maui. For dnx specifically, it would depend on how dnx resolves dotnet tools, but the standard dotnet tool install Microsoft.Maui.Clientmaui doctor flow works.

@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Cannot evaluate: this PR's branch does not include the evaluate-pr-tests skill (.github/skills/evaluate-pr-tests/SKILL.md is missing).

Fix: rebase your fork on the latest main branch, or use the workflow_dispatch trigger (Actions tab → "Evaluate PR Tests" → "Run workflow" → enter PR number) which handles this automatically.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Cannot evaluate: this PR's branch does not include the evaluate-pr-tests skill (.github/skills/evaluate-pr-tests/SKILL.md is missing).

Fix: rebase your fork on the latest main branch, or use the workflow_dispatch trigger (Actions tab → "Evaluate PR Tests" → "Run workflow" → enter PR number) which handles this automatically.

🧪 Test evaluation by Evaluate PR Tests

rmarinho and others added 16 commits March 26, 2026 13:53
- Split FixInfo.Command into fileName+arguments in TryFixAsync via
  ParseCommand(), fixing broken multi-word auto-fix commands
  (e.g., 'dotnet workload install maui' was passed as filename)
- Add ValidateInstallPath() to JdkManager to prevent recursive
  deletion of system/home/shallow directories via --path option
- Add 18 tests: ParseCommand (7) + ValidateInstallPath (4) + SanitizeArg
  existing coverage maintained

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ols ref

- Bump Xamarin.Android.Tools.AndroidSdk to 1.0.143-preview.8 from CI feed
- Remove Xamarin.Apple.Tools.MaciOS PackageReference (not used by CLI code yet)
- Build succeeds: 0 warnings, 0 errors

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove Moq dependency and replace all Mock<T> usage with simple
FakeAndroidProvider and FakeAppleProvider stubs. This avoids the
Moq dependency and makes test setup more explicit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Honor --ci flag by skipping interactive prompts in all commands
  (install, sdk install, emulator create/start/stop/delete)
- Fix iOS log filter double-quoting when filter contains spaces
  by escaping quotes directly instead of using SanitizeArg
- Fix Services property race condition by always acquiring lock
  (remove broken double-checked locking pattern)
- Fix process leak in StartAvdAsync by killing emulator process
  on cancellation before re-throwing OperationCanceledException

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove TakeScreenshotAsync from all layers:
- Adb: delete TakeScreenshotAsync method
- AndroidProvider: delete TakeScreenshotAsync method
- IAndroidProvider: delete TakeScreenshotAsync from interface
- IDeviceManager: delete TakeScreenshotAsync from interface
- DeviceManager: delete TakeScreenshotAsync and TakeAppleScreenshotAsync
- DeviceCommand: delete CreateScreenshotCommand and its registration
- FakeAndroidProvider: delete TakeScreenshotAsync and tracking field

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move test project from src/Tools/Microsoft.Maui.Client.Tests to
  src/Tools/tests/Microsoft.Maui.Client.UnitTests (repo convention)
- Rename csproj to Microsoft.Maui.Client.UnitTests.csproj
- Update ProjectReference path to ../../Microsoft.Maui.Client/
- Update InternalsVisibleTo from Tests to UnitTests
- Update namespace in all test files from
  Microsoft.Maui.Client.Tests to Microsoft.Maui.Client.UnitTests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove 95 explicit 'private' modifiers per editorconfig rule
  (dotnet_style_require_accessibility_modifiers = never)
- Add C# Coding Conventions section to copilot-instructions.md
  documenting: no private, var usage, file-scoped namespaces,
  _camelCase fields, braces, no this. qualifier, predefined types

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The tool and test projects were not included in the main solution
or the pack solution filters, so the tool was never packaged
during CI builds.

- Add Microsoft.Maui.Client to Microsoft.Maui.sln (Tools folder)
- Add Microsoft.Maui.Client.UnitTests to Microsoft.Maui.sln
- Add to eng/Microsoft.Maui.Packages.slnf (Windows pack)
- Add to eng/Microsoft.Maui.Packages-mac.slnf (macOS pack)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the test project to eng/helix.proj XUnitProject list so tests
run on Helix infrastructure during CI. Also add explicit xunit
package reference matching the pattern of other test projects.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove LogsCommand.cs, 'device logs' subcommand, and logs screenshot.
The logs feature will be added in a future phase.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI restores for net10.0 (from _MauiDotNetTfm) but projects targeted net9.0,
causing NETSDK1005 asset mismatch. Use $(_MauiDotNetTfm) to match repo SDK.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the default System.CommandLine help renderer with a custom
SpectreHelpBuilder that uses Spectre.Console markup for colored output:
- Yellow section headers (Description, Usage, Options, Commands)
- Green option aliases and command names
- Dim annotations for platform-specific commands (macOS)
- Alphabetically sorted subcommands

The custom help is triggered before the parser runs by checking args
for help aliases, then resolving the target command and rendering
via SpectreHelpBuilder. This avoids fighting System.CommandLine's
internal help middleware.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove Apple provider (IAppleProvider, AppleProvider, Simctl, XcodeSelect),
Apple commands, Apple error codes (E22xx), Apple health checks in DoctorService,
Apple device listing in DeviceManager, FakeAppleProvider test fake, and macOS
annotation logic in SpectreHelpBuilder.

Apple/macOS support will be added in a separate follow-up PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
….Details.xml

- ProcessRunner: Use ProcessStartInfo.ArgumentList instead of manual
  string quoting (addresses @rolfbjarne feedback). RunSync/RunAsync now
  take string[] args. SanitizeArg no longer quotes, only validates.
- Move package versions to eng/Versions.props (addresses @PureWeen
  feedback): SystemCommandLine, SpectreConsole, plus test project's
  Spectre.Console.Testing.
- System.CommandLine stays on beta (2.0.0-beta4.22272.1) — stable 2.0.x
  has breaking API changes requiring substantial migration.
- Align eng/Version.Details.xml android-tools entry to 1.0.143-preview.8
  to match eng/Versions.props (addresses Copilot review feedback).
- Update DoctorService.ParseCommand to return string[] args array.
- All 118 tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Full API migration from System.CommandLine 2.0.0-beta4.22272.1 to the
stable 2.0.5 release. Key changes:

- CommandLineBuilder pipeline → rootCommand.Parse(args).InvokeAsync()
- SetHandler(InvocationContext) → SetAction(ParseResult, CancellationToken)
- context.ExitCode = N → return N from handler (Task<int> overload)
- context.GetCancellationToken() → CancellationToken parameter
- GetValueForOption/Argument → GetValue
- AddGlobalOption → Option.Recursive = true + Add
- Option/Argument constructors → object initializer syntax
- Remove System.CommandLine.Builder and Invocation namespaces

All 118 tests pass. No behavioral changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rmarinho rmarinho force-pushed the maui-client-tool-android branch from 1bedb66 to cb25e55 Compare March 26, 2026 13:53
@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The 118 unit tests are well-structured and cover the most critical paths (security, command structure, DI), but several significant behaviors — including DoctorService.RunCategoryChecksAsync, AndroidEnvironment mappings, and error paths — are not covered.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34498 — Add Microsoft.Maui.Client tool for cross-platform device and SDK management
Test files evaluated: 11 (10 test classes + 1 fake)
Fix files: 35 (entirely new CLI tool implementation)


Overall Verdict

⚠️ Tests need improvement

Tests are good quality (hand-written fakes, xUnit conventions, security-critical paths well covered), but notable gaps exist in DoctorService.RunCategoryChecksAsync, AndroidEnvironment mapping functions, DeviceManager.GetRunningDeviceOrThrowAsync, and one Task.Delay anti-pattern in an async test.


1. Fix Coverage — ⚠️

Since this is a new feature (not a bug fix), the question is whether the tests cover the most important behaviors of the new tool.

Well covered:

  • ProcessRunner.SanitizeArg — security-critical injection prevention
  • JdkManager.ValidateInstallPath — path traversal prevention
  • AndroidCommands — command structure / CLI argument parsing
  • DeviceManager.GetAllDevicesAsync — device listing and running/stopped AVD merging
  • DoctorService.RunAllChecksAsync — health check aggregation
  • ServiceConfiguration — DI container wiring
  • OutputFormatter — JSON and Spectre output formatting
  • MauiToolException — factory methods and properties

Uncovered important behaviors:

  • DoctorService.RunCategoryChecksAsync — the category-based filtering path (dotnet / android / windows / unknown) is never exercised
  • DeviceManager.GetRunningDeviceOrThrowAsync — the "no running device" error path is not tested
  • DeviceManager.ParseSystemImage — complex string parsing logic for system image paths not directly tested
  • AndroidEnvironment mapping functions (MapAbiToArchitecture, MapTagIdToSubModel, MapApiLevelToVersion, GetRuntimeIdentifiers) — pure utility functions with no tests at all
  • AndroidCommandsTests tests structure only (do subcommands exist?) rather than actual execution behavior — consider adding tests that verify handler logic via FakeAndroidProvider

2. Edge Cases & Gaps — ⚠️

Covered:

  • SanitizeArg null input, all forbidden characters, multi-char injection sequences (e.g., ; rm -rf /)
  • ValidateInstallPath home directory, system directories, shallow paths, valid paths
  • DeviceManager AVD merge via Details["avd"] AND via EmulatorId fallback (two separate test cases)
  • GetMostRecentSystemImageAsync with no matching system images (null case)
  • Platforms.IsValid/Normalize with null, empty, uppercase, aliases

Missing:

  • DoctorService.RunCategoryChecksAsync("unknown-category") — should throw MauiToolException; currently untested
  • DeviceManager.GetAllDevicesAsync with null _androidProvider — what happens when no provider is registered?
  • DeviceManager.ParseSystemImage edge cases: null, empty string, slash-separated vs semicolon-separated formats, missing ABI segment
  • AndroidEnvironment.MapApiLevelToVersion boundary values — API level 33, 34, 35, out-of-range
  • AndroidCommandsTests — no test verifies actual command handler execution; all tests check structural metadata (option names, argument counts), not behavior

3. Test Type Appropriateness — ✅

Current: Unit tests exclusively (xUnit + Spectre.Console.Testing)
Recommendation: Correct choice — this is a CLI tool with no UI, no platform rendering, no Appium interaction needed. Unit tests with hand-written fakes are exactly right. Running on Helix (Windows + macOS via eng/helix.proj) provides real cross-platform coverage.

4. Convention Compliance — ✅

All test classes use [Fact] or [Theory], no convention issues detected by the automated script. No anti-patterns around test setup/teardown. FakeAndroidProvider is a well-designed hand-written fake (no Moq dependency, full control over behavior).

5. Flakiness Risk — ⚠️ Medium

One concern:

// AndroidProviderTests.InstallAsync_ReportsProgress
await provider.InstallAsync(progress: progress);

// Allow progress callbacks to complete
await Task.Delay(100);  // ⚠️ Arbitrary delay — flaky under load

Task.Delay(100) to wait for IProgress(string) callbacks is fragile. The Progress(T) class posts callbacks to the synchronization context; in tests without a UI dispatcher, the callbacks may or may not have completed after 100ms. Prefer using a SynchronousProgress(T) helper or collecting messages synchronously within the callback to avoid relying on timing.

6. Duplicate Coverage — ✅ No duplicates

This is entirely new functionality. No similar tests exist elsewhere in the repository.

7. Platform Scope — ✅

Tests are included in eng/helix.proj targeting Windows.10.Amd64.Open and osx.15.arm64.maui.open, which is appropriate for a cross-platform CLI tool. No iOS/macCatalyst tests needed (Apple support is intentionally deferred per the PR description).

8. Assertion Quality — ✅

Assertions are generally specific and meaningful:

  • Assert.Equal("system-images;android-35;google_apis;arm64-v8a", result) — exact match
  • Assert.Equal(("TestEmulator", true, true), provider.StartedAvds[0]) — tuple comparison
  • Assert.Contains(parseResult.Errors, ...) inverted as Assert.Empty(parseResult.Errors) — clean

Minor concern: AndroidCommandsTests assertions check only that metadata exists (Assert.Contains(subcommands, c => c.Name == "install")), not that the commands work correctly. These tests would not catch a regression where a command exists but its handler is broken.

9. Fix-Test Alignment — ✅

For a new feature, the test files map well to the implementation modules:

  • AndroidCommandsTestsCommands/AndroidCommands*.cs
  • AndroidProviderTestsProviders/Android/AndroidProvider.cs
  • DeviceManagerTestsServices/DeviceManager.cs
  • DoctorServiceTestsServices/DoctorService.cs
  • ProcessRunnerTestsUtils/ProcessRunner.cs + Services/DoctorService.ParseCommand + Providers/Android/JdkManager.ValidateInstallPath

Recommendations

  1. Add DoctorService.RunCategoryChecksAsync tests — this is a public-facing code path with its own logic (category dispatch, error on unknown category) and is completely uncovered. At minimum, test the "unknown" category throws MauiToolException, and that "android" returns only android checks.

  2. Add AndroidEnvironment mapping testsMapAbiToArchitecture, MapTagIdToSubModel, MapApiLevelToVersion are pure functions with lookup tables; they are trivial to test and directly affect the maui device list output. Example: MapApiLevelToVersion("35") should return "15.0".

  3. Fix the Task.Delay(100) in InstallAsync_ReportsProgress — replace with a synchronous progress collector or await Task.Yield() after invoking the method, to avoid test flakiness under CI load.

  4. Add DeviceManager.GetRunningDeviceOrThrowAsync error path test — verify it throws MauiToolException with ErrorCodes.DeviceNotFound when no booted device is present. This is a one-liner with FakeAndroidProvider.

  5. (Optional) Add AndroidCommandsTests execution smoke tests — consider testing that at least one command handler returns a non-crash result when invoked via the DI test provider. This would catch broken wiring between command definitions and handler implementations.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 2 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

rmarinho and others added 2 commits March 26, 2026 16:20
The previous 'dotnet sln add' introduced x64 and x86 platform configurations
to the entire solution, which breaks Android/iOS multi-target builds (CS0246:
'Android' namespace not found). Restored clean sln from main and manually
added tool projects with Any CPU configs only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sync

The test was asserting status != Unhealthy but RunAllChecksAsync always
runs dotnet SDK/workload checks that fail on Helix machines (no MAUI
workload installed). Changed to use RunCategoryChecksAsync('android')
which only exercises the mocked android provider checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ✅ Tests are adequate

The PR introduces a new Microsoft.Maui.Client CLI tool (78 test methods across 11 files) with well-structured xUnit unit tests. Test coverage is comprehensive for the business logic layer. Minor gaps exist around real provider implementations, one flakiness risk, and a Windows-specific untested code path.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34498 — Microsoft.Maui.Client CLI tool (Android support)
Test files evaluated: 11 (including 1 fake/helper)
Fix files: 35


Overall Verdict

✅ Tests are adequate

Tests are well-structured, use the correct type (unit tests), follow xUnit conventions, and cover the key business logic via a thoughtfully designed FakeAndroidProvider. One minor flakiness risk and a few coverage gaps are worth noting.


1. Fix Coverage — ✅

Tests cover all major subsystems introduced by the PR:

  • CLI structureAndroidCommandsTests verifies subcommands, options, and argument parsing for install, emulator, jdk, sdk
  • Device managementDeviceManagerTests covers list, filter, find, shutdown/running merge logic
  • Doctor serviceDoctorServiceTests covers dotnet + android health checks, status aggregation
  • Error modelMauiToolExceptionTests covers all three factory methods (AutoFixable, UserActionRequired, Terminal) and all constructor variants
  • Output formattingOutputFormatterTests covers JSON and Spectre console output for errors, doctor reports, device lists
  • SecurityProcessRunnerTests verifies SanitizeArg rejects shell metacharacters (;, &, |, `, $, newlines)
  • Path safetyJdkManagerValidateInstallPathTests verifies that home directory, system directories, and shallow paths are rejected

Gap: The real AndroidProvider implementation (talking to adb, avdmanager, sdkmanager) is not unit-tested — this is expected since those require real toolchain and are better suited for integration tests.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Null input in SanitizeArg throws ArgumentNullException
  • Empty string passes SanitizeArg
  • Repeated --packages flags parsed as multiple items
  • AVD merging with both Details["avd"] and EmulatorId fallback (two separate tests)
  • GetDeviceByIdAsync returns null when not found
  • ValidateInstallPath rejects home dir, system dirs, and shallow paths on both Windows and Unix

Missing:

  • Cancellation token propagation — no test verifies that passing a cancelled CancellationToken to async methods propagates correctly
  • DoctorService.RunCategoryChecksAsync("windows") on a non-Windows host returns a Skipped check — this code path is untested
  • CommandExtensions / DeviceCommand — the actual command handler execution (not just structure) is untested
  • Progress reporting under load — only happy-path progress is tested; no test for partial/interrupted progress reporting

3. Test Type Appropriateness — ✅

Current: Unit Tests (xUnit)
Recommendation: Correct choice. The Microsoft.Maui.Client tool is a CLI with injectable service abstractions — unit tests with FakeAndroidProvider are exactly right. No visual UI or platform-native API interaction is needed. UI tests or device tests would be unnecessary overhead here.


4. Convention Compliance — ✅

  • ✅ All test methods use [Fact] or [Theory] (xUnit) correctly
  • ✅ No detected convention issues from the automated script (0 issues)
  • FakeAndroidProvider is a clean hand-written fake with call-tracking and configurable return values
  • ✅ Namespace follows project convention (Microsoft.Maui.Client.UnitTests)
  • ✅ Tests are organized into logical classes (AndroidCommandsTests, DeviceManagerTests, etc.)

5. Flakiness Risk — ⚠️ Medium (one case)

AndroidProviderTests.InstallAsync_ReportsProgress uses:

await provider.InstallAsync(progress: progress);
await Task.Delay(100);  // Allow progress callbacks to complete

Progress(T) in .NET posts callbacks via the captured SynchronizationContext. In unit test environments (no SynchronizationContext), callbacks post to the thread pool, making the 100ms delay a race condition. Under CI load this could intermittently fail.

Recommended fix:

// Use a CountdownEvent or TaskCompletionSource for deterministic sync
var tcs = new TaskCompletionSource();
var progress = new Progress(string)(msg => {
    progressMessages.Add(msg);
    if (progressMessages.Count >= 4) tcs.TrySetResult();
});
await provider.InstallAsync(progress: progress);
await tcs.Task.WaitAsync(TimeSpan.FromSeconds(5));

All other tests have no flakiness risk (no Thread.Sleep, no screenshots, no external dependencies).


6. Duplicate Coverage — ✅ No duplicates

This is entirely new code and new tests. No existing similar test files were found in the repository.


7. Platform Scope — ⚠️

The tool is cross-platform (runs on Windows, macOS, Linux) but:

  • DoctorService.RunAllChecksAsync adds a Windows SDK check (PlatformDetector.IsWindows) — this path has no test coverage
  • JdkManagerValidateInstallPathTests correctly guards platform-specific system directory tests with if (OperatingSystem.IsWindows()) return
  • Unit tests run cross-platform by nature and the Helix pipeline configuration in eng/helix.proj includes the test project — good

Acceptable for a new tool at v0.0.1-alpha, but the Windows SDK check path (CheckWindowsSdkAsync) should be tested eventually.


8. Assertion Quality — ✅

Most assertions are specific and meaningful:

  • Assert.Equal("system-images;android-35;google_apis;arm64-v8a", result) — exact value
  • Assert.Equal(("TestEmulator", true, true), provider.StartedAvds[0]) — tuple comparison
  • Assert.Throws(ArgumentException)(() => ProcessRunner.SanitizeArg("test; rm -rf /")) — verifies security behavior

One slightly vague assertion:

  • ⚠️ Assert.True(report.Summary.Total >= 3) in RunAllChecksAsync_CalculatesCorrectSummary — uses >= rather than an exact count, since dotnet environment checks are environment-dependent. This is a reasonable trade-off for test stability.

9. Fix-Test Alignment — ✅

All test files map directly to their corresponding production code:

Test File Fix File(s) Covered
AndroidCommandsTests AndroidCommands*.cs
AndroidProviderTests IAndroidProvider, FakeAndroidProvider infrastructure
DeviceManagerTests DeviceManager.cs
DoctorServiceTests DoctorService.cs
ErrorCodesTests ErrorCodes.cs
MauiToolExceptionTests MauiToolException.cs
OutputFormatterTests JsonOutputFormatter.cs, SpectreOutputFormatter.cs
PlatformsTests Platforms.cs
ProcessRunnerTests ProcessRunner.cs, DoctorService.ParseCommand, JdkManager.ValidateInstallPath
ServiceConfigurationTests ServiceConfiguration.cs, Program.cs

Recommendations

  1. Fix the Task.Delay(100) flakiness in InstallAsync_ReportsProgress — use a synchronization primitive instead of an arbitrary delay to avoid intermittent CI failures.

  2. Add a test for RunCategoryChecksAsync("windows") on a non-Windows host to cover the Skipped status branch in DoctorService.

  3. Consider adding integration tests for the real AndroidProvider implementation against actual adb/avdmanager binaries, even if only run on Android-capable CI agents. The command injection protection in ProcessRunner.SanitizeArg is only validated in unit tests today.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

…ails.xml

Apple support was removed from this PR and deferred to a follow-up.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The unit test suite is well-structured and covers the main components of the new Microsoft.Maui.Client CLI tool. Coverage is good for data models, command structure, and output formatters, but command handler execution logic and actual process execution are largely untested.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34498 — Microsoft.Maui.Client CLI tool (Android SDK/JDK/emulator management)
Test files evaluated: 11 unit test classes
Fix files: 35 source files (new CLI tool)


Overall Verdict

⚠️ Tests need improvement

The tests cover core data models and service contracts well using a clean FakeAndroidProvider, but the actual command handler execution paths (DoctorCommand, DeviceCommand, AndroidCommands.*) are largely untested, and ProcessRunner's actual process execution has no coverage beyond argument sanitization.


1. Fix Coverage — ⚠️

Tests cover the main service/model layer well:

  • DeviceManagerTests — device listing, AVD merging (two merge paths: via Details["avd"] and EmulatorId)
  • DoctorServiceTests — health check aggregation and summary calculation
  • AndroidProviderTests — fake-based AVD lifecycle and package management
  • OutputFormatterTests — JSON and Spectre console formatters
  • ProcessRunnerTests — argument sanitization (security-critical code, well tested)

Gap: The command handler delegates in DoctorCommand.cs, DeviceCommand.cs, and the AndroidCommands.* files contain the actual business logic that wires services to CLI output. These are untested — if a command handler incorrectly passes arguments to a service or formats output wrongly, no test would catch it.

Gap: ProcessRunner.RunSync / RunAsync (the actual process spawning) has zero test coverage. Only SanitizeArg is tested.

2. Edge Cases & Gaps — ⚠️

Covered:

  • Shell injection via all documented forbidden characters (;, &, |, `, $, \n, \r, \0)
  • Two AVD-merge code paths in DeviceManager (via Details["avd"] and EmulatorId fallback)
  • GetMostRecentSystemImageAsync returns highest API level and null-when-empty
  • Platform alias normalization with many inputs (apple, iphone, mac, win32, etc.)
  • MauiToolException factory methods (AutoFixable, UserActionRequired, Terminal)

Missing:

  • Cancellation token propagationDeviceManager/DoctorService accept CancellationToken but no test verifies cancellation is forwarded to the provider
  • Windows-gated code pathDoctorService.RunAllChecksAsync calls CheckWindowsSdkAsync on Windows only; this path is never exercised
  • DoctorService assertion precisionAssert.True(report.Summary.Total >= 3) is loose; the test cannot distinguish between "3 checks" and "100 checks"
  • Error handling — no test for when IAndroidProvider throws an exception mid-run

3. Test Type Appropriateness — ✅

Current: Unit tests (xUnit)
Recommendation: Same — unit tests are exactly the right choice for a CLI tool. No visual rendering, no native UI, no platform-specific SDK calls. Using UI tests or device tests would be unnecessarily heavy here.

4. Convention Compliance — ✅

All 11 test classes use [Fact] / [Theory] (xUnit) correctly. FakeAndroidProvider is a well-designed hand-written fake with both configurable return values and call-tracking lists. No convention issues found by automated checks.

Minor note: FakeAndroidProvider implements IAndroidProvider — if that interface grows, the fake will need to be kept in sync.

5. Flakiness Risk — ⚠️ Medium

One risk identified:

// AndroidProviderTests.InstallAsync_ReportsProgress
await provider.InstallAsync(progress: progress);
await Task.Delay(100);  // <-- relies on timing for IProgress(T) callbacks

IProgress(T) callbacks are dispatched asynchronously (via SynchronizationContext). The Task.Delay(100) is a timing-based workaround. In a CI environment under load this could be flaky. A more robust approach: use TaskCompletionSource or collect progress in a synchronous callback, or use Progress(T) with a ManualResetEventSlim.

6. Duplicate Coverage — ✅ No duplicates

This is a new tool/project with no prior tests. No duplicates.

7. Platform Scope — ✅

Unit tests run on all platforms (no platform guards, no Appium). The fix code has some platform-gated paths (PlatformDetector.IsWindows, PlatformDetector.IsArm64), but since these are unit tests using fakes, all platforms are covered in CI equally.

The Windows-gated CheckWindowsSdkAsync path in DoctorService is not reachable in unit tests on non-Windows platforms, but that is acceptable for a CLI tool targeting developer machines.

8. Assertion Quality — ⚠️

Most assertions are specific and meaningful. Two assertions in DoctorServiceTests are too loose:

Assert.True(report.Summary.Total >= 3);   // vague — any number ≥ 3 passes
Assert.True(report.Summary.Warning >= 1); // vague
Assert.True(report.Summary.Error >= 1);   // vague

Since the fake provider returns exactly 3 checks (1 Ok, 1 Warning, 1 Error), these could be:

// Tighter: check the android-contributed checks specifically
Assert.Equal(1, report.Checks.Count(c => c.Category == "android" && c.Status == CheckStatus.Warning));
Assert.Equal(1, report.Checks.Count(c => c.Category == "android" && c.Status == CheckStatus.Error));

The ProcessRunnerTests security assertions are excellent — each injection variant is tested individually with [Theory] + [InlineData].

9. Fix-Test Alignment — ✅

Tests align with the main components. The FakeAndroidProvider correctly mirrors IAndroidProvider, so any service that depends on it is testable in isolation. The command-structure tests in AndroidCommandsTests verify the CLI shape (subcommands, required arguments, options) rather than runtime behavior — which is appropriate for System.CommandLine registration tests.


Recommendations

  1. Add execution-level tests for at least one command handler — e.g., verify that DoctorCommand calls IDoctorService.RunAllChecksAsync and passes the result to IOutputFormatter.WriteResult. This can be done with the existing fake/test-provider pattern.

  2. Tighten DoctorServiceTests assertions — replace Assert.True(summary.Total >= 3) with exact counts or at minimum Assert.Equal(3, ...) to prevent false-green results.

  3. Fix the Task.Delay(100) timing issue in InstallAsync_ReportsProgress — switch to a synchronous progress callback or use an awaitable synchronization primitive.

  4. Add a basic ProcessRunner smoke test — at minimum test that a known-safe command (e.g., echo or dotnet --version) produces a Success result with non-empty StandardOutput. This verifies the process-start plumbing works end-to-end.

  5. Add a cancellation test — verify that passing a cancelled CancellationToken to DeviceManager.GetAllDevicesAsync or DoctorService.RunAllChecksAsync throws OperationCanceledException or propagates the token to the provider.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 2 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

rmarinho and others added 2 commits March 26, 2026 18:39
1. DeviceManager.GetRunningDeviceOrThrowAsync now uses IsRunning instead
   of checking State == Booted. Android maps online devices to Connected
   not Booted, so they were never found.

2. DoctorCommand --fix now re-runs checks after applying fixes and bases
   exit code on post-fix results. Previously always exited with failure
   even after successful fixes.

3. Remove 'apple' from --platform option help text since the service
   throws for that value. Apple support deferred to follow-up PR.

4. Honor --accept-licenses flag in android install. Both Spectre and
   non-Spectre paths now skip license acceptance unless the flag is set.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ✅ Tests are adequate

This PR introduces a new Microsoft.Maui.Client CLI tool for Android developer tooling. The 11 unit test files (74 test methods) provide solid coverage of the tool's core services, commands, models, and security-sensitive utilities.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34498 — Microsoft.Maui.Client CLI tool (Android developer tooling)
Test files evaluated: 11 (unit tests only)
Fix files: 35 (new CLI tool: commands, services, providers, models, output formatters)


Overall Verdict

Tests are adequate

The PR uses the correct test type (xUnit unit tests) with a well-designed FakeAndroidProvider pattern, and covers the primary behaviors of the new CLI tool. A few gaps exist around error paths and the AndroidProvider concrete implementation.


1. Fix Coverage — ✅

The tests cover the major components of the new CLI tool:

  • AndroidCommandsTests: Validates command parsing, option structure, and subcommand existence (14 facts)
  • DoctorServiceTests: Exercises health check aggregation and category isolation via FakeAndroidProvider
  • ProcessRunnerTests: Directly tests SanitizeArg and ParseCommand security-sensitive logic with thorough injection scenarios
  • DeviceManagerTests: Covers running/shutdown device merging logic, including the two AVD merge paths
  • OutputFormatterTests: Covers JSON and Spectre output formatting for errors, reports, and device lists
  • ServiceConfigurationTests: Validates DI registration and test override mechanism

Tests would catch regressions in all of these code paths.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Command injection (;, &, |, `, $, \n, \r, \0) in SanitizeArg
  • Null input to SanitizeArg
  • Empty string to SanitizeArg
  • GetMostRecentSystemImageAsync with no system images (returns null)
  • Device merge when AVD is matched via avd key in Details dict vs. EmulatorId property
  • Null/invalid/empty inputs to Platforms.IsValid and Platforms.Normalize
  • JDK install path safety (home dir, system dirs, shallow paths)
  • ParseCommand with quoted executables and whitespace

Missing / Gaps:

  • AndroidProvider concrete class not tested: AndroidProvider.cs (417 lines) wraps SdkManager, AvdManager, Adb, and JdkManager with real process execution. None of the tests exercise the real provider — only the fake. Gaps include: what happens when SdkManager returns an unexpected format, error propagation when adb devices fails, etc. This is acceptable for a new PR but worth noting for follow-up.
  • DoctorService.RunAllChecksAsync is environment-dependent: The test for IncludesDotNetChecks actually runs dotnet --list-sdks on the host (per the commit message "Fix DoctorService test to use category check"). One remaining test (RunAllChecksAsync_IncludesDotNetChecks) still calls RunAllChecksAsync which invokes real dotnet commands — this could be flaky on agents without .NET installed as expected.
  • Cancellation token propagation: No tests verify that cancellation tokens are threaded through to child process calls.
  • SpectreOutputFormatter verbose mode: OutputFormatterTests creates the formatter with verbose = false only; verbose output paths are untested.
  • DeviceManager with empty device list: Tested indirectly via the AVD-only test, but the case of zero devices AND zero AVDs isn't explicitly tested.

3. Test Type Appropriateness — ✅

Current: Unit Tests (xUnit)
Recommendation: Correct choice. This is a CLI tool with services that are fully mockable via interfaces (IAndroidProvider, IDoctorService, etc.). Unit tests with a hand-written fake are the right approach — UI tests or device tests would be inappropriate here.

The FakeAndroidProvider design is well-constructed: it's configurable via properties, tracks call arguments, and supports optional delegate overrides for progress simulation. This is a solid pattern.


4. Convention Compliance — ✅

All test files pass automated convention checks:

  • xUnit [Fact] and [Theory] attributes used correctly
  • No obsolete APIs
  • No Thread.Sleep or Task.Delay (except one 100ms delay in InstallAsync_ReportsProgress — this is testing async IProgress(T) callbacks and is acceptable)
  • Follows _camelCase private field naming
  • File-scoped namespaces

5. Flakiness Risk — ⚠️ Medium

Potential risk:

  • DoctorServiceTests.RunAllChecksAsync_IncludesDotNetChecks calls RunAllChecksAsync() which runs real system processes (dotnet --version, dotnet workload list). On CI agents without the expected .NET SDK, this could produce unexpected results. The commit message "Fix DoctorService test to use category check instead of RunAllChecksAsync" suggests this was already known — but one test still uses RunAllChecksAsync.
  • AndroidProviderTests.InstallAsync_ReportsProgress uses await Task.Delay(100) to wait for IProgress(T) callbacks — fragile on slow CI machines. Consider using TaskCompletionSource or a ManualResetEvent instead.
  • All other tests are deterministic and should be stable.

6. Duplicate Coverage — ✅ No duplicates

This is a new Microsoft.Maui.Client project with no prior test coverage. All tests are additive.


7. Platform Scope — ⚠️

The script detected "cross-platform fix files" (22 files) alongside "Android-specific" files. The CLI tool itself is cross-platform (runs on macOS, Windows, Linux), but its tests don't contain #if guards — they run on all platforms.

One concern: JdkManagerValidateInstallPathTests.ValidateInstallPath_SystemDirectories_Throws skips on Windows (if (OperatingSystem.IsWindows()) return;). The Helix configuration should confirm these tests run on both macOS and Windows agents. No test explicitly covers Windows system directory validation (e.g., C:\Windows\System32).


8. Assertion Quality — ✅

Assertions are specific and meaningful:

  • SanitizeArg tests check exact exception messages
  • ParseCommand tests check exact fileName and args arrays
  • DeviceManager merge tests assert on specific property values (Id, EmulatorId, IsRunning)
  • OutputFormatter tests check for specific output strings (error codes like "E2001", symbol characters like "✓", "⚠")

One minor concern: DoctorServiceTests.RunAllChecksAsync_CalculatesCorrectSummary uses >= comparisons (Assert.True(report.Summary.Total >= 3)) because it can't predict the exact number of dotnet checks from the environment. This is acceptable but worth noting.


9. Fix-Test Alignment — ✅

Tests align well with the fix files:

Fix Area Test Coverage
AndroidCommands*.cs AndroidCommandsTests — all subcommands and options
DoctorService.cs DoctorServiceTests, ProcessRunnerTests.DoctorServiceParseCommandTests
ProcessRunner.cs (SanitizeArg) ProcessRunnerTests — thorough injection scenarios
DeviceManager.cs DeviceManagerTests — merge logic fully covered
JsonOutputFormatter.cs, SpectreOutputFormatter.cs OutputFormatterTests
ServiceConfiguration.cs ServiceConfigurationTests
Platforms.cs PlatformsTests
MauiToolException.cs, ErrorCodes.cs MauiToolExceptionTests, ErrorCodesTests
AndroidProvider.cs Only via FakeAndroidProvider (concrete class untested)

Recommendations

  1. Consider replacing Task.Delay(100) in InstallAsync_ReportsProgress with a synchronous IProgress(T) implementation or a TaskCompletionSource approach to avoid timing-dependent assertions.
  2. Add a Windows-specific system directory test in JdkManagerValidateInstallPathTests (e.g., C:\Windows) to match the Unix coverage.
  3. Watch RunAllChecksAsync_IncludesDotNetChecks — it runs real dotnet CLI commands; if CI agents have unusual .NET SDK configurations this could surface as a flaky test.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 2 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@rmarinho
Copy link
Member Author

/azp run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants