Invoke onProgress callback in SdkManager.InstallPackagesAsync

[rmarinho]

Summary

SdkManager.InstallPackagesAsync accepts an Action<string, int, int>? onProgress parameter but never invoked it, making the Spectre live-progress UI in AndroidCommands.Install unable to show per-package installation progress.

Fix

When onProgress is provided, install packages one at a time and report progress for each:

for (var i = 0; i < packageList.Count; i++)
{
    onProgress(packageList[i], i + 1, packageList.Count);
    await _sdkManager.InstallAsync(new[] { packageList[i] }, acceptLicenses, cancellationToken);
}

When no callback is provided, the existing batch behavior is preserved (all packages in a single InstallAsync call).

Changes

• SdkManager.cs — iterate packages individually when onProgress is set
• FakeAndroidProvider.cs — updated fake to invoke onProgress for test fidelity
• AndroidProviderTests.cs — added InstallPackagesAsync_InvokesOnProgressForEachPackage test

Fixes #38

[Copilot]

Pull request overview

This PR fixes missing per-package progress reporting for Android SDK package installation by ensuring SdkManager.InstallPackagesAsync invokes the provided onProgress callback, enabling the Spectre live-progress UI to display package-level progress.

Changes:

• Update SdkManager.InstallPackagesAsync to install packages one-by-one when onProgress is provided, while preserving batch install behavior otherwise.
• Update FakeAndroidProvider.InstallPackagesAsync to invoke onProgress for improved test fidelity.
• Add a unit test validating onProgress is called once per package.

Reviewed changes

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

File	Description
src/Client/Microsoft.Maui.Client/Providers/Android/SdkManager.cs	Adds per-package install loop and invokes onProgress callback.
src/Client/Microsoft.Maui.Client.UnitTests/Fakes/FakeAndroidProvider.cs	Updates fake implementation to emit per-package progress callbacks.
src/Client/Microsoft.Maui.Client.UnitTests/AndroidProviderTests.cs	Adds test asserting progress callback invocation per package.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

onProgress is currently invoked with idx as i + 1 (1-based). The only in-repo caller (AndroidCommands.Install) formats ({idx + 1}/{total}), which will display 2..(total+1) and can show 4/3 on the last package. Either change the callback contract to be 0-based here (pass i) or update the caller (and document the contract) so all consumers agree on the indexing semantics.

Suggested change

	onProgress(packageList[i], i + 1, packageList.Count);
	// onProgress index is 0-based; callers can display (idx + 1)/total
	onProgress(packageList[i], i, packageList.Count);

[Copilot]

packages.ToList() is executed before the try/catch. If packages is null or its enumerator throws, the exception will bypass the existing error translation (UnauthorizedAccessException/MauiToolException) and change behavior compared to the previous implementation. Consider moving the ToList() inside the try block and/or validating packages with an explicit ArgumentNullException to keep error handling consistent.

Suggested change

	var packageList = packages.ToList();
	try
	{
	if (packages is null)
	throw new System.ArgumentNullException(nameof(packages));
	try
	{
	var packageList = packages.ToList();

[Copilot]

The fake invokes onProgress with idx as i + 1 (1-based). If production code/callers expect 0-based indexing (as AndroidCommands.Install currently does), this will cause tests to pass while the UI is off-by-one. Align the fake with the chosen contract (0-based vs 1-based) and keep it consistent with SdkManager.InstallPackagesAsync.

Suggested change

	onProgress(pkgList[i], i + 1, pkgList.Count);
	onProgress(pkgList[i], i, pkgList.Count);

[Copilot]

This test asserts 1-based idx values (1..N). However, the CLI formatting in AndroidCommands.Install uses idx + 1 when displaying ({idx + 1}/{total}), implying it expects 0-based indexing. Update the test expectations to match the finalized onProgress contract (and ensure the production caller aligns), otherwise the UI can end up off-by-one while tests still pass.

Suggested change

	Assert.Equal(("platform-tools", 1, 3), progressCalls[0]);
	Assert.Equal(("build-tools;35.0.0", 2, 3), progressCalls[1]);
	Assert.Equal(("platforms;android-35", 3, 3), progressCalls[2]);
	Assert.Equal(("platform-tools", 0, 3), progressCalls[0]);
	Assert.Equal(("build-tools;35.0.0", 1, 3), progressCalls[1]);
	Assert.Equal(("platforms;android-35", 2, 3), progressCalls[2]);

[jfversluis]

this completes the last open item from the PR #24 review by properly invoking the onProgress callback in InstallPackagesAsync. Clean implementation with batch vs one-at-a-time logic and good test coverage.LGTM