From 6bdd0e0575de7fbd77be895ed4e034c107c1a366 Mon Sep 17 00:00:00 2001 From: Alex Soto Date: Fri, 27 Mar 2026 16:24:05 -0400 Subject: [PATCH] Preserve explicit AndroidSdkHome when directory doesn't exist yet When a user specifies an explicit AndroidSdkHome directory (e.g. for Acquire/DownloadSdk), the SdkLocator.Locate() call discards it because PathLocator only returns paths where Directory.Exists() is true. This causes AndroidSdkHome to be null or resolve to a different SDK, making Acquire() fail with: 'Android SDK Directory was not specified.' Honor the explicitly provided path directly instead of passing it through Locate(). Only auto-discover via SdkLocator when no path is specified. Also remove the redundant re-assignment in the obsolete SdkTool(DirectoryInfo?) constructor which was overwriting the result from the delegated SdkToolOptions constructor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SdkTool_PreserveHome_Tests.cs | 141 ++++++++++++++++++ AndroidSdk/AndroidSdkManager.cs | 3 +- AndroidSdk/SdkTool.cs | 8 +- 3 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 AndroidSdk.Tests/SdkTool_PreserveHome_Tests.cs diff --git a/AndroidSdk.Tests/SdkTool_PreserveHome_Tests.cs b/AndroidSdk.Tests/SdkTool_PreserveHome_Tests.cs new file mode 100644 index 0000000..ef89f82 --- /dev/null +++ b/AndroidSdk.Tests/SdkTool_PreserveHome_Tests.cs @@ -0,0 +1,141 @@ +#nullable enable +using System; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace AndroidSdk.Tests; + +/// +/// Tests that SdkTool and AndroidSdkManager preserve explicitly specified +/// home directories even when the directory does not yet exist on disk. +/// This is critical for the Acquire/DownloadSdk workflow where the SDK +/// must be downloaded to a new directory. +/// +public class SdkTool_PreserveHome_Tests : TestsBase +{ + public SdkTool_PreserveHome_Tests(ITestOutputHelper outputHelper) + : base(outputHelper) + { + } + + [Fact] + public void SdkManager_PreservesHome_WhenDirectoryDoesNotExist() + { + var nonExistentPath = Path.Combine(Path.GetTempPath(), "AndroidSdk.Tests", "NonExistent", Guid.NewGuid().ToString("N")); + Assert.False(Directory.Exists(nonExistentPath)); + + var mgr = new SdkManager(new SdkManagerToolOptions + { + AndroidSdkHome = new DirectoryInfo(nonExistentPath), + SkipVersionCheck = true + }); + + Assert.NotNull(mgr.AndroidSdkHome); + Assert.Equal(nonExistentPath, mgr.AndroidSdkHome!.FullName); + } + + [Fact] + public void AndroidSdkManager_PreservesHome_WhenDirectoryDoesNotExist() + { + var nonExistentPath = Path.Combine(Path.GetTempPath(), "AndroidSdk.Tests", "NonExistent", Guid.NewGuid().ToString("N")); + Assert.False(Directory.Exists(nonExistentPath)); + + var mgr = new AndroidSdkManager(new DirectoryInfo(nonExistentPath)); + + Assert.NotNull(mgr.Home); + Assert.Equal(nonExistentPath, mgr.Home!.FullName); + } + + [Fact] + public async Task SdkManager_DownloadSdk_DoesNotThrow_WhenDirectoryDoesNotExist() + { + var nonExistentPath = Path.Combine(Path.GetTempPath(), "AndroidSdk.Tests", "NonExistent", Guid.NewGuid().ToString("N")); + Assert.False(Directory.Exists(nonExistentPath)); + + var mgr = new SdkManager(new SdkManagerToolOptions + { + AndroidSdkHome = new DirectoryInfo(nonExistentPath), + SkipVersionCheck = true + }); + + // DownloadSdk should not throw DirectoryNotFoundException + // because AndroidSdkHome should be preserved + var ex = await Record.ExceptionAsync(() => mgr.DownloadSdk()); + + // If it throws, it should NOT be the "Directory was not specified" error + if (ex != null) + { + Assert.DoesNotContain("was not specified", ex.Message); + } + + // Clean up if anything was created + try { if (Directory.Exists(nonExistentPath)) Directory.Delete(nonExistentPath, true); } catch { } + } + + [Fact] + public void SdkManager_UsesLocatedPath_WhenDirectoryExists() + { + // When the directory exists and contains an SDK, Locate should find it + var existingPath = Path.Combine(Path.GetTempPath(), "AndroidSdk.Tests", "ExistingDir", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(existingPath); + + try + { + var mgr = new SdkManager(new SdkManagerToolOptions + { + AndroidSdkHome = new DirectoryInfo(existingPath), + SkipVersionCheck = true + }); + + // Should still resolve to the existing path + Assert.NotNull(mgr.AndroidSdkHome); + Assert.Equal(existingPath, mgr.AndroidSdkHome!.FullName); + } + finally + { + try { Directory.Delete(existingPath, true); } catch { } + } + } + + [Fact] + public void AndroidSdkManager_UsesLocatedPath_WhenDirectoryExists() + { + var existingPath = Path.Combine(Path.GetTempPath(), "AndroidSdk.Tests", "ExistingDir", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(existingPath); + + try + { + var mgr = new AndroidSdkManager(new DirectoryInfo(existingPath)); + + Assert.NotNull(mgr.Home); + Assert.Equal(existingPath, mgr.Home!.FullName); + } + finally + { + try { Directory.Delete(existingPath, true); } catch { } + } + } + + [Fact] + public void SdkManager_HomeIsNull_WhenNoPathSpecified() + { + // When no path is given and no SDK is found via env vars, + // Home might be null or point to a discovered SDK — either is valid. + // The key invariant: if Locate() returns something, use it. + var mgr = new SdkManager(new SdkManagerToolOptions + { + AndroidSdkHome = null, + SkipVersionCheck = true + }); + + // AndroidSdkHome should be whatever Locate() found (possibly null) + var located = new SdkLocator().Locate()?.FirstOrDefault(); + if (located != null) + Assert.Equal(located.FullName, mgr.AndroidSdkHome?.FullName); + else + Assert.Null(mgr.AndroidSdkHome); + } +} diff --git a/AndroidSdk/AndroidSdkManager.cs b/AndroidSdk/AndroidSdkManager.cs index d0eec62..20edf9c 100644 --- a/AndroidSdk/AndroidSdkManager.cs +++ b/AndroidSdk/AndroidSdkManager.cs @@ -28,7 +28,8 @@ public static IEnumerable FindHome(string specificHome = null, pa public AndroidSdkManager(DirectoryInfo home = null) { - Home = new SdkLocator().Locate(home?.FullName)?.FirstOrDefault(); + Home = home + ?? new SdkLocator().Locate()?.FirstOrDefault(); SdkManager = new SdkManager(new SdkManagerToolOptions { AndroidSdkHome = Home }); AvdManager = new AvdManager(Home); diff --git a/AndroidSdk/SdkTool.cs b/AndroidSdk/SdkTool.cs index 822cf53..25ed37b 100644 --- a/AndroidSdk/SdkTool.cs +++ b/AndroidSdk/SdkTool.cs @@ -24,14 +24,16 @@ public SdkTool(string? androidSdkHome) public SdkTool(DirectoryInfo? androidSdkHome) : this(new SdkToolOptions { AndroidSdkHome = androidSdkHome }) { - AndroidSdkHome = new SdkLocator().Locate(androidSdkHome?.FullName)?.FirstOrDefault(); - Jdks = new JdkLocator().LocateJdk()?.ToArray() ?? new JdkInfo[0]; } public SdkTool(SdkToolOptions? options) { options ??= new(); - AndroidSdkHome = new SdkLocator().Locate(options.AndroidSdkHome?.FullName)?.FirstOrDefault(); + // When a path is explicitly provided, always honor it — the caller + // may intend to download/create the SDK there. Only auto-discover + // when no path is specified. + AndroidSdkHome = options.AndroidSdkHome + ?? new SdkLocator().Locate()?.FirstOrDefault(); Jdks = new JdkLocator().LocateJdk()?.ToArray() ?? new JdkInfo[0]; }