diff --git a/Assets/Tests/Editor/CliDetectionCommandRunnerTests.cs b/Assets/Tests/Editor/CliDetectionCommandRunnerTests.cs new file mode 100644 index 0000000000..3347b60436 --- /dev/null +++ b/Assets/Tests/Editor/CliDetectionCommandRunnerTests.cs @@ -0,0 +1,118 @@ +using System.Diagnostics; +using System.Threading; + +using NUnit.Framework; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies CLI detection command execution behavior. + /// + public class CliDetectionCommandRunnerTests + { + /// + /// Verifies that command execution preserves stdout line order and the exit code. + /// + [Test] + public void Execute_WhenCommandWritesMultipleLines_ReturnsOrderedLinesAndExitCode() + { + ProcessStartInfo startInfo = BuildOutputProcessStartInfo(0); + + CliDetectionCommandResult result = CliDetectionCommandRunner.Execute( + startInfo, + CancellationToken.None); + + Assert.That(result, Is.Not.Null); + Assert.That(result.StandardOutputLines, Is.EqualTo(new[] { "first", "second" })); + Assert.That(result.ExitCode, Is.Zero); + } + + /// + /// Verifies that command execution returns captured output for the detection policy to reject. + /// + [Test] + public void Execute_WhenCommandExitsWithFailure_ReturnsOutputAndExitCode() + { + ProcessStartInfo startInfo = BuildOutputProcessStartInfo(7); + + CliDetectionCommandResult result = CliDetectionCommandRunner.Execute( + startInfo, + CancellationToken.None); + + Assert.That(result, Is.Not.Null); + Assert.That(result.StandardOutputLines, Is.EqualTo(new[] { "first", "second" })); + Assert.That(result.ExitCode, Is.EqualTo(7)); + } + + /// + /// Verifies that a missing executable is represented by an absent execution result. + /// + [Test] + public void Execute_WhenExecutableDoesNotExist_ReturnsNull() + { + ProcessStartInfo startInfo = CreateStartInfo( + "__uloop_missing_cli_detection_test_executable__", + string.Empty); + + CliDetectionCommandResult result = CliDetectionCommandRunner.Execute( + startInfo, + CancellationToken.None); + + Assert.That(result, Is.Null); + } + + /// + /// Verifies that process cleanup tolerates the child exiting before Kill. + /// + [Test] + public void KillProcessIfRunning_WhenProcessAlreadyExited_DoesNotThrow() + { + ProcessStartInfo startInfo = BuildImmediateExitProcessStartInfo(); + + using Process process = Process.Start(startInfo); + process.WaitForExit(); + + Assert.DoesNotThrow(() => CliDetectionCommandRunner.KillProcessIfRunning(process)); + } + + private static ProcessStartInfo BuildOutputProcessStartInfo(int exitCode) + { + if (UnityEngine.Application.platform == RuntimePlatform.WindowsEditor) + { + return CreateStartInfo( + "cmd.exe", + $"/d /s /c \"echo first&&echo second&&exit /b {exitCode}\""); + } + + return CreateStartInfo( + "/bin/sh", + $"-c \"printf 'first\\nsecond\\n'; exit {exitCode}\""); + } + + private static ProcessStartInfo BuildImmediateExitProcessStartInfo() + { + if (UnityEngine.Application.platform == RuntimePlatform.WindowsEditor) + { + return CreateStartInfo("cmd.exe", "/c exit 0"); + } + + return CreateStartInfo("/bin/sh", "-c \"exit 0\""); + } + + private static ProcessStartInfo CreateStartInfo(string fileName, string arguments) + { + return new ProcessStartInfo + { + FileName = fileName, + Arguments = arguments, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + } + } +} diff --git a/Assets/Tests/Editor/CliDetectionCommandRunnerTests.cs.meta b/Assets/Tests/Editor/CliDetectionCommandRunnerTests.cs.meta new file mode 100644 index 0000000000..792a40441d --- /dev/null +++ b/Assets/Tests/Editor/CliDetectionCommandRunnerTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ab8af7e561eb4ad7a910dc74357c9974 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index 9d4c632c1a..3b94f0eab2 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -1,5 +1,3 @@ -using System.Diagnostics; - using NUnit.Framework; using io.github.hatayama.UnityCliLoop.Infrastructure; @@ -304,42 +302,5 @@ public void ParseShellCliInstallationOutput_WhenVersionCommandFails_ReturnsPathW Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/ExampleUser/.npm-global/bin/uloop")); } - [Test] - public void KillProcessIfRunning_WhenProcessAlreadyExited_DoesNotThrow() - { - // Verifies that process cleanup tolerates the race where the child exits before Kill. - ProcessStartInfo startInfo = BuildImmediateExitProcessStartInfo(); - - using Process process = Process.Start(startInfo); - process.WaitForExit(); - - Assert.DoesNotThrow(() => CliInstallationDetector.KillProcessIfRunning(process)); - } - - private static ProcessStartInfo BuildImmediateExitProcessStartInfo() - { - if (UnityEngine.Application.platform == UnityEngine.RuntimePlatform.WindowsEditor) - { - return new ProcessStartInfo - { - FileName = "cmd.exe", - Arguments = "/c exit 0", - UseShellExecute = false, - RedirectStandardOutput = true, - RedirectStandardError = true, - CreateNoWindow = true - }; - } - - return new ProcessStartInfo - { - FileName = "/bin/sh", - Arguments = "-c \"exit 0\"", - UseShellExecute = false, - RedirectStandardOutput = true, - RedirectStandardError = true, - CreateNoWindow = true - }; - } } } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliDetectionCommandRunner.cs b/Packages/src/Editor/Infrastructure/CLI/CliDetectionCommandRunner.cs new file mode 100644 index 0000000000..9ed5dedca2 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliDetectionCommandRunner.cs @@ -0,0 +1,90 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Describes the observable result of a completed CLI detection command. + /// + internal sealed class CliDetectionCommandResult + { + internal CliDetectionCommandResult(IReadOnlyList standardOutputLines, int exitCode) + { + UnityEngine.Debug.Assert(standardOutputLines != null, "standardOutputLines must not be null"); + + StandardOutputLines = standardOutputLines; + ExitCode = exitCode; + } + + internal IReadOnlyList StandardOutputLines { get; } + internal int ExitCode { get; } + } + + /// + /// Runs CLI detection commands and captures their process-level results. + /// + internal static class CliDetectionCommandRunner + { + private const int PROCESS_TIMEOUT_MS = 5000; + + internal static CliDetectionCommandResult Execute(ProcessStartInfo startInfo, CancellationToken ct) + { + UnityEngine.Debug.Assert(startInfo != null, "startInfo must not be null"); + UnityEngine.Debug.Assert(startInfo.RedirectStandardOutput, "RedirectStandardOutput must be true"); + UnityEngine.Debug.Assert(startInfo.RedirectStandardError, "RedirectStandardError must be true"); + + Process process = ProcessStartHelper.TryStart(startInfo); + if (process == null) + { + return null; + } + + using (process) + { + List standardOutputLines = new(); + process.OutputDataReceived += (sender, e) => + { + if (e.Data != null) + { + standardOutputLines.Add(e.Data); + } + }; + process.ErrorDataReceived += (sender, e) => { }; + + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + using CancellationTokenRegistration registration = ct.Register(() => KillProcessIfRunning(process)); + bool exited = process.WaitForExit(PROCESS_TIMEOUT_MS); + if (!exited) + { + KillProcessIfRunning(process); + return null; + } + + // Parameterless WaitForExit flushes async output buffers. + process.WaitForExit(); + return new CliDetectionCommandResult(standardOutputLines.ToArray(), process.ExitCode); + } + } + + internal static void KillProcessIfRunning(Process process) + { + UnityEngine.Debug.Assert(process != null, "process must not be null"); + + try + { + process.Kill(); + } + catch (System.InvalidOperationException) + { + // Process exit can race with cancellation or timeout cleanup. + } + } + } +} diff --git a/Packages/src/Editor/Infrastructure/CLI/CliDetectionCommandRunner.cs.meta b/Packages/src/Editor/Infrastructure/CLI/CliDetectionCommandRunner.cs.meta new file mode 100644 index 0000000000..b27cbbcff7 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliDetectionCommandRunner.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7650e6f84efa4e2493e0dacef228f908 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index e0a02f3bad..52d819f9e1 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -1,7 +1,6 @@ using System.Diagnostics; using System; using System.IO; -using System.Text; using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json; @@ -33,7 +32,6 @@ public CliInstallationDetection(string version, string executablePath, bool isDi /// public sealed class CliInstallationDetector : ICliInstallationDetector { - private const int PROCESS_TIMEOUT_MS = 5000; private const string SHELL_PATH_START_MARKER = "__ULOOP_PATH_START__"; private const string SHELL_PATH_END_MARKER = "__ULOOP_PATH_END__"; private const string SHELL_VERSION_START_MARKER = "__ULOOP_VERSION_START__"; @@ -217,7 +215,10 @@ private static CliInstallationDetection DetectShellCliInstallationFromLoginShell pathSetupPlan, Environment.GetEnvironmentVariable(CliConstants.POSIX_PATH_ENVIRONMENT_VARIABLE)); - string output = ExecuteAndGetOutput(startInfo, ct); + CliDetectionCommandResult commandResult = CliDetectionCommandRunner.Execute(startInfo, ct); + string output = commandResult == null + ? null + : string.Join(Environment.NewLine, commandResult.StandardOutputLines).Trim(); return ParseShellCliInstallationOutput(output); } @@ -421,60 +422,6 @@ private static CliInstallationDetection ParseDispatcherContract(JObject parsed, true); } - private static string ExecuteAndGetOutput(ProcessStartInfo startInfo, CancellationToken ct) - { - UnityEngine.Debug.Assert(startInfo != null, "startInfo must not be null"); - UnityEngine.Debug.Assert(startInfo.RedirectStandardOutput, "RedirectStandardOutput must be true"); - UnityEngine.Debug.Assert(startInfo.RedirectStandardError, "RedirectStandardError must be true"); - - Process process = ProcessStartHelper.TryStart(startInfo); - if (process == null) - { - return null; - } - - using (process) - { - StringBuilder outputBuilder = new(); - - process.OutputDataReceived += (sender, e) => - { - if (e.Data != null) - { - outputBuilder.AppendLine(e.Data); - } - }; - process.ErrorDataReceived += (sender, e) => { }; - - process.BeginOutputReadLine(); - process.BeginErrorReadLine(); - - using CancellationTokenRegistration registration = ct.Register(() => KillProcessIfRunning(process)); - bool exited = process.WaitForExit(PROCESS_TIMEOUT_MS); - if (!exited) - { - KillProcessIfRunning(process); - return null; - } - - process.WaitForExit(); - return outputBuilder.ToString().Trim(); - } - } - - internal static void KillProcessIfRunning(Process process) - { - UnityEngine.Debug.Assert(process != null, "process must not be null"); - - try - { - process.Kill(); - } - catch (System.InvalidOperationException) - { - } - } - private static bool IsSuccessfulShellStatus(string statusBlock) { string status = ExtractFirstNonEmptyLine(statusBlock); @@ -540,54 +487,21 @@ private static string ExecuteCliVersionCommand( CreateNoWindow = true }; - Process process = ProcessStartHelper.TryStart(startInfo); - if (process == null) - { - return null; - } - - StringBuilder outputBuilder = new(); - - process.OutputDataReceived += (sender, e) => - { - if (e.Data != null) - { - outputBuilder.Append(e.Data); - } - }; - process.ErrorDataReceived += (sender, e) => { }; - - process.BeginOutputReadLine(); - process.BeginErrorReadLine(); - - using CancellationTokenRegistration registration = ct.Register(() => - { - KillProcessIfRunning(process); - }); - try { - bool exited = process.WaitForExit(PROCESS_TIMEOUT_MS); - - if (!exited) + CliDetectionCommandResult commandResult = CliDetectionCommandRunner.Execute(startInfo, ct); + if (commandResult == null) { - KillProcessIfRunning(process); - process.Dispose(); return null; } - // Parameterless WaitForExit flushes async output buffers - process.WaitForExit(); - - string output = outputBuilder.ToString().Trim(); - bool failed = process.ExitCode != 0 || string.IsNullOrEmpty(output); - process.Dispose(); + string output = string.Concat(commandResult.StandardOutputLines).Trim(); + bool failed = commandResult.ExitCode != 0 || string.IsNullOrEmpty(output); return failed ? null : output; } catch (Exception ex) { - process.Dispose(); if (!ct.IsCancellationRequested) { UnityEngine.Debug.LogWarning($"[UnityCliLoop] Failed to detect CLI version: {ex.Message}");