Skip to content

Commit 8b3c6c5

Browse files
rmarinhoCopilot
rmarinho
and
Copilot
committed
Address review feedback: xcrun xcodebuild, version param, tests
- Use xcrun xcodebuild instead of resolving xcodebuild path inside Xcode bundle - Remove ResolveXcodebuild and XcodebuildRelativePath (rolfbjarne feedback) - Add version parameter to DownloadPlatform (-buildVersion flag) - Add input validation to ListByPlatform - Catch InvalidOperationException alongside Win32Exception - Add RuntimeServiceTests with smoke tests - Adopt file-scoped namespaces and flat usings - Sync SimctlOutputParser and tests.csproj from PR #158 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a80e415 commit 8b3c6c5

2 files changed

Lines changed: 139 additions & 116 deletions

File tree

‎Xamarin.MacDev/RuntimeService.cs‎

Lines changed: 87 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -5,147 +5,118 @@
55
using System.Collections.Generic;
66
using System.IO;
77
using System.Linq;
8-
98
using Xamarin.MacDev.Models;
109

1110
#nullable enable
1211

13-
namespace Xamarin.MacDev {
14-
15-
/// <summary>
16-
/// Manages simulator runtimes via <c>xcrun simctl</c> and <c>xcodebuild</c>.
17-
/// Lists installed runtimes and supports downloading new platform runtimes.
18-
/// Download approach from ClientTools.Platform: <c>xcodebuild -downloadPlatform iOS</c>.
19-
/// </summary>
20-
public class RuntimeService {
21-
22-
static readonly string XcrunPath = "/usr/bin/xcrun";
23-
static readonly string XcodebuildRelativePath = "Contents/Developer/usr/bin/xcodebuild";
12+
namespace Xamarin.MacDev;
2413

25-
readonly ICustomLogger log;
14+
/// <summary>
15+
/// Manages simulator runtimes via <c>xcrun simctl</c> and <c>xcrun xcodebuild</c>.
16+
/// </summary>
17+
public class RuntimeService {
2618

27-
public RuntimeService (ICustomLogger log)
28-
{
29-
this.log = log ?? throw new ArgumentNullException (nameof (log));
30-
}
19+
static readonly string XcrunPath = "/usr/bin/xcrun";
3120

32-
/// <summary>
33-
/// Lists installed simulator runtimes. Optionally filters by availability.
34-
/// Uses <c>xcrun simctl list runtimes --json</c>.
35-
/// </summary>
36-
public List<SimulatorRuntimeInfo> List (bool availableOnly = false)
37-
{
38-
var json = RunSimctl ("list", "runtimes", "--json");
39-
if (json is null)
40-
return new List<SimulatorRuntimeInfo> ();
21+
readonly ICustomLogger log;
4122

42-
var runtimes = SimctlOutputParser.ParseRuntimes (json);
23+
public RuntimeService (ICustomLogger log)
24+
{
25+
this.log = log ?? throw new ArgumentNullException (nameof (log));
26+
}
4327

44-
if (availableOnly)
45-
runtimes.RemoveAll (r => !r.IsAvailable);
28+
/// <summary>
29+
/// Lists installed simulator runtimes. Optionally filters by availability.
30+
/// </summary>
31+
public List<SimulatorRuntimeInfo> List (bool availableOnly = false)
32+
{
33+
var json = RunSimctl ("list", "runtimes", "--json");
34+
if (json is null)
35+
return new List<SimulatorRuntimeInfo> ();
4636

47-
log.LogInfo ("Found {0} simulator runtime(s).", runtimes.Count);
48-
return runtimes;
49-
}
37+
var runtimes = SimctlOutputParser.ParseRuntimes (json);
5038

51-
/// <summary>
52-
/// Lists runtimes for a specific platform (e.g. "iOS", "tvOS", "watchOS", "visionOS").
53-
/// </summary>
54-
public List<SimulatorRuntimeInfo> ListByPlatform (string platform, bool availableOnly = false)
55-
{
56-
var all = List (availableOnly);
57-
return all.Where (r => string.Equals (r.Platform, platform, StringComparison.OrdinalIgnoreCase)).ToList ();
58-
}
39+
if (availableOnly)
40+
runtimes.RemoveAll (r => !r.IsAvailable);
5941

60-
/// <summary>
61-
/// Downloads a platform runtime using <c>xcodebuild -downloadPlatform</c>.
62-
/// Pattern from ClientTools.Platform RemoteSimulatorValidator.
63-
/// </summary>
64-
/// <param name="platform">The platform to download (e.g. "iOS", "tvOS", "watchOS", "visionOS").</param>
65-
/// <param name="xcodePath">The Xcode.app path. If null, looks for xcodebuild in PATH via xcrun.</param>
66-
/// <returns>True if the download command succeeded.</returns>
67-
public bool DownloadPlatform (string platform, string? xcodePath = null)
68-
{
69-
if (string.IsNullOrEmpty (platform))
70-
throw new ArgumentException ("Platform must not be null or empty.", nameof (platform));
71-
72-
var xcodebuildPath = ResolveXcodebuild (xcodePath);
73-
if (xcodebuildPath is null) {
74-
log.LogInfo ("Cannot download platform: xcodebuild not found.");
75-
return false;
76-
}
42+
log.LogInfo ("Found {0} simulator runtime(s).", runtimes.Count);
43+
return runtimes;
44+
}
7745

78-
log.LogInfo ("Downloading {0} platform runtime via xcodebuild...", platform);
46+
/// <summary>
47+
/// Lists runtimes for a specific platform (e.g. "iOS", "tvOS", "watchOS", "visionOS").
48+
/// </summary>
49+
public List<SimulatorRuntimeInfo> ListByPlatform (string platform, bool availableOnly = false)
50+
{
51+
if (string.IsNullOrEmpty (platform))
52+
throw new ArgumentException ("Platform must not be null or empty.", nameof (platform));
7953

80-
try {
81-
var (exitCode, stdout, stderr) = ProcessUtils.Exec (xcodebuildPath, "-downloadPlatform", platform);
82-
if (exitCode != 0) {
83-
log.LogInfo ("xcodebuild -downloadPlatform {0} failed (exit {1}): {2}", platform, exitCode, stderr.Trim ());
84-
return false;
85-
}
54+
var all = List (availableOnly);
55+
return all.Where (r => string.Equals (r.Platform, platform, StringComparison.OrdinalIgnoreCase)).ToList ();
56+
}
8657

87-
log.LogInfo ("Successfully downloaded {0} platform runtime.", platform);
88-
return true;
89-
} catch (System.ComponentModel.Win32Exception ex) {
90-
log.LogInfo ("Could not run xcodebuild: {0}", ex.Message);
58+
/// <summary>
59+
/// Downloads a platform runtime using <c>xcrun xcodebuild -downloadPlatform</c>.
60+
/// </summary>
61+
/// <param name="platform">The platform to download (e.g. "iOS", "tvOS", "watchOS", "visionOS").</param>
62+
/// <param name="version">Optional specific version to download (e.g. "17.5").</param>
63+
/// <returns>True if the download command succeeded.</returns>
64+
public bool DownloadPlatform (string platform, string? version = null)
65+
{
66+
if (string.IsNullOrEmpty (platform))
67+
throw new ArgumentException ("Platform must not be null or empty.", nameof (platform));
68+
69+
log.LogInfo ("Downloading {0} platform runtime via xcodebuild...", platform);
70+
71+
try {
72+
var args = string.IsNullOrEmpty (version)
73+
? new [] { "xcodebuild", "-downloadPlatform", platform }
74+
: new [] { "xcodebuild", "-downloadPlatform", platform, "-buildVersion", version! };
75+
76+
var (exitCode, _, stderr) = ProcessUtils.Exec (XcrunPath, args);
77+
if (exitCode != 0) {
78+
log.LogInfo ("xcodebuild -downloadPlatform {0} failed (exit {1}): {2}", platform, exitCode, stderr.Trim ());
9179
return false;
9280
}
93-
}
94-
95-
/// <summary>
96-
/// Resolves the path to xcodebuild. If xcodePath is given, looks inside the Xcode bundle.
97-
/// Otherwise falls back to /usr/bin/xcrun xcodebuild.
98-
/// </summary>
99-
string? ResolveXcodebuild (string? xcodePath)
100-
{
101-
if (!string.IsNullOrEmpty (xcodePath)) {
102-
var path = Path.Combine (xcodePath!, XcodebuildRelativePath);
103-
if (File.Exists (path))
104-
return path;
105-
}
10681

107-
// Fall back to xcrun to find xcodebuild
108-
if (File.Exists (XcrunPath)) {
109-
try {
110-
var (exitCode, stdout, _) = ProcessUtils.Exec (XcrunPath, "--find", "xcodebuild");
111-
if (exitCode == 0) {
112-
var path = stdout.Trim ();
113-
if (File.Exists (path))
114-
return path;
115-
}
116-
} catch (System.ComponentModel.Win32Exception) {
117-
// fall through
118-
}
119-
}
82+
log.LogInfo ("Successfully downloaded {0} platform runtime.", platform);
83+
return true;
84+
} catch (System.ComponentModel.Win32Exception ex) {
85+
log.LogInfo ("Could not run xcodebuild: {0}", ex.Message);
86+
return false;
87+
} catch (InvalidOperationException ex) {
88+
log.LogInfo ("Could not run xcodebuild: {0}", ex.Message);
89+
return false;
90+
}
91+
}
12092

93+
/// <summary>
94+
/// Runs a simctl subcommand and returns stdout, or null on failure.
95+
/// </summary>
96+
string? RunSimctl (params string [] args)
97+
{
98+
if (!File.Exists (XcrunPath)) {
99+
log.LogInfo ("xcrun not found at '{0}'.", XcrunPath);
121100
return null;
122101
}
123102

124-
/// <summary>
125-
/// Runs a simctl subcommand and returns stdout, or null on failure.
126-
/// </summary>
127-
string? RunSimctl (params string [] args)
128-
{
129-
if (!File.Exists (XcrunPath)) {
130-
log.LogInfo ("xcrun not found at '{0}'.", XcrunPath);
131-
return null;
132-
}
103+
var fullArgs = new string [args.Length + 1];
104+
fullArgs [0] = "simctl";
105+
Array.Copy (args, 0, fullArgs, 1, args.Length);
133106

134-
var fullArgs = new string [args.Length + 1];
135-
fullArgs [0] = "simctl";
136-
Array.Copy (args, 0, fullArgs, 1, args.Length);
137-
138-
try {
139-
var (exitCode, stdout, stderr) = ProcessUtils.Exec (XcrunPath, fullArgs);
140-
if (exitCode != 0) {
141-
log.LogInfo ("simctl {0} failed (exit {1}): {2}", args [0], exitCode, stderr.Trim ());
142-
return null;
143-
}
144-
return stdout;
145-
} catch (System.ComponentModel.Win32Exception ex) {
146-
log.LogInfo ("Could not run xcrun: {0}", ex.Message);
107+
try {
108+
var (exitCode, stdout, stderr) = ProcessUtils.Exec (XcrunPath, fullArgs);
109+
if (exitCode != 0) {
110+
log.LogInfo ("simctl {0} failed (exit {1}): {2}", args [0], exitCode, stderr.Trim ());
147111
return null;
148112
}
113+
return stdout;
114+
} catch (System.ComponentModel.Win32Exception ex) {
115+
log.LogInfo ("Could not run xcrun: {0}", ex.Message);
116+
return null;
117+
} catch (InvalidOperationException ex) {
118+
log.LogInfo ("Could not run xcrun: {0}", ex.Message);
119+
return null;
149120
}
150121
}
151122
}

‎tests/RuntimeServiceTests.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using NUnit.Framework;
6+
using Xamarin.MacDev;
7+
8+
#nullable enable
9+
10+
namespace tests;
11+
12+
[TestFixture]
13+
public class RuntimeServiceTests {
14+
15+
[Test]
16+
public void Constructor_ThrowsOnNullLogger ()
17+
{
18+
Assert.Throws<ArgumentNullException> (() => new RuntimeService (null!));
19+
}
20+
21+
[Test]
22+
public void ListByPlatform_ThrowsOnNullPlatform ()
23+
{
24+
var svc = new RuntimeService (ConsoleLogger.Instance);
25+
Assert.Throws<ArgumentException> (() => svc.ListByPlatform (null!));
26+
Assert.Throws<ArgumentException> (() => svc.ListByPlatform (""));
27+
}
28+
29+
[Test]
30+
public void DownloadPlatform_ThrowsOnNullPlatform ()
31+
{
32+
var svc = new RuntimeService (ConsoleLogger.Instance);
33+
Assert.Throws<ArgumentException> (() => svc.DownloadPlatform (null!));
34+
Assert.Throws<ArgumentException> (() => svc.DownloadPlatform (""));
35+
}
36+
37+
[Test]
38+
[Platform ("MacOsX")]
39+
public void List_DoesNotThrow ()
40+
{
41+
var svc = new RuntimeService (ConsoleLogger.Instance);
42+
Assert.DoesNotThrow (() => svc.List ());
43+
}
44+
45+
[Test]
46+
[Platform ("MacOsX")]
47+
public void ListByPlatform_DoesNotThrow ()
48+
{
49+
var svc = new RuntimeService (ConsoleLogger.Instance);
50+
Assert.DoesNotThrow (() => svc.ListByPlatform ("iOS"));
51+
}
52+
}

0 commit comments

Comments
 (0)