Fix flaky CLI tests: serialize Program.Services mutators with [Collection]

[rmarinho]

Why

Three CLI test classes — AppleCommandsTests, AndroidCommandsTests, ServiceConfigurationTests — mutate the static Program.Services to inject fake providers, but were not part of any xUnit [Collection]. They therefore parallelized against each other and against the existing [Collection("CLI")] DevFlow tests, which also mutate Program.Services.

When two such tests ran concurrently, one would overwrite Program.Services mid-test, so the action under test resolved a different (or the production) provider. The try { Program.Services = testProvider; ... } finally { Program.ResetServices(); } pattern is not safe for parallel execution because Program.Services is process-wide.

Concrete symptom

On macOS CI for PR #202:

Microsoft.Maui.Cli.UnitTests.AppleCommandsTests.InstallCommand_Json_ReturnsOneForFailedStatus
  Assert.Equal() Failure: Values differ
  Expected: 1
  Actual:   0

The install handler in AppleCommands.cs correctly returns 1 for Status = "failed":

return result.Status is "ok" or "skipped" ? 0 : 1;

…but Program.AppleProvider had been replaced by a parallel test's fake (whose default InstallResult.Status = "ok"), so the handler saw "ok" and returned 0.

This also explains why the failure was intermittent: PR #200's CI run happened to win the race; PR #202's lost it.

Fix

Tag all three classes with [Collection("CLI")] so they share the DisableParallelization = true collection already defined in Fixtures/CliCollection.cs. No production code changes.

+[Collection("CLI")]
 public class AppleCommandsTests
+[Collection("CLI")]
 public class AndroidCommandsTests
+[Collection("CLI")]
 public class ServiceConfigurationTests

Verification

• Local Windows: dotnet test over the affected classes — 35/35 pass (AppleCommandsTests Install* skip on non-macOS, expected).
• Macros tests will exercise the install path on CI here.

Follow-up

Unblocks CI on PR #200 and PR #202.

[Copilot]

Pull request overview

This PR addresses flaky CLI unit tests by ensuring test classes that mutate the process-wide static Program.Services run serially. It does so by adding the existing xUnit [Collection("CLI")] attribute (backed by DisableParallelization = true) to the affected test classes; no production code changes are included.

Changes:

• Add [Collection("CLI")] to AppleCommandsTests to prevent concurrent Program.Services overrides.
• Add [Collection("CLI")] to AndroidCommandsTests to prevent concurrent Program.Services overrides.
• Add [Collection("CLI")] to ServiceConfigurationTests to prevent concurrent Program.Services overrides.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/Cli/Microsoft.Maui.Cli.UnitTests/AppleCommandsTests.cs	Serializes execution with other CLI tests to avoid cross-test DI contamination via Program.Services.
src/Cli/Microsoft.Maui.Cli.UnitTests/AndroidCommandsTests.cs	Serializes execution with other CLI tests to avoid cross-test DI contamination via Program.Services.
src/Cli/Microsoft.Maui.Cli.UnitTests/ServiceConfigurationTests.cs	Serializes execution with other CLI tests to avoid cross-test DI contamination via Program.Services.

[github-actions]

Expert Code Review — PR #222

Methodology: 3 independent reviewers with adversarial consensus

Result: ✅ The fix is correct. All 3 reviewers independently confirmed the [Collection("CLI")] additions properly serialize tests that mutate the static Program.Services, resolving the described race condition without introducing regressions.

Findings: 1 minor (outside diff)

All consensus findings are outside the changed diff hunks and cannot be posted inline:

#	Severity	Consensus	File	Line(s)	Finding
1	🟢 Minor	2/3	AppleCommandsTests.cs, AndroidCommandsTests.cs	~70, ~160, ~254 (outside diff)	Dead originalServices variable: captured via var originalServices = Program.Services; but never used — finally calls Program.ResetServices() instead of restoring it. Harmless but misleading to future readers. Consider removing the dead capture.

Discarded findings (did not reach consensus)

• AppleSimulatorCommandsTests lacks [Collection("CLI")] — flagged by 1/3 reviewers as a proactive guard. Not a current bug (that class doesn't mutate Program.Services), discarded per consensus rules.

CI Status

Check	Status
build (macOS)	❌ Failed
build (Windows)	⚠️ Cancelled
license/cla	✅ Passed

⚠️ The macOS build failed — worth investigating whether this is related to the PR or a pre-existing issue (the PR only adds test attributes with no production code changes).

Test Coverage

The PR modifies only test infrastructure (adding [Collection] attributes). No production code changes, so no new test coverage is needed. The fix is self-verifying — if incorrect, the previously-flaky tests would still fail under parallel execution.

Generated by Expert Code Review · 3 independent reviewers with adversarial consensus

Generated by Expert Code Review (auto) for issue #222 · ● 4.1M · ◷