diff --git a/Assets/Tests/Editor/CliInstallationDetectorCacheTests.cs b/Assets/Tests/Editor/CliInstallationDetectorCacheTests.cs new file mode 100644 index 0000000000..5d2ebb743c --- /dev/null +++ b/Assets/Tests/Editor/CliInstallationDetectorCacheTests.cs @@ -0,0 +1,348 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies how the CLI installation detector caches, refreshes, and invalidates detection results. + /// + public sealed class CliInstallationDetectorCacheTests + { + private const string DetectedVersion = "3.6.0"; + private const string DetectedPath = "/bin/uloop"; + + /// + /// Verifies a refresh stores the detected version, path, and dispatcher flag and marks the check completed. + /// + [Test] + public async Task RefreshCliVersionAsync_WhenDetectionSucceeds_CachesDetection() + { + CountingDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath, true)); + CliInstallationDetector detector = CreateDetector(detection); + + await detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detector.IsCheckCompleted(), Is.True); + Assert.That(detector.IsCliInstalled(), Is.True); + Assert.That(detector.GetCachedCliVersion(), Is.EqualTo(DetectedVersion)); + Assert.That(detector.GetCachedCliExecutablePath(), Is.EqualTo(DetectedPath)); + Assert.That(detector.GetCachedCliIsDispatcher(), Is.True); + Assert.That(detection.CallCount, Is.EqualTo(1)); + } + + /// + /// Verifies a refresh hands the caller's token to the detection so cancelling it can stop the CLI process. + /// + [Test] + public async Task RefreshCliVersionAsync_PassesTheCallerTokenToDetection() + { + CountingDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath)); + CliInstallationDetector detector = CreateDetector(detection); + using CancellationTokenSource cancellation = new(); + + await detector.RefreshCliVersionAsync(cancellation.Token); + + Assert.That(detection.ReceivedTokens, Is.EqualTo(new[] { cancellation.Token })); + } + + /// + /// Verifies a forced refresh hands the caller's token to the detection so cancelling it can stop the CLI process. + /// + [Test] + public async Task ForceRefreshCliVersionAsync_PassesTheCallerTokenToDetection() + { + CountingDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath)); + CliInstallationDetector detector = CreateDetector(detection); + using CancellationTokenSource cancellation = new(); + + await detector.ForceRefreshCliVersionAsync(cancellation.Token); + + Assert.That(detection.ReceivedTokens, Is.EqualTo(new[] { cancellation.Token })); + } + + /// + /// Verifies a completed check without a detected version reports the CLI as not installed. + /// + [Test] + public async Task RefreshCliVersionAsync_WhenNoVersionIsDetected_ReportsNotInstalled() + { + CountingDetection detection = new(new CliInstallationDetection(null, DetectedPath)); + CliInstallationDetector detector = CreateDetector(detection); + + await detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detector.IsCheckCompleted(), Is.True); + Assert.That(detector.IsCliInstalled(), Is.False); + Assert.That(detector.GetCachedCliExecutablePath(), Is.EqualTo(DetectedPath)); + Assert.That(detector.GetCachedCliIsDispatcher(), Is.False); + } + + /// + /// Verifies a second refresh reuses the cached result instead of detecting again. + /// + [Test] + public async Task RefreshCliVersionAsync_WhenCacheIsInitialized_DoesNotDetectAgain() + { + CountingDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath)); + CliInstallationDetector detector = CreateDetector(detection); + await detector.RefreshCliVersionAsync(CancellationToken.None); + + await detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detection.CallCount, Is.EqualTo(1)); + } + + /// + /// Verifies a refresh requested while another refresh is pending returns without starting a second detection. + /// + [Test] + public async Task RefreshCliVersionAsync_WhenRefreshIsPending_DoesNotStartSecondDetection() + { + PendingDetection detection = new(); + CliInstallationDetector detector = CreateDetector(detection); + Task firstRefresh = detector.RefreshCliVersionAsync(CancellationToken.None); + + Task secondRefresh = detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(secondRefresh.IsCompleted, Is.True); + Assert.That(detection.CallCount, Is.EqualTo(1)); + Assert.That(detector.IsCheckCompleted(), Is.False); + + detection.Complete(new CliInstallationDetection(DetectedVersion, DetectedPath)); + await firstRefresh; + + Assert.That(detector.GetCachedCliVersion(), Is.EqualTo(DetectedVersion)); + } + + /// + /// Verifies a failed detection clears the in-progress flag so the next refresh detects again. + /// + [Test] + public async Task RefreshCliVersionAsync_WhenDetectionThrows_AllowsNextRefresh() + { + ThrowOnceDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath)); + CliInstallationDetector detector = CreateDetector(detection); + + // Awaited in try / catch instead of Assert.ThrowsAsync, which blocks the main thread in this NUnit. + try + { + await detector.RefreshCliVersionAsync(CancellationToken.None); + Assert.Fail("Expected the detection failure to propagate."); + } + catch (InvalidOperationException exception) + { + Assert.That(exception.Message, Is.EqualTo(ThrowOnceDetection.ErrorMessage)); + } + + Assert.That(detector.IsCheckCompleted(), Is.False); + + await detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detection.CallCount, Is.EqualTo(2)); + Assert.That(detector.GetCachedCliVersion(), Is.EqualTo(DetectedVersion)); + } + + /// + /// Verifies a forced refresh detects again and replaces an already cached result. + /// + [Test] + public async Task ForceRefreshCliVersionAsync_WhenCacheIsInitialized_ReplacesCachedDetection() + { + SequenceDetection detection = new( + new CliInstallationDetection(DetectedVersion, DetectedPath, true), + new CliInstallationDetection("3.7.0", "/other/uloop")); + CliInstallationDetector detector = CreateDetector(detection); + await detector.RefreshCliVersionAsync(CancellationToken.None); + + await detector.ForceRefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detection.CallCount, Is.EqualTo(2)); + Assert.That(detector.GetCachedCliVersion(), Is.EqualTo("3.7.0")); + Assert.That(detector.GetCachedCliExecutablePath(), Is.EqualTo("/other/uloop")); + Assert.That(detector.GetCachedCliIsDispatcher(), Is.False); + Assert.That(detector.IsCheckCompleted(), Is.True); + } + + /// + /// Verifies a forced refresh fills the cache even when no refresh ran before. + /// + [Test] + public async Task ForceRefreshCliVersionAsync_WhenCacheIsEmpty_CachesDetection() + { + CountingDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath, true)); + CliInstallationDetector detector = CreateDetector(detection); + + await detector.ForceRefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detector.IsCheckCompleted(), Is.True); + Assert.That(detector.GetCachedCliVersion(), Is.EqualTo(DetectedVersion)); + Assert.That(detector.GetCachedCliIsDispatcher(), Is.True); + } + + /// + /// Verifies invalidating the cache hides the cached result and lets the next refresh detect again. + /// + [Test] + public async Task InvalidateCache_WhenCacheIsInitialized_ClearsResultAndAllowsRefresh() + { + CountingDetection detection = new(new CliInstallationDetection(DetectedVersion, DetectedPath, true)); + CliInstallationDetector detector = CreateDetector(detection); + await detector.RefreshCliVersionAsync(CancellationToken.None); + + detector.InvalidateCache(); + + Assert.That(detector.IsCheckCompleted(), Is.False); + Assert.That(detector.IsCliInstalled(), Is.False); + Assert.That(detector.GetCachedCliVersion(), Is.Null); + Assert.That(detector.GetCachedCliExecutablePath(), Is.Null); + Assert.That(detector.GetCachedCliIsDispatcher(), Is.False); + + await detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detection.CallCount, Is.EqualTo(2)); + } + + /// + /// Verifies invalidating the cache during a pending refresh lets a new refresh start its own detection. + /// + [Test] + public async Task InvalidateCache_WhenRefreshIsPending_AllowsNewRefresh() + { + PendingDetection detection = new(); + CliInstallationDetector detector = CreateDetector(detection); + Task firstRefresh = detector.RefreshCliVersionAsync(CancellationToken.None); + + detector.InvalidateCache(); + Task secondRefresh = detector.RefreshCliVersionAsync(CancellationToken.None); + + Assert.That(detection.CallCount, Is.EqualTo(2)); + + detection.Complete(new CliInstallationDetection(DetectedVersion, DetectedPath)); + await firstRefresh; + await secondRefresh; + + Assert.That(detector.GetCachedCliVersion(), Is.EqualTo(DetectedVersion)); + } + + private static CliInstallationDetector CreateDetector(IFakeDetection detection) + { + CliInstallationDetector detector = new(new UnusedPinReader()); + detector.SetDetectionForTesting(detection.DetectAsync); + return detector; + } + + private interface IFakeDetection + { + Task DetectAsync(CancellationToken ct); + } + + private sealed class CountingDetection : IFakeDetection + { + private readonly CliInstallationDetection _result; + + public CountingDetection(CliInstallationDetection result) + { + _result = result; + } + + public int CallCount { get; private set; } + + public List ReceivedTokens { get; } = new(); + + public Task DetectAsync(CancellationToken ct) + { + CallCount++; + ReceivedTokens.Add(ct); + return Task.FromResult(_result); + } + } + + private sealed class SequenceDetection : IFakeDetection + { + private readonly CliInstallationDetection[] _results; + + public SequenceDetection(params CliInstallationDetection[] results) + { + _results = results; + } + + public int CallCount { get; private set; } + + public Task DetectAsync(CancellationToken ct) + { + CliInstallationDetection result = _results[CallCount]; + CallCount++; + return Task.FromResult(result); + } + } + + private sealed class ThrowOnceDetection : IFakeDetection + { + public const string ErrorMessage = "detection failed in this test"; + + private readonly CliInstallationDetection _result; + + public ThrowOnceDetection(CliInstallationDetection result) + { + _result = result; + } + + public int CallCount { get; private set; } + + public Task DetectAsync(CancellationToken ct) + { + CallCount++; + if (CallCount == 1) + { + return Task.FromException(new InvalidOperationException(ErrorMessage)); + } + + return Task.FromResult(_result); + } + } + + // Every call shares one task that the test completes itself, so nothing outlives the test. + private sealed class PendingDetection : IFakeDetection + { + private readonly TaskCompletionSource _completion = new(); + + public int CallCount { get; private set; } + + public Task DetectAsync(CancellationToken ct) + { + CallCount++; + return _completion.Task; + } + + public void Complete(CliInstallationDetection result) + { + _completion.SetResult(result); + } + } + + private sealed class UnusedPinReader : ICliPinReader + { + public CliPinLoadResult LoadPackagePin() + { + throw new InvalidOperationException("The cache tests never read the pin."); + } + + public DispatcherBootstrapPinLoadResult LoadDispatcherBootstrapPin() + { + throw new InvalidOperationException("The cache tests never read the pin."); + } + + public string LoadMinimumDispatcherVersionOrThrow() + { + throw new InvalidOperationException("The cache tests never read the pin."); + } + } + } +} diff --git a/Assets/Tests/Editor/CliInstallationDetectorCacheTests.cs.meta b/Assets/Tests/Editor/CliInstallationDetectorCacheTests.cs.meta new file mode 100644 index 0000000000..537452009b --- /dev/null +++ b/Assets/Tests/Editor/CliInstallationDetectorCacheTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: c99b2772b0a054948bd907ecc799de9d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliInstallationDetectorVersionCommandTests.cs b/Assets/Tests/Editor/CliInstallationDetectorVersionCommandTests.cs new file mode 100644 index 0000000000..19125c4ffb --- /dev/null +++ b/Assets/Tests/Editor/CliInstallationDetectorVersionCommandTests.cs @@ -0,0 +1,305 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Text.RegularExpressions; +using System.Threading; + +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies how the CLI installation detector runs and interprets the CLI version commands. + /// + public sealed class CliInstallationDetectorVersionCommandTests + { + private const string ExecutablePath = "/bin/uloop"; + private const string ContractArguments = CliConstants.VERSION_FLAG + " " + CliConstants.JSON_FLAG; + + private int _warningCount; + + [SetUp] + public void SetUp() + { + _warningCount = 0; + UnityEngine.Application.logMessageReceived += CountWarning; + } + + [TearDown] + public void TearDown() + { + UnityEngine.Application.logMessageReceived -= CountWarning; + } + + /// + /// Verifies a version read from the JSON contract output is used without running the plain version command. + /// + [Test] + public void DetectCliInstallationAtExecutablePath_WhenContractReportsVersion_UsesContractVersion() + { + RecordingCommandRunner runner = new(); + runner.Add(ContractArguments, new CliDetectionCommandResult(new[] { "{\"DispatcherVersion\":\"3.6.2\"}" }, 0)); + + CliInstallationDetection detection = CliInstallationDetector.DetectCliInstallationAtExecutablePath( + ExecutablePath, + CancellationToken.None, + runner.Execute); + + Assert.That(detection.Version, Is.EqualTo("3.6.2")); + Assert.That(detection.IsDispatcher, Is.True); + Assert.That(detection.ExecutablePath, Is.EqualTo(ExecutablePath)); + Assert.That(runner.Arguments, Is.EqualTo(new[] { ContractArguments })); + Assert.That(runner.FileNames, Is.EqualTo(new[] { ExecutablePath })); + } + + /// + /// Verifies the plain version output is used when the JSON contract output has no version. + /// + [Test] + public void DetectCliInstallationAtExecutablePath_WhenContractHasNoVersion_FallsBackToPlainVersion() + { + RecordingCommandRunner runner = new(); + runner.Add(ContractArguments, new CliDetectionCommandResult(new[] { "not json" }, 0)); + runner.Add(CliConstants.VERSION_FLAG, new CliDetectionCommandResult(new[] { "2.9.0" }, 0)); + + CliInstallationDetection detection = CliInstallationDetector.DetectCliInstallationAtExecutablePath( + ExecutablePath, + CancellationToken.None, + runner.Execute); + + Assert.That(detection.Version, Is.EqualTo("2.9.0")); + Assert.That(detection.IsDispatcher, Is.False); + Assert.That(detection.ExecutablePath, Is.EqualTo(ExecutablePath)); + Assert.That(runner.Arguments, Is.EqualTo(new[] { ContractArguments, CliConstants.VERSION_FLAG })); + } + + /// + /// Verifies both version commands receive the caller's token so cancelling it can stop the CLI process. + /// + [Test] + public void DetectCliInstallationAtExecutablePath_PassesTheCallerTokenToEveryCommand() + { + RecordingCommandRunner runner = new(); + runner.Add(ContractArguments, new CliDetectionCommandResult(Array.Empty(), 1)); + runner.Add(CliConstants.VERSION_FLAG, new CliDetectionCommandResult(new[] { "2.9.0" }, 0)); + using CancellationTokenSource cancellation = new(); + + CliInstallationDetector.DetectCliInstallationAtExecutablePath( + ExecutablePath, + cancellation.Token, + runner.Execute); + + Assert.That(runner.Tokens, Is.EqualTo(new[] { cancellation.Token, cancellation.Token })); + } + + /// + /// Verifies the version command receives the caller's token so cancelling it can stop the CLI process. + /// + [Test] + public void ExecuteCliVersionCommand_PassesTheCallerTokenToTheCommand() + { + RecordingCommandRunner runner = new(); + runner.Add(CliConstants.VERSION_FLAG, new CliDetectionCommandResult(new[] { "3.6.0" }, 0)); + using CancellationTokenSource cancellation = new(); + + CliInstallationDetector.ExecuteCliVersionCommand( + ExecutablePath, + CliConstants.VERSION_FLAG, + cancellation.Token, + runner.Execute); + + Assert.That(runner.Tokens, Is.EqualTo(new[] { cancellation.Token })); + } + + /// + /// Verifies both version commands failing reports no version while keeping the executable path. + /// + [Test] + public void DetectCliInstallationAtExecutablePath_WhenBothCommandsFail_ReportsNoVersion() + { + RecordingCommandRunner runner = new(); + runner.Add(ContractArguments, new CliDetectionCommandResult(Array.Empty(), 1)); + runner.Add(CliConstants.VERSION_FLAG, new CliDetectionCommandResult(new[] { "2.9.0" }, 1)); + + CliInstallationDetection detection = CliInstallationDetector.DetectCliInstallationAtExecutablePath( + ExecutablePath, + CancellationToken.None, + runner.Execute); + + Assert.That(detection.Version, Is.Null); + Assert.That(detection.ExecutablePath, Is.EqualTo(ExecutablePath)); + } + + /// + /// Verifies a missing executable path runs the commands through the bare executable name. + /// + [Test] + public void DetectCliInstallationAtExecutablePath_WhenPathIsNull_RunsExecutableName() + { + RecordingCommandRunner runner = new(); + runner.Add(ContractArguments, new CliDetectionCommandResult(new[] { "{\"ProjectRunnerVersion\":\"3.6.0\"}" }, 0)); + + CliInstallationDetection detection = CliInstallationDetector.DetectCliInstallationAtExecutablePath( + null, + CancellationToken.None, + runner.Execute); + + Assert.That(detection.Version, Is.EqualTo("3.6.0")); + Assert.That(detection.ExecutablePath, Is.Null); + Assert.That(runner.FileNames, Is.EqualTo(new[] { CliConstants.EXECUTABLE_NAME })); + } + + /// + /// Verifies a successful command returns its output lines joined and trimmed, and runs with redirected output. + /// + [Test] + public void ExecuteCliVersionCommand_WhenCommandSucceeds_ReturnsTrimmedOutput() + { + RecordingCommandRunner runner = new(); + runner.Add(CliConstants.VERSION_FLAG, new CliDetectionCommandResult(new[] { " 3.6", ".0 " }, 0)); + + string output = CliInstallationDetector.ExecuteCliVersionCommand( + ExecutablePath, + CliConstants.VERSION_FLAG, + CancellationToken.None, + runner.Execute); + + Assert.That(output, Is.EqualTo("3.6.0")); + ProcessStartInfo startInfo = runner.StartInfos[0]; + Assert.That(startInfo.UseShellExecute, Is.False); + Assert.That(startInfo.RedirectStandardOutput, Is.True); + Assert.That(startInfo.RedirectStandardError, Is.True); + Assert.That(startInfo.CreateNoWindow, Is.True); + } + + /// + /// Verifies a command that could not run, exited with an error, or printed nothing yields no output without warning. + /// + [TestCase(false, 0, "3.6.0")] + [TestCase(true, 1, "3.6.0")] + [TestCase(true, 0, " ")] + public void ExecuteCliVersionCommand_WhenCommandDoesNotSucceed_ReturnsNull( + bool hasResult, + int exitCode, + string outputLine) + { + RecordingCommandRunner runner = new(); + runner.Add( + CliConstants.VERSION_FLAG, + hasResult ? new CliDetectionCommandResult(new[] { outputLine }, exitCode) : null); + + string output = CliInstallationDetector.ExecuteCliVersionCommand( + ExecutablePath, + CliConstants.VERSION_FLAG, + CancellationToken.None, + runner.Execute); + + Assert.That(output, Is.Null); + Assert.That(_warningCount, Is.EqualTo(0)); + } + + /// + /// Verifies a command that throws yields no output and logs a warning. + /// + [Test] + public void ExecuteCliVersionCommand_WhenCommandThrows_ReturnsNullAndWarns() + { + LogAssert.Expect(LogType.Warning, new Regex("Failed to detect CLI version: command failed in this test")); + + string output = CliInstallationDetector.ExecuteCliVersionCommand( + ExecutablePath, + CliConstants.VERSION_FLAG, + CancellationToken.None, + ThrowingRunner); + + Assert.That(output, Is.Null); + Assert.That(_warningCount, Is.EqualTo(1)); + } + + /// + /// Verifies a command that throws after cancellation yields no output without logging a warning. + /// + [Test] + public void ExecuteCliVersionCommand_WhenCommandThrowsAfterCancellation_ReturnsNullSilently() + { + using CancellationTokenSource cancellation = new(); + cancellation.Cancel(); + + string output = CliInstallationDetector.ExecuteCliVersionCommand( + ExecutablePath, + CliConstants.VERSION_FLAG, + cancellation.Token, + ThrowingRunner); + + Assert.That(output, Is.Null); + Assert.That(_warningCount, Is.EqualTo(0)); + } + + private void CountWarning(string condition, string stackTrace, LogType type) + { + if (type == LogType.Warning) + { + _warningCount++; + } + } + + private static CliDetectionCommandResult ThrowingRunner(ProcessStartInfo startInfo, CancellationToken ct) + { + throw new InvalidOperationException("command failed in this test"); + } + + private sealed class RecordingCommandRunner + { + private readonly Dictionary _resultsByArguments = new(); + + public List StartInfos { get; } = new(); + + public List Tokens { get; } = new(); + + public List Arguments + { + get + { + List arguments = new(); + foreach (ProcessStartInfo startInfo in StartInfos) + { + arguments.Add(startInfo.Arguments); + } + + return arguments; + } + } + + public List FileNames + { + get + { + List fileNames = new(); + foreach (ProcessStartInfo startInfo in StartInfos) + { + fileNames.Add(startInfo.FileName); + } + + return fileNames; + } + } + + public void Add(string arguments, CliDetectionCommandResult result) + { + _resultsByArguments.Add(arguments, result); + } + + public CliDetectionCommandResult Execute(ProcessStartInfo startInfo, CancellationToken ct) + { + StartInfos.Add(startInfo); + Tokens.Add(ct); + return _resultsByArguments[startInfo.Arguments]; + } + } + } +} diff --git a/Assets/Tests/Editor/CliInstallationDetectorVersionCommandTests.cs.meta b/Assets/Tests/Editor/CliInstallationDetectorVersionCommandTests.cs.meta new file mode 100644 index 0000000000..1e4b46aa41 --- /dev/null +++ b/Assets/Tests/Editor/CliInstallationDetectorVersionCommandTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 58d44273ca58c4549a0f7cd6956df8f4 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliPlayModeRunInBackgroundServiceTests.cs b/Assets/Tests/Editor/CliPlayModeRunInBackgroundServiceTests.cs new file mode 100644 index 0000000000..61b329e9d1 --- /dev/null +++ b/Assets/Tests/Editor/CliPlayModeRunInBackgroundServiceTests.cs @@ -0,0 +1,150 @@ +#nullable enable +using NUnit.Framework; +using UnityEditor; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the service applies the controller's runInBackground decisions on CLI Play start and Play Mode exit, + /// using an in-memory store and a recorded runInBackground value instead of SessionState and the real Application setting. + /// + public sealed class CliPlayModeRunInBackgroundServiceTests + { + private InMemoryStore _store = new(); + private bool _runInBackground; + private int _setCount; + private CliPlayModeRunInBackgroundService _service = null!; + + [SetUp] + public void SetUp() + { + _store = new InMemoryStore(); + _runInBackground = false; + _setCount = 0; + _service = new CliPlayModeRunInBackgroundService( + new CliPlayModeRunInBackgroundController(_store), + value => + { + _setCount++; + _runInBackground = value; + }, + () => _runInBackground); + } + + /// + /// Verifies a CLI Play start records the current value as the original and turns runInBackground on. + /// + [Test] + public void EnableForCliPlayStart_RecordsOriginalAndEnablesRunInBackground() + { + _service.EnableForCliPlayStart(); + + Assert.That(_runInBackground, Is.True); + Assert.That(_store.IsActive, Is.True); + Assert.That(_store.OriginalRunInBackground, Is.False); + } + + /// + /// Verifies exiting Play Mode re-applies the original value but keeps the override active until Edit Mode is stable. + /// + [Test] + public void OnPlayModeStateChanged_ExitingPlayModeAfterCliStart_ReappliesOriginalAndKeepsOverride() + { + _service.EnableForCliPlayStart(); + + _service.OnPlayModeStateChanged(PlayModeStateChange.ExitingPlayMode); + + Assert.That(_runInBackground, Is.False); + Assert.That(_store.IsActive, Is.True); + } + + /// + /// Verifies entering Edit Mode restores the original value and clears the override. + /// + [Test] + public void OnPlayModeStateChanged_EnteredEditModeAfterCliStart_RestoresOriginalAndClearsOverride() + { + _service.EnableForCliPlayStart(); + + _service.OnPlayModeStateChanged(PlayModeStateChange.EnteredEditMode); + + Assert.That(_runInBackground, Is.False); + Assert.That(_store.IsActive, Is.False); + } + + /// + /// Verifies Play Mode exit after a manual Play start never writes runInBackground. + /// + [Test] + public void OnPlayModeStateChanged_WithoutCliStart_DoesNotWriteRunInBackground() + { + _service.OnPlayModeStateChanged(PlayModeStateChange.ExitingPlayMode); + _service.OnPlayModeStateChanged(PlayModeStateChange.EnteredEditMode); + + Assert.That(_setCount, Is.EqualTo(0)); + } + + /// + /// Verifies transitions other than exiting Play Mode and entering Edit Mode leave runInBackground and the override alone. + /// + [Test] + public void OnPlayModeStateChanged_EnteredPlayMode_LeavesOverrideUntouched() + { + _service.EnableForCliPlayStart(); + int setCountAfterStart = _setCount; + + _service.OnPlayModeStateChanged(PlayModeStateChange.EnteredPlayMode); + _service.OnPlayModeStateChanged(PlayModeStateChange.ExitingEditMode); + + Assert.That(_setCount, Is.EqualTo(setCountAfterStart)); + Assert.That(_runInBackground, Is.True); + Assert.That(_store.IsActive, Is.True); + } + + /// + /// Verifies a project that already had runInBackground on gets it back after CLI Play, even when Unity + /// overwrote the value during the transition, instead of having it turned off. + /// + [Test] + public void OnPlayModeStateChanged_WhenOriginalWasOn_RestoresOnAfterPlayModeExit() + { + _runInBackground = true; + + _service.EnableForCliPlayStart(); + + Assert.That(_store.OriginalRunInBackground, Is.True); + + _runInBackground = false; + _service.OnPlayModeStateChanged(PlayModeStateChange.ExitingPlayMode); + + Assert.That(_runInBackground, Is.True); + + _runInBackground = false; + _service.OnPlayModeStateChanged(PlayModeStateChange.EnteredEditMode); + + Assert.That(_runInBackground, Is.True); + Assert.That(_store.IsActive, Is.False); + } + + private sealed class InMemoryStore : ICliPlayModeRunInBackgroundStore + { + public bool IsActive { get; private set; } + + public bool OriginalRunInBackground { get; private set; } + + public void Activate(bool originalRunInBackground) + { + IsActive = true; + OriginalRunInBackground = originalRunInBackground; + } + + public void Clear() + { + IsActive = false; + OriginalRunInBackground = false; + } + } + } +} diff --git a/Assets/Tests/Editor/CliPlayModeRunInBackgroundServiceTests.cs.meta b/Assets/Tests/Editor/CliPlayModeRunInBackgroundServiceTests.cs.meta new file mode 100644 index 0000000000..b53c09876e --- /dev/null +++ b/Assets/Tests/Editor/CliPlayModeRunInBackgroundServiceTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0d69f29e6d5f344a69830390c5a0ea45 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CompileControllerPipelineTests.cs b/Assets/Tests/Editor/CompileControllerPipelineTests.cs new file mode 100644 index 0000000000..70da971f47 --- /dev/null +++ b/Assets/Tests/Editor/CompileControllerPipelineTests.cs @@ -0,0 +1,372 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using UnityEditor.Compilation; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies CompileController drives the compile request through its pipeline port: + /// event registration, completion, the wait-for-existing-task path, and cleanup. + /// + [TestFixture] + public sealed class CompileControllerPipelineTests + { + private UnityCliLoopEditorSessionStateSnapshot _originalSnapshot; + + [SetUp] + public void SetUp() + { + _originalSnapshot = UnityCliLoopEditorSessionStateTestFactory.CaptureSnapshot(); + } + + [TearDown] + public void TearDown() + { + CompileApiUpdaterConsentState.EndCliCompile(); + _originalSnapshot.Restore(); + } + + /// + /// What: a normal compile refreshes assets, registers both callbacks, requests a non-clean + /// compile, starts the watchdog, and completes with the messages the assembly callback reported. + /// + [Test] + public async Task TryCompileAsync_WhenPipelineFinishes_ReturnsReportedMessagesAndClearsState() + { + FakeCompilePipelinePort pipeline = new() + { + CompleteOnRequest = true, + ReportedMessages = new[] { CreateMessage(CompilerMessageType.Warning, "sample warning") } + }; + using CompileController controller = CreateController(pipeline); + List startedMessages = new(); + List compiledAssemblies = new(); + controller.OnCompileStarted += startedMessages.Add; + controller.OnAssemblyCompiled += (assemblyName, _) => compiledAssemblies.Add(assemblyName); + + CompileResult result = await UncanceledAwaits.AwaitValueAsync(controller.TryCompileAsync( + forceRecompile: false, + playModeStopWarning: null, + CancellationToken.None)); + + Assert.That(result.Success, Is.True); + Assert.That(result.WarningCount, Is.EqualTo(1)); + Assert.That(pipeline.RefreshCount, Is.EqualTo(1)); + Assert.That(pipeline.SubscribeCount, Is.EqualTo(1)); + Assert.That(pipeline.UnsubscribeCount, Is.EqualTo(1)); + Assert.That(pipeline.MismatchedUnsubscribeCount, Is.EqualTo(0)); + Assert.That(pipeline.RequestedCleanBuildCache, Is.EqualTo(new[] { false })); + Assert.That(pipeline.WatchdogStartCount, Is.EqualTo(1)); + Assert.That(startedMessages, Is.EqualTo(new[] { "Compilation started after asset refresh..." })); + Assert.That(compiledAssemblies, Is.EqualTo(new[] { "Sample.dll" })); + Assert.That(controller.IsCompiling, Is.False); + Assert.That(CompileApiUpdaterConsentState.IsCliCompileInFlight, Is.False); + } + + /// + /// What: a forced recompile requests a clean build cache and returns an indeterminate result. + /// + [Test] + public async Task TryCompileAsync_WhenForced_RequestsCleanBuildCacheAndReturnsIndeterminateResult() + { + FakeCompilePipelinePort pipeline = new() { CompleteOnRequest = true }; + using CompileController controller = CreateController(pipeline); + List startedMessages = new(); + controller.OnCompileStarted += startedMessages.Add; + + CompileResult result = await UncanceledAwaits.AwaitValueAsync(controller.TryCompileAsync( + forceRecompile: true, + playModeStopWarning: null, + CancellationToken.None)); + + Assert.That(result.IsIndeterminate, Is.True); + Assert.That(pipeline.RequestedCleanBuildCache, Is.EqualTo(new[] { true })); + Assert.That(startedMessages, Is.EqualTo(new[] { "Forced recompile started after asset refresh..." })); + } + + /// + /// What: Assembly Definition errors found after the refresh stop the compile before any callback + /// is registered or compile is requested, and the controller returns to idle. + /// + [Test] + public async Task TryCompileAsync_WhenAssemblyDefinitionErrorsExist_ReturnsFailureWithoutRequestingCompile() + { + // Why CompleteOnRequest: if the early return regresses, the request still finishes + // and the assertions fail instead of the test hanging on a compile that never ends. + FakeCompilePipelinePort pipeline = new() + { + CompleteOnRequest = true, + AssemblyDefinitionErrors = new AssemblyDefinitionConsoleErrorResult(new[] + { + new AssemblyDefinitionConsoleError("broken asmdef", "Assets/Broken.asmdef", 1) + }) + }; + using CompileController controller = CreateController(pipeline); + + CompileResult result = await UncanceledAwaits.AwaitValueAsync(controller.TryCompileAsync( + forceRecompile: false, + playModeStopWarning: null, + CancellationToken.None)); + + Assert.That(result.Success, Is.False); + Assert.That(pipeline.RefreshCount, Is.EqualTo(1)); + Assert.That(pipeline.SubscribeCount, Is.EqualTo(0)); + Assert.That(pipeline.RequestedCleanBuildCache, Is.Empty); + Assert.That(pipeline.WatchdogStartCount, Is.EqualTo(0)); + Assert.That(controller.IsCompiling, Is.False); + Assert.That(CompileApiUpdaterConsentState.IsCliCompileInFlight, Is.False); + } + + /// + /// What: a second TryCompileAsync during an in-flight compile waits for the existing request + /// instead of starting another one, and both callers receive the same result. + /// + [Test] + public async Task TryCompileAsync_WhenAlreadyCompiling_WaitsForExistingRequest() + { + FakeCompilePipelinePort pipeline = new(); + using CompileController controller = CreateController(pipeline); + + Task firstCompile = controller.TryCompileAsync( + forceRecompile: false, + playModeStopWarning: null, + CancellationToken.None); + Task secondCompile = controller.TryCompileAsync( + forceRecompile: true, + playModeStopWarning: null, + CancellationToken.None); + + // Why before completing: if the second call started its own compile, the first request + // would never finish, so the check must fail before awaiting it. + Assert.That(pipeline.RefreshCount, Is.EqualTo(1)); + Assert.That(pipeline.RequestedCleanBuildCache, Is.EqualTo(new[] { false })); + Assert.That(controller.IsCompiling, Is.True); + Assert.That(secondCompile.IsCompleted, Is.False); + pipeline.CompilationFinished(null); + CompileResult firstResult = await UncanceledAwaits.AwaitValueAsync(firstCompile); + CompileResult secondResult = await UncanceledAwaits.AwaitValueAsync(secondCompile); + + Assert.That(secondResult, Is.SameAs(firstResult)); + } + + /// + /// What: when requesting compile throws after callbacks were registered, the controller + /// unregisters them, clears the in-flight state, and lets the exception reach the caller. + /// + [Test] + public async Task TryCompileAsync_WhenRequestThrows_UnregistersCallbacksAndClearsState() + { + FakeCompilePipelinePort pipeline = new() + { + RequestException = new InvalidOperationException("request failed") + }; + using CompileController controller = CreateController(pipeline); + + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await UncanceledAwaits.AwaitValueAsync(controller.TryCompileAsync( + forceRecompile: false, + playModeStopWarning: null, + CancellationToken.None)); + Assert.Fail("TryCompileAsync should rethrow the request failure."); + } + catch (InvalidOperationException e) + { + Assert.That(e.Message, Is.EqualTo("request failed")); + } + + Assert.That(pipeline.UnsubscribeCount, Is.EqualTo(1)); + Assert.That(pipeline.MismatchedUnsubscribeCount, Is.EqualTo(0)); + Assert.That(pipeline.WatchdogStartCount, Is.EqualTo(0)); + Assert.That(controller.IsCompiling, Is.False); + Assert.That(CompileApiUpdaterConsentState.IsCliCompileInFlight, Is.False); + } + + /// + /// What: Cleanup unregisters the callbacks through the port and cancels a pending compile request. + /// + [Test] + public async Task Cleanup_WhenCompileIsPending_UnregistersCallbacksAndCancelsRequest() + { + FakeCompilePipelinePort pipeline = new(); + using CompileController controller = CreateController(pipeline); + Task pendingCompile = controller.TryCompileAsync( + forceRecompile: false, + playModeStopWarning: null, + CancellationToken.None); + + controller.Cleanup(); + + Assert.That(pipeline.UnsubscribeCount, Is.EqualTo(1)); + Assert.That(pipeline.MismatchedUnsubscribeCount, Is.EqualTo(0)); + Assert.That(controller.IsCompiling, Is.False); + Assert.That(CompileApiUpdaterConsentState.IsCliCompileInFlight, Is.False); + try + { + await pendingCompile; + Assert.Fail("The pending compile should be canceled by Cleanup."); + } + catch (TaskCanceledException) + { + } + } + + /// + /// What: replacing the pipeline during a pending compile is rejected, so Cleanup still unsubscribes from the original port. + /// + [Test] + public async Task SetCompilePipelineForTesting_WhenCompileIsPending_RejectsReplacementAndKeepsOriginalPort() + { + FakeCompilePipelinePort pipeline = new(); + FakeCompilePipelinePort replacement = new(); + using CompileController controller = CreateController(pipeline); + Task pendingCompile = controller.TryCompileAsync( + forceRecompile: false, + playModeStopWarning: null, + CancellationToken.None); + + try + { + controller.SetCompilePipelineForTesting(replacement); + Assert.Fail("Replacing the pipeline during a pending compile should be rejected."); + } + catch (InvalidOperationException) + { + } + + controller.Cleanup(); + + Assert.That(pipeline.UnsubscribeCount, Is.EqualTo(1)); + Assert.That(pipeline.MismatchedUnsubscribeCount, Is.EqualTo(0)); + Assert.That(replacement.UnsubscribeCount + replacement.MismatchedUnsubscribeCount, Is.EqualTo(0)); + try + { + await pendingCompile; + Assert.Fail("The pending compile should be canceled by Cleanup."); + } + catch (TaskCanceledException) + { + } + } + + private static CompileController CreateController(FakeCompilePipelinePort pipeline) + { + CompileController controller = new( + UnityCliLoopEditorSessionStateTestFactory.CreateCompileResultSessionRepository(), + UnityCliLoopEditorSessionStateTestFactory.CreatePendingCompileSessionRepository()); + // Why: the real resolver subscribes to Scene and Play Mode events and reads the open Scenes. + controller.SetExternalSceneChangeResolutionForTesting(_ => (true, null, Array.Empty())); + controller.SetCompilePipelineForTesting(pipeline); + return controller; + } + + private static CompilerMessage CreateMessage(CompilerMessageType type, string message) + { + return new CompilerMessage + { + type = type, + message = message, + file = "Assets/Sample.cs", + line = 1, + column = 1 + }; + } + + /// + /// Records pipeline calls and optionally finishes the compile synchronously on request. + /// + private sealed class FakeCompilePipelinePort : ICompilePipelinePort + { + private Action _compilationFinished; + private Action _assemblyFinished; + + public bool CompleteOnRequest { get; set; } + public CompilerMessage[] ReportedMessages { get; set; } = Array.Empty(); + public AssemblyDefinitionConsoleErrorResult AssemblyDefinitionErrors { get; set; } = + new(Array.Empty()); + public Exception RequestException { get; set; } + public int RefreshCount { get; private set; } + public int SubscribeCount { get; private set; } + public int UnsubscribeCount { get; private set; } + public int MismatchedUnsubscribeCount { get; private set; } + public int WatchdogStartCount { get; private set; } + public List RequestedCleanBuildCache { get; } = new(); + + public void RefreshAssets() + { + RefreshCount++; + } + + public AssemblyDefinitionConsoleErrorResult FindCurrentAssemblyDefinitionErrors() + { + return AssemblyDefinitionErrors; + } + + public UnityCliLoopConsoleLogEntry[] ReadConsoleErrorEntries() + { + return Array.Empty(); + } + + public void SubscribeCompilationEvents( + Action compilationFinished, + Action assemblyFinished) + { + SubscribeCount++; + _compilationFinished = compilationFinished; + _assemblyFinished = assemblyFinished; + } + + public void UnsubscribeCompilationEvents( + Action compilationFinished, + Action assemblyFinished) + { + // Why compare delegates: production unsubscribes with -=, which silently removes nothing + // when it is handed a different delegate than the one it subscribed. + if (compilationFinished == _compilationFinished && assemblyFinished == _assemblyFinished) + { + UnsubscribeCount++; + return; + } + + MismatchedUnsubscribeCount++; + } + + public void RequestScriptCompilation(bool cleanBuildCache) + { + RequestedCleanBuildCache.Add(cleanBuildCache); + if (RequestException != null) + { + throw RequestException; + } + + if (!CompleteOnRequest) + { + return; + } + + _assemblyFinished("Library/ScriptAssemblies/Sample.dll", ReportedMessages); + _compilationFinished(null); + } + + public void StartWatchdog( + CompileLifecycleRecoveryCoordinator coordinator, + TaskCompletionSource compileTask, + CancellationToken ct) + { + WatchdogStartCount++; + } + + public void CompilationFinished(object context) + { + _compilationFinished(context); + } + } + } +} diff --git a/Assets/Tests/Editor/CompileControllerPipelineTests.cs.meta b/Assets/Tests/Editor/CompileControllerPipelineTests.cs.meta new file mode 100644 index 0000000000..b46069c5c4 --- /dev/null +++ b/Assets/Tests/Editor/CompileControllerPipelineTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 343cf8dcc404443ab9c17048fb93078e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ControlPlayModeUseCaseTests.cs b/Assets/Tests/Editor/ControlPlayModeUseCaseTests.cs index 27af15bb18..b8e1cd882f 100644 --- a/Assets/Tests/Editor/ControlPlayModeUseCaseTests.cs +++ b/Assets/Tests/Editor/ControlPlayModeUseCaseTests.cs @@ -880,12 +880,16 @@ public async Task ExecuteAsync_WhenPlayStartsFreshSession_SetsPlayingAndReportsW StubEditorUnsavedChangesQuietSaver quietSaver = new( saveFailures: System.Array.Empty(), remainingAfterSave: System.Array.Empty()); + // Why a recording starter: the default is the Editor's real service, which would write + // Application.runInBackground and leave the CLI override active in SessionState. + RecordingRunInBackgroundStarter runInBackgroundStarter = new(); ControlPlayModeUseCase useCase = new ControlPlayModeUseCase( new StubCompilationFailureProvider(System.Array.Empty()), new StubCompilationFailureGate(false), quietSaver, editorState, - new StubDomainReloadDropStateProvider()); + new StubDomainReloadDropStateProvider(), + runInBackgroundStarter: runInBackgroundStarter); ControlPlayModeSchema schema = new ControlPlayModeSchema { Action = PlayModeAction.Play, @@ -897,6 +901,7 @@ public async Task ExecuteAsync_WhenPlayStartsFreshSession_SetsPlayingAndReportsW Assert.That(response.ResumedFromPause, Is.False); Assert.That(response.Warning, Is.EqualTo(ControlPlayModeUseCase.FreshPlayStartFromNewSessionWarning)); Assert.That(editorState.IsPlaying, Is.True); + Assert.That(runInBackgroundStarter.EnableCallCount, Is.EqualTo(1)); } /// @@ -915,7 +920,8 @@ public async Task ExecuteAsync_WhenPlayStartsWithActivePatches_AppendsDropWarnin new StubCompilationFailureGate(false), quietSaver, editorState, - new StubDomainReloadDropStateProvider(changeCount: 2)); + new StubDomainReloadDropStateProvider(changeCount: 2), + runInBackgroundStarter: new RecordingRunInBackgroundStarter()); ControlPlayModeSchema schema = new ControlPlayModeSchema { Action = PlayModeAction.Play, @@ -950,7 +956,8 @@ public async Task ExecuteAsync_WhenPlayStartsWithAPersistedPausePoint_ReportsThe new StubCompilationFailureGate(false), quietSaver, editorState, - new StubDomainReloadDropStateProvider(pausePointCount: 1, persistedPausePointCount: 1)); + new StubDomainReloadDropStateProvider(pausePointCount: 1, persistedPausePointCount: 1), + runInBackgroundStarter: new RecordingRunInBackgroundStarter()); ControlPlayModeSchema schema = new ControlPlayModeSchema { Action = PlayModeAction.Play, @@ -1006,7 +1013,8 @@ public async Task ExecuteAsync_WhenPlayStartsWithZeroDropCounts_KeepsOnlyFreshSt new StubCompilationFailureGate(false), quietSaver, editorState, - new StubDomainReloadDropStateProvider()); + new StubDomainReloadDropStateProvider(), + runInBackgroundStarter: new RecordingRunInBackgroundStarter()); ControlPlayModeSchema schema = new ControlPlayModeSchema { Action = PlayModeAction.Play, diff --git a/Assets/Tests/Editor/EditorFrameWaiterServiceTests.cs b/Assets/Tests/Editor/EditorFrameWaiterServiceTests.cs index 17b7a7672c..d0766aef5b 100644 --- a/Assets/Tests/Editor/EditorFrameWaiterServiceTests.cs +++ b/Assets/Tests/Editor/EditorFrameWaiterServiceTests.cs @@ -1,3 +1,7 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -6,6 +10,8 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor { /// /// Test fixture that verifies the frame waiter service state that the static facade tests do not observe. + /// Frames are advanced by calling UpdateRequests directly and the timeout is a fake the test completes, + /// so no test registers on the real Editor update loop or waits on wall-clock time. /// public sealed class EditorFrameWaiterServiceTests { @@ -19,5 +25,193 @@ public void PendingWaitCount_WhenServiceIsNew_ReturnsZero() Assert.That(service.PendingWaitCount, Is.EqualTo(0)); } + + /// + /// Verifies that a zero-frame wait succeeds immediately without starting a timeout. + /// + [Test] + public async Task WaitFramesOrTimeoutAsync_WhenFrameCountIsZero_ReturnsTrueWithoutTimeout() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + + bool completed = await UncanceledAwaits.AwaitValueAsync( + service.WaitFramesOrTimeoutAsync(0, 100, CancellationToken.None)); + + Assert.That(completed, Is.True); + Assert.That(timeout.Requests, Is.Empty); + } + + /// + /// Verifies that the wait completes only once the requested number of frames has elapsed, + /// and that reaching the frames cancels the pending timeout. + /// + [Test] + public async Task WaitFramesOrTimeoutAsync_WhenFramesElapse_ReturnsTrueAndCancelsTimeout() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + + Task wait = service.WaitFramesOrTimeoutAsync(2, 100, CancellationToken.None); + service.UpdateRequests(); + + Assert.That(wait.IsCompleted, Is.False); + Assert.That(service.PendingWaitCount, Is.EqualTo(1)); + + service.UpdateRequests(); + // Why before the await: a frame that never completes would otherwise hang until the test timeout. + Assert.That(service.PendingWaitCount, Is.EqualTo(0)); + bool completed = await UncanceledAwaits.AwaitValueAsync(wait); + + Assert.That(completed, Is.True); + Assert.That(timeout.Requests, Is.EqualTo(new[] { 100 })); + Assert.That(timeout.LastTask.IsCanceled, Is.True); + } + + /// + /// Verifies that one frame tick completes only the requests whose target frame was reached. + /// + [Test] + public async Task UpdateRequests_WhenRequestsTargetDifferentFrames_CompletesOnlyReadyRequests() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + + Task shortWait = service.WaitFramesOrTimeoutAsync(1, 100, CancellationToken.None); + Task longWait = service.WaitFramesOrTimeoutAsync(3, 100, CancellationToken.None); + service.UpdateRequests(); + Assert.That(service.PendingWaitCount, Is.EqualTo(1)); + bool shortCompleted = await UncanceledAwaits.AwaitValueAsync(shortWait); + + Assert.That(shortCompleted, Is.True); + Assert.That(longWait.IsCompleted, Is.False); + + service.UpdateRequests(); + service.UpdateRequests(); + Assert.That(service.PendingWaitCount, Is.EqualTo(0)); + bool longCompleted = await UncanceledAwaits.AwaitValueAsync(longWait); + + Assert.That(longCompleted, Is.True); + } + + /// + /// Verifies that a timeout before the frames arrive returns false and removes the frame request. + /// + [Test] + public async Task WaitFramesOrTimeoutAsync_WhenTimeoutElapsesFirst_ReturnsFalseAndRemovesRequest() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + + Task wait = service.WaitFramesOrTimeoutAsync(2, 100, CancellationToken.None); + timeout.CompleteLast(); + bool completed = await UncanceledAwaits.AwaitValueAsync(wait); + + Assert.That(completed, Is.False); + Assert.That(service.PendingWaitCount, Is.EqualTo(0)); + } + + /// + /// Verifies that canceling the caller token cancels the wait and removes the frame request. + /// + [Test] + public async Task WaitFramesOrTimeoutAsync_WhenCallerCancels_ThrowsAndRemovesRequest() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + using CancellationTokenSource cancellationSource = new CancellationTokenSource(); + + Task wait = service.WaitFramesOrTimeoutAsync(2, 100, cancellationSource.Token); + cancellationSource.Cancel(); + + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await wait; + Assert.Fail("The wait should be canceled with the caller token."); + } + catch (OperationCanceledException) + { + } + + Assert.That(service.PendingWaitCount, Is.EqualTo(0)); + } + + /// + /// Verifies that an already-canceled caller token is rejected before any frame request is queued. + /// + [Test] + public async Task WaitFramesOrTimeoutAsync_WhenTokenIsAlreadyCanceled_ThrowsWithoutQueueing() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + using CancellationTokenSource cancellationSource = new CancellationTokenSource(); + cancellationSource.Cancel(); + + try + { + await service.WaitFramesOrTimeoutAsync(2, 100, cancellationSource.Token); + Assert.Fail("The wait should reject an already-canceled token."); + } + catch (OperationCanceledException) + { + } + + Assert.That(service.PendingWaitCount, Is.EqualTo(0)); + Assert.That(timeout.Requests, Is.Empty); + } + + /// + /// Verifies that ClearAllForTests cancels a pending wait and cancels its timeout. + /// + [Test] + public async Task ClearAllForTests_WhenWaitIsPending_CancelsWaitAndTimeout() + { + FakeTimeout timeout = new FakeTimeout(); + EditorFrameWaiterService service = new EditorFrameWaiterService(timeout.Wait); + + Task wait = service.WaitFramesOrTimeoutAsync(2, 100, CancellationToken.None); + service.ClearAllForTests(); + + try + { + await wait; + Assert.Fail("ClearAllForTests should cancel the pending wait."); + } + catch (OperationCanceledException) + { + } + + Assert.That(service.PendingWaitCount, Is.EqualTo(0)); + Assert.That(timeout.LastTask.IsCanceled, Is.True); + } + + /// + /// Timeout fake built like TimerDelay.Wait: continuations run asynchronously and the token cancels it, + /// so canceling a token never runs the service's continuation inline on the test thread. + /// + private sealed class FakeTimeout + { + private TaskCompletionSource _lastSource; + + public List Requests { get; } = new List(); + + public Task LastTask => _lastSource.Task; + + public Task Wait(int milliseconds, CancellationToken ct) + { + Requests.Add(milliseconds); + TaskCompletionSource source = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + ct.Register(() => source.TrySetCanceled(ct)); + _lastSource = source; + return source.Task; + } + + public void CompleteLast() + { + _lastSource.TrySetResult(true); + } + } } } diff --git a/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs b/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs index c1aee892e1..ac386ea028 100644 --- a/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs +++ b/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs @@ -317,5 +317,105 @@ public void ExtractDirectoryServiceUserShell_WhenNoUserShellLineExists_ReturnsNu Assert.That(result, Is.Null); } + + /// + /// Verifies a .cmd or .exe entry is chosen over an earlier extensionless entry, ignoring extension case. + /// + [TestCase(@"C:\tools\uloop.cmd")] + [TestCase(@"C:\tools\uloop.EXE")] + public void SelectWindowsExecutable_WhenLaunchableEntryFollowsExtensionlessEntry_ReturnsLaunchableEntry( + string launchablePath) + { + string[] paths = { @"C:\tools\uloop", launchablePath }; + + string result = NodeEnvironmentResolver.SelectWindowsExecutable(paths); + + Assert.That(result, Is.EqualTo(launchablePath)); + } + + /// + /// Verifies the first entry is chosen when no entry has a .cmd or .exe extension. + /// + [Test] + public void SelectWindowsExecutable_WhenNoLaunchableEntryExists_ReturnsFirstEntry() + { + string[] paths = { @"C:\tools\uloop", @"C:\tools\uloop.ps1" }; + + string result = NodeEnvironmentResolver.SelectWindowsExecutable(paths); + + Assert.That(result, Is.EqualTo(@"C:\tools\uloop")); + } + + /// + /// Verifies that missing or empty where results select nothing. + /// + [Test] + public void SelectWindowsExecutable_WhenNoPathsExist_ReturnsNull() + { + Assert.That(NodeEnvironmentResolver.SelectWindowsExecutable(null), Is.Null); + Assert.That(NodeEnvironmentResolver.SelectWindowsExecutable(Array.Empty()), Is.Null); + } + + /// + /// Verifies CRLF where output is split into trimmed paths without carriage returns or blank lines. + /// + [Test] + public void ParseWhereOutput_WhenOutputUsesCrlf_ReturnsTrimmedPaths() + { + string output = "C:\\tools\\uloop\r\n\r\n C:\\tools\\uloop.cmd \r\n"; + + string[] result = NodeEnvironmentResolver.ParseWhereOutput(output); + + Assert.That(result, Is.EqualTo(new[] { @"C:\tools\uloop", @"C:\tools\uloop.cmd" })); + } + + /// + /// Verifies LF where output is split into one path per line. + /// + [Test] + public void ParseWhereOutput_WhenOutputUsesLf_ReturnsPaths() + { + string output = "C:\\tools\\uloop\nC:\\tools\\uloop.exe"; + + string[] result = NodeEnvironmentResolver.ParseWhereOutput(output); + + Assert.That(result, Is.EqualTo(new[] { @"C:\tools\uloop", @"C:\tools\uloop.exe" })); + } + + /// + /// Verifies that missing output, or output with only blank lines, yields no paths. + /// + [TestCase(null)] + [TestCase("")] + [TestCase(" \r\n\r\n ")] + public void ParseWhereOutput_WhenOutputHasNoPaths_ReturnsNull(string output) + { + string[] result = NodeEnvironmentResolver.ParseWhereOutput(output); + + Assert.That(result, Is.Null); + } + + /// + /// Verifies user names made of letters, digits, underscores, hyphens, and dots are accepted for the directory lookup. + /// + [TestCase("user_name-1.test")] + [TestCase("a")] + public void IsSafeDirectoryServiceUserName_WhenNameUsesAllowedCharacters_ReturnsTrue(string userName) + { + Assert.That(NodeEnvironmentResolver.IsSafeDirectoryServiceUserName(userName), Is.True); + } + + /// + /// Verifies empty names and names with characters that could change the lookup path are rejected. + /// + [TestCase(null)] + [TestCase("")] + [TestCase("user name")] + [TestCase("../user")] + [TestCase("user;id")] + public void IsSafeDirectoryServiceUserName_WhenNameIsEmptyOrUnsafe_ReturnsFalse(string userName) + { + Assert.That(NodeEnvironmentResolver.IsSafeDirectoryServiceUserName(userName), Is.False); + } } } diff --git a/Assets/Tests/Editor/PresentationTestDoubles.cs b/Assets/Tests/Editor/PresentationTestDoubles.cs new file mode 100644 index 0000000000..4f585b6b46 --- /dev/null +++ b/Assets/Tests/Editor/PresentationTestDoubles.cs @@ -0,0 +1,94 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Presentation; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Runs presentation background work on the calling thread so tests leave no thread-pool work behind. + /// + internal sealed class InlineBackgroundWorkRunner : IBackgroundWorkRunner + { + internal int RunCount { get; private set; } + + public Task RunAsync(Func work) + { + RunCount++; + return Task.FromResult(work()); + } + + public Task RunTaskAsync(Func> work) + { + RunCount++; + return work(); + } + } + + /// + /// Records presentation dialogs instead of opening modal editor windows. + /// + internal sealed class RecordingPresentationDialogs : IPresentationDialogs + { + internal List MessageTitles { get; } = new List(); + internal List Messages { get; } = new List(); + internal List ConfirmTitles { get; } = new List(); + internal List ConfirmMessages { get; } = new List(); + internal List ConfirmOkLabels { get; } = new List(); + internal List ConfirmCancelLabels { get; } = new List(); + internal bool ConfirmResult { get; set; } = true; + internal int SkillsInstalledCount { get; private set; } + internal bool CliUninstallConfirmResult { get; set; } = true; + internal int CliUninstallConfirmCount { get; private set; } + internal CliPathSetupFlowResult CliPathSetupResult { get; set; } + internal int CliPathSetupCount { get; private set; } + internal List CliPathSetupPlatforms { get; } = new List(); + internal List CliPathSetupServices { get; } = + new List(); + internal List CliPathSetupTokens { get; } = new List(); + + public void ShowMessage(string title, string message) + { + MessageTitles.Add(title); + Messages.Add(message); + } + + public bool Confirm(string title, string message, string ok, string cancel) + { + ConfirmTitles.Add(title); + ConfirmMessages.Add(message); + ConfirmOkLabels.Add(ok); + ConfirmCancelLabels.Add(cancel); + return ConfirmResult; + } + + public void ShowSkillsInstalled() + { + SkillsInstalledCount++; + } + + public bool ConfirmCliUninstall() + { + CliUninstallConfirmCount++; + return CliUninstallConfirmResult; + } + + public Task EnsureCliVisibleAndShowResultAsync( + RuntimePlatform platform, + CliSetupApplicationService cliSetupApplicationService, + CancellationToken ct) + { + CliPathSetupCount++; + CliPathSetupPlatforms.Add(platform); + CliPathSetupServices.Add(cliSetupApplicationService); + CliPathSetupTokens.Add(ct); + return Task.FromResult(CliPathSetupResult); + } + } +} diff --git a/Assets/Tests/Editor/PresentationTestDoubles.cs.meta b/Assets/Tests/Editor/PresentationTestDoubles.cs.meta new file mode 100644 index 0000000000..e92fd9bd77 --- /dev/null +++ b/Assets/Tests/Editor/PresentationTestDoubles.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 17a6161735f1b412480ba60ce7d009fc +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/RecordVideoSessionHostTests.cs b/Assets/Tests/Editor/RecordVideoSessionHostTests.cs new file mode 100644 index 0000000000..47362edcfa --- /dev/null +++ b/Assets/Tests/Editor/RecordVideoSessionHostTests.cs @@ -0,0 +1,422 @@ +using System; +using System.Collections.Generic; +using System.IO; +using NUnit.Framework; +using UnityEditor; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the recording session host starts, ticks, stops, and reacts to Play Mode, assembly reload, + /// and Editor quit without touching the Editor-resident recording or real Editor callbacks. + /// + public sealed class RecordVideoSessionHostTests + { + private LastCompletedRecording _previousRecording; + private string _outputDirectory; + private double _now; + private bool _clockThrows; + private List _encoderRequests; + private List _encoders; + private List _updateCallbacks; + private int _unsubscribeCount; + private int _retentionCount; + private RecordVideoSessionHost _host; + + [SetUp] + public void SetUp() + { + // The last-recording store is shared with the live recording, so the tests do not run while one is + // in progress. The test framework still runs TearDown when SetUp fails an assumption, and the fixture + // instance is reused across tests, so clear the previous test's host first; TearDown skips cleanup + // when no host was created. + _host = null; + _previousRecording = default; + Assume.That(RecordVideoService.IsRecording, Is.False); + _previousRecording = LastCompletedRecordingTestState.TakeAndClear(); + _outputDirectory = Path.Combine(Path.GetTempPath(), "uloop-record-video-host-" + Guid.NewGuid().ToString("N")); + _now = 0.0; + _clockThrows = false; + _encoderRequests = new List(); + _encoders = new List(); + _updateCallbacks = new List(); + _unsubscribeCount = 0; + _retentionCount = 0; + _host = new RecordVideoSessionHost( + CreateEncoder, + ReadClock, + callback => _updateCallbacks.Add(callback), + callback => + { + _unsubscribeCount++; + _updateCallbacks.Remove(callback); + }, + () => _retentionCount++); + } + + [TearDown] + public void TearDown() + { + if (_host == null) + { + return; + } + + // Why: the session owns a HideAndDontSave Texture2D that only Stop destroys. + _clockThrows = false; + _host.Stop("teardown"); + LastCompletedRecordingTestState.Restore(_previousRecording); + if (Directory.Exists(_outputDirectory)) + { + Directory.Delete(_outputDirectory, true); + } + } + + /// + /// What: Start creates the output directory and an MP4 encoder with the requested settings, clears the + /// last completed recording, and subscribes exactly one update callback. + /// + [Test] + public void Start_WithMp4Path_CreatesEncoderAndSubscribesUpdate() + { + LastCompletedRecordingStore.Save(CreateStoppedSnapshot()); + string outputPath = Path.Combine(_outputDirectory, "clip.mp4"); + + VideoRecordingSnapshot snapshot = StartRecording(outputPath, usedDefaultOutputPath: false, stopOnPlayModeExit: false); + + Assert.That(snapshot.IsRecording, Is.True); + Assert.That(snapshot.OutputPath, Is.EqualTo(outputPath)); + Assert.That(_host.IsRecording, Is.True); + Assert.That(Directory.Exists(_outputDirectory), Is.True); + Assert.That(_encoderRequests.Count, Is.EqualTo(1)); + Assert.That(_encoderRequests[0].OutputPath, Is.EqualTo(outputPath)); + Assert.That(_encoderRequests[0].Width, Is.EqualTo(4)); + Assert.That(_encoderRequests[0].Height, Is.EqualTo(2)); + Assert.That(_encoderRequests[0].FrameRate, Is.EqualTo(30)); + Assert.That(_encoderRequests[0].UseVp8, Is.False); + Assert.That(_encoderRequests[0].Quality, Is.EqualTo(RecordVideoQuality.high)); + Assert.That(_updateCallbacks.Count, Is.EqualTo(1)); + Assert.That(LastCompletedRecordingStore.TryRead().HasValue, Is.False); + } + + /// + /// What: a .webm output path selects the VP8 encoder regardless of extension casing. + /// + [Test] + public void Start_WithWebmPath_RequestsVp8Encoder() + { + StartRecording(Path.Combine(_outputDirectory, "clip.WEBM"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + + Assert.That(_encoderRequests[0].UseVp8, Is.True); + } + + /// + /// What: when the session cannot be created, Start disposes the encoder it created and stays idle. + /// + [Test] + public void Start_WhenSessionCreationThrows_DisposesEncoderAndStaysIdle() + { + _clockThrows = true; + + try + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + Assert.Fail("Start should rethrow the session creation failure."); + } + catch (InvalidOperationException e) + { + Assert.That(e.Message, Is.EqualTo("clock failed")); + } + + Assert.That(_encoders[0].DisposeCallCount, Is.EqualTo(1)); + Assert.That(_host.IsRecording, Is.False); + Assert.That(_updateCallbacks, Is.Empty); + } + + /// + /// What: an update tick encodes due frames while the session keeps recording. + /// + [Test] + public void EditorUpdate_WhileRecording_EncodesDueFrames() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + _now = 0.5; + + _updateCallbacks[0](); + + Assert.That(_host.GetSnapshot().EncodedFrameCount, Is.EqualTo(15)); + Assert.That(_host.IsRecording, Is.True); + Assert.That(_updateCallbacks.Count, Is.EqualTo(1)); + } + + /// + /// What: a tick past the maximum duration finishes the session, unsubscribes the update callback, + /// saves the last completed recording, and applies retention for the default output folder. + /// + [Test] + public void EditorUpdate_WhenMaxDurationReached_FinishesSessionAndAppliesRetention() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: true, stopOnPlayModeExit: false); + _now = 11.0; + + _updateCallbacks[0](); + + LastCompletedRecording lastRecording = LastCompletedRecordingStore.TryRead(); + Assert.That(_host.IsRecording, Is.False); + Assert.That(_updateCallbacks, Is.Empty); + Assert.That(_encoders[0].DisposeCallCount, Is.EqualTo(1)); + Assert.That(lastRecording.HasValue, Is.True); + Assert.That(lastRecording.Snapshot.StoppedBy, Is.EqualTo(RecordVideoConstants.StoppedByMaxDuration)); + Assert.That(_retentionCount, Is.EqualTo(1)); + Assert.That(_host.GetSnapshot().OutputPath, Is.Null); + } + + /// + /// What: an update tick without a session does nothing. + /// + [Test] + public void OnEditorUpdate_WithoutSession_DoesNothing() + { + _host.OnEditorUpdate(); + + Assert.That(_host.IsRecording, Is.False); + Assert.That(_unsubscribeCount, Is.EqualTo(0)); + } + + /// + /// What: a CLI stop returns the stopped snapshot, does not save it as the last completed recording, + /// and skips retention for a caller-chosen output path. + /// + [Test] + public void Stop_ByCli_ReturnsStoppedSnapshotWithoutSavingOrRetention() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + + VideoRecordingSnapshot snapshot = _host.Stop(RecordVideoConstants.StoppedByCli); + + Assert.That(snapshot.IsRecording, Is.False); + Assert.That(snapshot.StoppedBy, Is.EqualTo(RecordVideoConstants.StoppedByCli)); + Assert.That(_host.IsRecording, Is.False); + Assert.That(_updateCallbacks, Is.Empty); + Assert.That(LastCompletedRecordingStore.TryRead().HasValue, Is.False); + Assert.That(_retentionCount, Is.EqualTo(0)); + Assert.That(_host.GetSnapshot().OutputPath, Is.Null); + + // A stopped session must not linger: a later assembly reload would otherwise save it as a new recording. + _host.OnBeforeAssemblyReload(); + + Assert.That(LastCompletedRecordingStore.TryRead().HasValue, Is.False); + } + + /// + /// What: Stop and GetSnapshot return the default snapshot when nothing is recording. + /// + [Test] + public void StopAndGetSnapshot_WithoutSession_ReturnDefault() + { + Assert.That(_host.Stop(RecordVideoConstants.StoppedByCli).OutputPath, Is.Null); + Assert.That(_host.GetSnapshot().OutputPath, Is.Null); + } + + /// + /// What: leaving Play Mode stops a Game View recording and saves it as the last completed recording. + /// + [Test] + public void OnPlayModeStateChanged_ExitingPlayModeWithGameViewRecording_StopsAndSaves() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: true); + + _host.OnPlayModeStateChanged(PlayModeStateChange.ExitingPlayMode); + + Assert.That(_host.IsRecording, Is.False); + Assert.That(LastCompletedRecordingStore.TryRead().Snapshot.StoppedBy, Is.EqualTo(RecordVideoConstants.StoppedByPlayModeExit)); + } + + /// + /// What: a window recording keeps running when Play Mode ends, and other Play Mode transitions are ignored. + /// + [Test] + public void OnPlayModeStateChanged_WindowRecordingOrOtherTransition_KeepsRecording() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + + _host.OnPlayModeStateChanged(PlayModeStateChange.ExitingPlayMode); + _host.OnPlayModeStateChanged(PlayModeStateChange.EnteredEditMode); + + Assert.That(_host.IsRecording, Is.True); + } + + /// + /// What: a Game View recording ignores Play Mode transitions other than exiting Play Mode. + /// + [Test] + public void OnPlayModeStateChanged_GameViewRecordingEnteringPlayMode_KeepsRecording() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: true); + + _host.OnPlayModeStateChanged(PlayModeStateChange.EnteredPlayMode); + + Assert.That(_host.IsRecording, Is.True); + } + + /// + /// What: the lifecycle callbacks do nothing when no recording is active. + /// + [Test] + public void LifecycleCallbacks_WithoutSession_DoNothing() + { + _host.OnPlayModeStateChanged(PlayModeStateChange.ExitingPlayMode); + _host.OnBeforeAssemblyReload(); + _host.OnEditorQuitting(); + + Assert.That(_unsubscribeCount, Is.EqualTo(0)); + Assert.That(LastCompletedRecordingStore.TryRead().HasValue, Is.False); + } + + /// + /// What: an assembly reload stops the recording and saves it as the last completed recording. + /// + [Test] + public void OnBeforeAssemblyReload_WhileRecording_StopsAndSaves() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + + _host.OnBeforeAssemblyReload(); + + Assert.That(_host.IsRecording, Is.False); + Assert.That(LastCompletedRecordingStore.TryRead().Snapshot.StoppedBy, Is.EqualTo(RecordVideoConstants.StoppedByAssemblyReload)); + } + + /// + /// What: an Editor quit stops the recording without saving it, because SessionState does not survive a quit. + /// + [Test] + public void OnEditorQuitting_WhileRecording_StopsWithoutSaving() + { + StartRecording(Path.Combine(_outputDirectory, "clip.mp4"), usedDefaultOutputPath: false, stopOnPlayModeExit: false); + + _host.OnEditorQuitting(); + + Assert.That(_host.IsRecording, Is.False); + Assert.That(_encoders[0].DisposeCallCount, Is.EqualTo(1)); + Assert.That(LastCompletedRecordingStore.TryRead().HasValue, Is.False); + } + + private VideoRecordingSnapshot StartRecording(string outputPath, bool usedDefaultOutputPath, bool stopOnPlayModeExit) + { + return _host.Start( + frameRate: 30, + maxDurationSeconds: 10, + outputPath, + usedDefaultOutputPath, + width: 4, + height: 2, + new FakeGameViewFrameSource(), + stopOnPlayModeExit, + RecordVideoQuality.high); + } + + private IVideoFrameEncoder CreateEncoder( + string outputPath, + int width, + int height, + int frameRate, + bool useVp8, + RecordVideoQuality quality) + { + _encoderRequests.Add(new EncoderRequest(outputPath, width, height, frameRate, useVp8, quality)); + FakeVideoFrameEncoder encoder = new(width, height); + _encoders.Add(encoder); + return encoder; + } + + private double ReadClock() + { + if (_clockThrows) + { + throw new InvalidOperationException("clock failed"); + } + + return _now; + } + + private static VideoRecordingSnapshot CreateStoppedSnapshot() + { + return new VideoRecordingSnapshot( + "previous.mp4", + 4, + 2, + 30, + 1, + 0, + 1.0, + RecordVideoConstants.StoppedByMaxDuration, + false, + "high"); + } + + private readonly struct EncoderRequest + { + public EncoderRequest( + string outputPath, + int width, + int height, + int frameRate, + bool useVp8, + RecordVideoQuality quality) + { + OutputPath = outputPath; + Width = width; + Height = height; + FrameRate = frameRate; + UseVp8 = useVp8; + Quality = quality; + } + + public string OutputPath { get; } + public int Width { get; } + public int Height { get; } + public int FrameRate { get; } + public bool UseVp8 { get; } + public RecordVideoQuality Quality { get; } + } + + private sealed class FakeVideoFrameEncoder : IVideoFrameEncoder + { + public FakeVideoFrameEncoder(int width, int height) + { + Width = width; + Height = height; + } + + internal int DisposeCallCount { get; private set; } + + public int Width { get; } + + public int Height { get; } + + public bool AddFrame(Texture2D texture) + { + return true; + } + + public void Dispose() + { + DisposeCallCount++; + } + } + + private sealed class FakeGameViewFrameSource : IGameViewFrameSource + { + public bool IsSourceClosed => false; + + public bool TryReadFrame(Texture2D destination) + { + return true; + } + } + } +} diff --git a/Assets/Tests/Editor/RecordVideoSessionHostTests.cs.meta b/Assets/Tests/Editor/RecordVideoSessionHostTests.cs.meta new file mode 100644 index 0000000000..4a2d53de15 --- /dev/null +++ b/Assets/Tests/Editor/RecordVideoSessionHostTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 44464d31dab494cf5be31c8fafaad73b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/RecordVideoUseCaseLastRecordingTests.cs b/Assets/Tests/Editor/RecordVideoUseCaseLastRecordingTests.cs index 2697d01b3b..08787abc92 100644 --- a/Assets/Tests/Editor/RecordVideoUseCaseLastRecordingTests.cs +++ b/Assets/Tests/Editor/RecordVideoUseCaseLastRecordingTests.cs @@ -16,19 +16,28 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public sealed class RecordVideoUseCaseLastRecordingTests { private LastCompletedRecording _previousRecording; + private bool _storeSetAside; [SetUp] public void SetUp() { - // Stop and start act on a real recording, so the tests do not run while one is in progress. This comes - // before setting the store aside because TearDown does not run when SetUp fails an assumption. + // Stop and start act on a real recording, so the tests do not run while one is in progress. The test + // framework still runs TearDown when SetUp fails an assumption, so TearDown restores the store only + // when this SetUp actually set it aside; otherwise it would overwrite the live recording's store. + _storeSetAside = false; Assume.That(RecordVideoService.IsRecording, Is.False); _previousRecording = LastCompletedRecordingTestState.TakeAndClear(); + _storeSetAside = true; } [TearDown] public void TearDown() { + if (!_storeSetAside) + { + return; + } + LastCompletedRecordingTestState.Restore(_previousRecording); } diff --git a/Assets/Tests/Editor/RecordVideoUseCaseTests.cs b/Assets/Tests/Editor/RecordVideoUseCaseTests.cs index 49c9c095f5..67b327453c 100644 --- a/Assets/Tests/Editor/RecordVideoUseCaseTests.cs +++ b/Assets/Tests/Editor/RecordVideoUseCaseTests.cs @@ -12,19 +12,28 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public sealed class RecordVideoUseCaseTests { private LastCompletedRecording _previousRecording; + private bool _storeSetAside; [SetUp] public void SetUp() { - // Stop and start act on a real recording, so the tests do not run while one is in progress. This comes - // before setting the store aside because TearDown does not run when SetUp fails an assumption. + // Stop and start act on a real recording, so the tests do not run while one is in progress. The test + // framework still runs TearDown when SetUp fails an assumption, so TearDown restores the store only + // when this SetUp actually set it aside; otherwise it would overwrite the live recording's store. + _storeSetAside = false; Assume.That(RecordVideoService.IsRecording, Is.False); _previousRecording = LastCompletedRecordingTestState.TakeAndClear(); + _storeSetAside = true; } [TearDown] public void TearDown() { + if (!_storeSetAside) + { + return; + } + LastCompletedRecordingTestState.Restore(_previousRecording); } diff --git a/Assets/Tests/Editor/ScreenshotUseCaseTests.cs b/Assets/Tests/Editor/ScreenshotUseCaseTests.cs index ed0761878f..948603f8aa 100644 --- a/Assets/Tests/Editor/ScreenshotUseCaseTests.cs +++ b/Assets/Tests/Editor/ScreenshotUseCaseTests.cs @@ -15,7 +15,7 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public class ScreenshotUseCaseTests { [Test] - public void ExecuteAsync_WhenRaycastLayerMaskIsSetWithoutRaycastGrid_ShouldThrowValidationException() + public async Task ExecuteAsync_WhenRaycastLayerMaskIsSetWithoutRaycastGrid_ShouldThrowValidationException() { // Tests that setting RaycastLayerMask without AnnotateRaycastGrid fails validation. JObject parameters = new JObject @@ -23,15 +23,20 @@ public void ExecuteAsync_WhenRaycastLayerMaskIsSetWithoutRaycastGrid_ShouldThrow ["RaycastLayerMask"] = "Default" }; - UnityCliLoopToolParameterValidationException? exception = - Assert.ThrowsAsync( - async () => await ExecuteScreenshot(parameters)); - - Assert.That(exception!.Message, Does.Contain("RaycastLayerMask requires AnnotateRaycastGrid=true")); + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await UncanceledAwaits.AwaitValueAsync(ExecuteScreenshot(parameters)); + Assert.Fail("Expected UnityCliLoopToolParameterValidationException."); + } + catch (UnityCliLoopToolParameterValidationException exception) + { + Assert.That(exception.Message, Does.Contain("RaycastLayerMask requires AnnotateRaycastGrid=true")); + } } [Test] - public void ExecuteAsync_WhenElementsOnlyHasNoAnnotationMode_ShouldThrowValidationException() + public async Task ExecuteAsync_WhenElementsOnlyHasNoAnnotationMode_ShouldThrowValidationException() { // Tests that ElementsOnly without AnnotateElements or AnnotateRaycastGrid fails validation. JObject parameters = new JObject @@ -40,13 +45,18 @@ public void ExecuteAsync_WhenElementsOnlyHasNoAnnotationMode_ShouldThrowValidati ["ElementsOnly"] = true }; - UnityCliLoopToolParameterValidationException? exception = - Assert.ThrowsAsync( - async () => await ExecuteScreenshot(parameters)); - - Assert.That( - exception!.Message, - Does.Contain("ElementsOnly requires AnnotateElements=true or AnnotateRaycastGrid=true")); + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await UncanceledAwaits.AwaitValueAsync(ExecuteScreenshot(parameters)); + Assert.Fail("Expected UnityCliLoopToolParameterValidationException."); + } + catch (UnityCliLoopToolParameterValidationException exception) + { + Assert.That( + exception.Message, + Does.Contain("ElementsOnly requires AnnotateElements=true or AnnotateRaycastGrid=true")); + } } [Test] @@ -67,7 +77,7 @@ public async Task ExecuteAsync_WhenElementsOnlyUsesRaycastGrid_ShouldPassValidat } [Test] - public void ExecuteAsync_WhenRaycastLayerMaskContainsUnknownLayer_ShouldThrowValidationException() + public async Task ExecuteAsync_WhenRaycastLayerMaskContainsUnknownLayer_ShouldThrowValidationException() { // Tests that an unrecognized layer name in RaycastLayerMask fails validation with the layer name in the message. JObject parameters = new JObject @@ -77,12 +87,17 @@ public void ExecuteAsync_WhenRaycastLayerMaskContainsUnknownLayer_ShouldThrowVal ["RaycastLayerMask"] = "MissingLayerForTest" }; - UnityCliLoopToolParameterValidationException? exception = - Assert.ThrowsAsync( - async () => await ExecuteScreenshot(parameters)); - - Assert.That(exception!.Message, Does.Contain("unknown layer name")); - Assert.That(exception!.Message, Does.Contain("MissingLayerForTest")); + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await UncanceledAwaits.AwaitValueAsync(ExecuteScreenshot(parameters)); + Assert.Fail("Expected UnityCliLoopToolParameterValidationException."); + } + catch (UnityCliLoopToolParameterValidationException exception) + { + Assert.That(exception.Message, Does.Contain("unknown layer name")); + Assert.That(exception.Message, Does.Contain("MissingLayerForTest")); + } } /// @@ -102,27 +117,32 @@ public void ConvertToSchema_WhenCaptureModeIsOmitted_DefaultsToAuto() /// What: Edit Mode auto + annotate-elements still fails validation because auto resolves to window. /// [Test] - public void ExecuteAsync_WhenCaptureModeOmittedWithAnnotateElementsInEditMode_ShouldThrowValidationException() + public async Task ExecuteAsync_WhenCaptureModeOmittedWithAnnotateElementsInEditMode_ShouldThrowValidationException() { JObject parameters = new JObject { ["AnnotateElements"] = true }; - UnityCliLoopToolParameterValidationException? exception = - Assert.ThrowsAsync( - async () => await ExecuteScreenshot(parameters)); - - Assert.That( - exception!.Message, - Is.EqualTo("AnnotateElements is only supported when CaptureMode=rendering")); + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await UncanceledAwaits.AwaitValueAsync(ExecuteScreenshot(parameters)); + Assert.Fail("Expected UnityCliLoopToolParameterValidationException."); + } + catch (UnityCliLoopToolParameterValidationException exception) + { + Assert.That( + exception.Message, + Is.EqualTo("AnnotateElements is only supported when CaptureMode=rendering")); + } } /// /// What: explicit window + annotate-elements is still rejected while Play Mode is injected. /// [Test] - public void CaptureAsync_WhenWindowSpecifiedWithAnnotateElementsWhilePlaying_ShouldThrowValidationException() + public async Task CaptureAsync_WhenWindowSpecifiedWithAnnotateElementsWhilePlaying_ShouldThrowValidationException() { JObject parameters = new JObject { @@ -132,13 +152,18 @@ public void CaptureAsync_WhenWindowSpecifiedWithAnnotateElementsWhilePlaying_Sho ScreenshotSchema schema = DeserializeScreenshotSchema(parameters); ScreenshotUseCase useCase = new ScreenshotUseCase(new FakeScreenshotEditorStateReader(true)); - UnityCliLoopToolParameterValidationException? exception = - Assert.ThrowsAsync( - async () => await useCase.CaptureAsync(schema, CancellationToken.None)); - - Assert.That( - exception!.Message, - Is.EqualTo("AnnotateElements is only supported when CaptureMode=rendering")); + // Why not Assert.ThrowsAsync: it blocks the main thread synchronously in this NUnit version. + try + { + await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync(schema, CancellationToken.None)); + Assert.Fail("Expected UnityCliLoopToolParameterValidationException."); + } + catch (UnityCliLoopToolParameterValidationException exception) + { + Assert.That( + exception.Message, + Is.EqualTo("AnnotateElements is only supported when CaptureMode=rendering")); + } } /// diff --git a/Assets/Tests/Editor/ScreenshotUseCaseWindowCaptureTests.cs b/Assets/Tests/Editor/ScreenshotUseCaseWindowCaptureTests.cs new file mode 100644 index 0000000000..e342aef1ac --- /dev/null +++ b/Assets/Tests/Editor/ScreenshotUseCaseWindowCaptureTests.cs @@ -0,0 +1,291 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using UnityEditor; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the window-capture branch of ScreenshotUseCase with a fake capture service: + /// window lookup, Simulator fallback, per-window saving, and the failure paths. + /// + public sealed class ScreenshotUseCaseWindowCaptureTests + { + private string _outputDirectory = ""; + + [SetUp] + public void SetUp() + { + // An existing input-visualization overlay makes the use case wait on real Editor frames. + Assume.That(OverlayCanvasFactory.TryGetExisting(), Is.Null); + _outputDirectory = Path.Combine( + Path.GetTempPath(), + "uloop-screenshot-window-" + Guid.NewGuid().ToString("N")); + } + + [TearDown] + public void TearDown() + { + if (Directory.Exists(_outputDirectory)) + { + Directory.Delete(_outputDirectory, true); + } + } + + /// + /// What: a missing non-default window returns a not-found failure without capturing anything. + /// + [Test] + public async Task CaptureAsync_WhenWindowIsMissing_ReturnsNotFound() + { + FakeWindowCaptureService captureService = new(); + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest("Inspector"), + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Is.EqualTo("Window 'Inspector' not found (MatchMode: exact)")); + Assert.That(captureService.FindRequests, Is.EqualTo(new[] { "Inspector" })); + Assert.That(captureService.CaptureCount, Is.EqualTo(0)); + } + + /// + /// What: a missing Game window retries the Simulator window and reports both missing when neither exists. + /// + [Test] + public async Task CaptureAsync_WhenGameAndSimulatorAreMissing_ReturnsSimulatorAwareNotFound() + { + FakeWindowCaptureService captureService = new(); + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest(UnityCliLoopConstants.SCREENSHOT_DEFAULT_WINDOW_NAME), + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That( + response.Message, + Is.EqualTo("Neither Game nor Simulator window found; open the Game view or Device Simulator and retry")); + Assert.That( + captureService.FindRequests, + Is.EqualTo(new[] + { + UnityCliLoopConstants.SCREENSHOT_DEFAULT_WINDOW_NAME, + UnityCliLoopConstants.SCREENSHOT_SIMULATOR_WINDOW_NAME + })); + } + + /// + /// What: when only the Simulator window exists, the default Game request captures it under the Simulator name. + /// + [Test] + public async Task CaptureAsync_WhenOnlySimulatorExists_CapturesSimulatorWindow() + { + FakeWindowCaptureService captureService = new(); + captureService.WindowCounts[UnityCliLoopConstants.SCREENSHOT_SIMULATOR_WINDOW_NAME] = 1; + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest(UnityCliLoopConstants.SCREENSHOT_DEFAULT_WINDOW_NAME), + CancellationToken.None)); + + Assert.That(response.Screenshots.Count, Is.EqualTo(1)); + Assert.That(Path.GetFileName(response.Screenshots[0].ImagePath), Does.StartWith("Simulator_")); + } + + /// + /// What: a single captured window is saved as one PNG named after the window, with its size reported. + /// + [Test] + public async Task CaptureAsync_WhenOneWindowIsCaptured_SavesPngAndReportsIt() + { + FakeWindowCaptureService captureService = new(); + captureService.WindowCounts["Inspector"] = 1; + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest("Inspector"), + CancellationToken.None)); + + Assert.That(response.Screenshots.Count, Is.EqualTo(1)); + ScreenshotInfo info = response.Screenshots[0]; + Assert.That(File.Exists(info.ImagePath), Is.True); + Assert.That(Path.GetDirectoryName(info.ImagePath), Is.EqualTo(Path.GetFullPath(_outputDirectory))); + Assert.That(Path.GetFileName(info.ImagePath), Does.Match("^Inspector_[0-9]{8}_[0-9]{6}_[0-9]{3}\\.png$")); + Assert.That(info.Width, Is.EqualTo(4)); + Assert.That(info.Height, Is.EqualTo(2)); + Assert.That(info.FileSizeBytes, Is.GreaterThan(0)); + Assert.That(captureService.CapturedTextures[0] == null, Is.True); + } + + /// + /// What: several matching windows are saved with a 1-based index in each file name. + /// + [Test] + public async Task CaptureAsync_WhenSeveralWindowsMatch_SavesIndexedFiles() + { + FakeWindowCaptureService captureService = new(); + captureService.WindowCounts["Inspector"] = 2; + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest("Inspector"), + CancellationToken.None)); + + Assert.That(response.Screenshots.Count, Is.EqualTo(2)); + Assert.That(Path.GetFileName(response.Screenshots[0].ImagePath), Does.StartWith("Inspector_1_")); + Assert.That(Path.GetFileName(response.Screenshots[1].ImagePath), Does.StartWith("Inspector_2_")); + } + + /// + /// What: a window whose capture returns no texture is skipped while the remaining windows are still saved. + /// + [Test] + public async Task CaptureAsync_WhenOneCaptureReturnsNoTexture_SkipsThatWindow() + { + FakeWindowCaptureService captureService = new(); + captureService.WindowCounts["Inspector"] = 2; + captureService.NullTextureCaptureIndexes.Add(0); + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest("Inspector"), + CancellationToken.None)); + + Assert.That(response.Screenshots.Count, Is.EqualTo(1)); + Assert.That(Path.GetFileName(response.Screenshots[0].ImagePath), Does.StartWith("Inspector_2_")); + } + + /// + /// What: a capture timeout returns the timed-out failure with the screenshots saved before it. + /// + [Test] + public async Task CaptureAsync_WhenCaptureTimesOut_ReturnsTimedOutResult() + { + FakeWindowCaptureService captureService = new(); + captureService.WindowCounts["Inspector"] = 2; + captureService.TimedOutCaptureIndex = 1; + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest("Inspector"), + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Does.Contain("EditorWindow capture")); + Assert.That(response.Screenshots.Count, Is.EqualTo(1)); + } + + /// + /// What: a PNG that cannot be written is left out of the response and its texture is still destroyed. + /// + [Test] + public async Task CaptureAsync_WhenSavingFails_OmitsScreenshotAndDestroysTexture() + { + FakeWindowCaptureService captureService = new(); + captureService.WindowCounts["Inspector"] = 1; + // Why delete here: the use case creates the output directory before capturing, so removing it + // afterwards makes the PNG write throw without depending on file-system permissions. + captureService.BeforeReturningTexture = () => Directory.Delete(_outputDirectory, true); + ScreenshotUseCase useCase = CreateUseCase(captureService); + + ScreenshotResponse response = await UncanceledAwaits.AwaitValueAsync(useCase.CaptureAsync( + CreateRequest("Inspector"), + CancellationToken.None)); + + Assert.That(response.Screenshots, Is.Empty); + Assert.That(captureService.CapturedTextures[0] == null, Is.True); + } + + private ScreenshotUseCase CreateUseCase(FakeWindowCaptureService captureService) + { + return new ScreenshotUseCase(new EditModeStateReader(), captureService); + } + + private ScreenshotSchema CreateRequest(string windowName) + { + return new ScreenshotSchema + { + CaptureMode = CaptureMode.window, + WindowName = windowName, + MatchMode = WindowMatchMode.exact, + OutputDirectory = _outputDirectory + }; + } + + private sealed class EditModeStateReader : IScreenshotEditorStateReader + { + public bool IsPlaying => false; + public bool IsPaused => false; + } + + /// + /// Returns placeholder windows by name and small textures for each capture without touching real windows. + /// + private sealed class FakeWindowCaptureService : IEditorWindowCaptureService + { + public Dictionary WindowCounts { get; } = new(); + public List FindRequests { get; } = new(); + public HashSet NullTextureCaptureIndexes { get; } = new(); + public int TimedOutCaptureIndex { get; set; } = -1; + public Action? BeforeReturningTexture { get; set; } + public List CapturedTextures { get; } = new(); + public int CaptureCount { get; private set; } + + public EditorWindow[] FindWindowsByName(string windowName, WindowMatchMode matchMode) + { + FindRequests.Add(windowName); + int count = WindowCounts.TryGetValue(windowName, out int value) ? value : 0; + // Why null entries: the use case only forwards each window to CaptureWindowAsync. + return new EditorWindow[count]; + } + + public Task<(Texture2D? texture, bool timedOut)> CaptureWindowAsync( + EditorWindow window, + float resolutionScale, + int timeoutMilliseconds, + CancellationToken ct) + { + int captureIndex = CaptureCount; + CaptureCount++; + if (captureIndex == TimedOutCaptureIndex) + { + return Task.FromResult<(Texture2D?, bool)>((null, true)); + } + + if (NullTextureCaptureIndexes.Contains(captureIndex)) + { + return Task.FromResult<(Texture2D?, bool)>((null, false)); + } + + BeforeReturningTexture?.Invoke(); + Texture2D texture = new(4, 2, TextureFormat.RGBA32, false); + CapturedTextures.Add(texture); + return Task.FromResult<(Texture2D?, bool)>((texture, false)); + } + + public string[] GetOpenWindowNames() + { + return Array.Empty(); + } + + public Task<(Texture2D? texture, int yOffset, bool timedOut)> CaptureGameRenderingAsync( + float resolutionScale, + int timeoutMilliseconds, + CancellationToken ct) + { + throw new NotSupportedException("Window-capture tests never capture Game rendering."); + } + } + } +} diff --git a/Assets/Tests/Editor/ScreenshotUseCaseWindowCaptureTests.cs.meta b/Assets/Tests/Editor/ScreenshotUseCaseWindowCaptureTests.cs.meta new file mode 100644 index 0000000000..dd7843e8ca --- /dev/null +++ b/Assets/Tests/Editor/ScreenshotUseCaseWindowCaptureTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0bafce7cdd62a41ccb6f034e86481bf1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs b/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs index 3587e6e27d..e47fd69640 100644 --- a/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs +++ b/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs @@ -15,7 +15,8 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor { /// /// Verifies the setup wizard's version-change evaluation for already-seen package versions: - /// when it records the last-seen state, when it refreshes the CLI, and when it does nothing. + /// when it records the last-seen state, when it refreshes the CLI, and when it does nothing. Also covers the + /// migration auto-scan poll actions and the fallback full scan with recording ports. /// public sealed class SetupWizardStartupFlowVersionChangeTests { @@ -25,6 +26,12 @@ public sealed class SetupWizardStartupFlowVersionChangeTests private StubProjectSettingsPort _projectSettingsPort; private StubCliInstallationDetector _cliDetector; private int _showWindowCount; + private int _showAutoScanCount; + private RecordingSessionFlagsRepository _sessionFlagsRepository; + private RecordingAutoScanSeedRepository _autoScanSeedRepository; + private RecordingSkillSetupPort _skillSetupPort; + private RecordingMigrationPort _migrationPort; + private InlineBackgroundWorkRunner _backgroundWorkRunner; private SetupWizardStartupFlow _flow; [SetUp] @@ -34,6 +41,12 @@ public void SetUp() _projectSettingsPort = new StubProjectSettingsPort(); _cliDetector = new StubCliInstallationDetector(); _showWindowCount = 0; + _showAutoScanCount = 0; + _sessionFlagsRepository = new RecordingSessionFlagsRepository(); + _autoScanSeedRepository = new RecordingAutoScanSeedRepository(); + _skillSetupPort = new RecordingSkillSetupPort(); + _migrationPort = new RecordingMigrationPort(); + _backgroundWorkRunner = new InlineBackgroundWorkRunner(); CliSetupApplicationService cliSetupApplicationService = new CliSetupApplicationService( _cliDetector, new UnusedNativeCliInstaller(), @@ -41,13 +54,14 @@ public void SetUp() _flow = new SetupWizardStartupFlow( _editorSettingsPort, _projectSettingsPort, - new UnusedSessionFlagsRepository(), - new UnusedAutoScanSeedRepository(), + _sessionFlagsRepository, + _autoScanSeedRepository, cliSetupApplicationService, - new SkillSetupUseCase(new UnusedSkillSetupPort()), - new ThirdPartyToolMigrationUseCase(new UnusedMigrationPort()), + new SkillSetupUseCase(_skillSetupPort), + new ThirdPartyToolMigrationUseCase(_migrationPort), () => _showWindowCount++, - () => throw new InvalidOperationException("the migration auto-scan must not open")); + () => _showAutoScanCount++, + _backgroundWorkRunner); } /// @@ -160,6 +174,137 @@ public void TryShowOnVersionChange_WhenOnlyTheMinimumChangedAndNoCliIsInstalled_ Assert.That(_editorSettingsPort.UpdateCount, Is.EqualTo(1)); } + /// + /// Verifies a new package version with a current CLI scans the installed skills on the background runner, + /// at the project root and in the wizard's forced layout, and records the state when nothing is outdated. + /// + [Test] + public void TryShowOnVersionChange_WhenThePackageChangedAndTheSkillsAreCurrent_ScansSkillsAndRecords() + { + const string PreviousVersion = "3.0.0-previous.1"; + Assume.That(UnityCliLoopConstants.PackageInfo.version, Is.Not.EqualTo(PreviousVersion)); + _editorSettingsPort.Settings = new UnityCliLoopEditorSettingsData + { + lastSeenSetupWizardVersion = PreviousVersion, + lastSeenSetupWizardMinimumDispatcherVersion = MinimumDispatcherVersion + }; + _cliDetector.CliVersion = "3.1.0"; + _cliDetector.IsDispatcher = true; + + _flow.TryShowOnVersionChange(); + + Assert.That(_backgroundWorkRunner.RunCount, Is.EqualTo(1)); + Assert.That(_skillSetupPort.DetectProjectRoots, Is.EqualTo(new List { UnityCliLoopPathResolver.GetProjectRoot() })); + Assert.That(_skillSetupPort.DetectGroupFlags, Is.EqualTo(new List { !SetupWizardWindow.ForceFlatSkillInstall })); + Assert.That(_editorSettingsPort.UpdateCount, Is.EqualTo(1)); + Assert.That( + _editorSettingsPort.Settings.lastSeenSetupWizardVersion, + Is.EqualTo(UnityCliLoopConstants.PackageInfo.version)); + Assert.That(_showWindowCount, Is.EqualTo(0)); + } + + /// + /// Verifies a compile-error detection that finds legacy files stores them as seeds, flags the auto-scan for + /// this session, and opens the migration window once. + /// + [Test] + public void ApplyMigrationAutoScanPollAction_WhenDetectionFindsFiles_StoresSeedsAndOpensTheMigrationWindow() + { + _migrationPort.DetectionFound = true; + _migrationPort.DetectedFilePaths = new List { "/Project/Assets/A.cs", "/Project/Assets/B.cs" }; + + _flow.ApplyMigrationAutoScanPollAction(MigrationAutoScanPollAction.RunDetection); + + Assert.That( + _migrationPort.DetectionProjectRoots, + Is.EqualTo(new List { UnityCliLoopPathResolver.GetProjectRoot() })); + Assert.That(_autoScanSeedRepository.StoredSeedFilePaths.Count, Is.EqualTo(1)); + Assert.That( + _autoScanSeedRepository.StoredSeedFilePaths[0], + Is.EqualTo(new[] { "/Project/Assets/A.cs", "/Project/Assets/B.cs" })); + Assert.That(_sessionFlagsRepository.ShouldAutoScanValues, Is.EqualTo(new List { true })); + Assert.That(_showAutoScanCount, Is.EqualTo(1)); + } + + /// + /// Verifies a detection that finds nothing yet stores nothing and keeps the migration window closed. + /// + [Test] + public void ApplyMigrationAutoScanPollAction_WhenDetectionFindsNothing_KeepsTheWindowClosed() + { + _flow.ApplyMigrationAutoScanPollAction(MigrationAutoScanPollAction.RunDetection); + + Assert.That(_migrationPort.DetectionProjectRoots.Count, Is.EqualTo(1)); + Assert.That(_autoScanSeedRepository.StoredSeedFilePaths, Is.Empty); + Assert.That(_sessionFlagsRepository.ShouldAutoScanValues, Is.Empty); + Assert.That(_showAutoScanCount, Is.EqualTo(0)); + } + + /// + /// Verifies a throwing detection reaches the caller without opening the migration window. + /// + [Test] + public void ApplyMigrationAutoScanPollAction_WhenDetectionThrows_RethrowsWithoutOpeningTheWindow() + { + _migrationPort.DetectionFailure = new InvalidOperationException(""); + + InvalidOperationException exception = Assert.Throws( + () => _flow.ApplyMigrationAutoScanPollAction(MigrationAutoScanPollAction.RunDetection)); + + Assert.That(exception.Message, Is.EqualTo("")); + Assert.That(_showAutoScanCount, Is.EqualTo(0)); + } + + /// + /// Verifies waiting and terminating poll actions touch neither the migration port nor the window. + /// + [TestCase(MigrationAutoScanPollAction.ContinueWaiting)] + [TestCase(MigrationAutoScanPollAction.Terminate)] + public void ApplyMigrationAutoScanPollAction_WhenWaitingOrTerminating_DoesNotScan(MigrationAutoScanPollAction action) + { + _flow.ApplyMigrationAutoScanPollAction(action); + + Assert.That(_migrationPort.DetectionProjectRoots, Is.Empty); + Assert.That(_migrationPort.HasTargetsProjectRoots, Is.Empty); + Assert.That(_showAutoScanCount, Is.EqualTo(0)); + } + + /// + /// Verifies the timeout fallback runs a full scan at the project root and opens the migration window when it + /// finds targets. + /// + [Test] + public void ApplyMigrationAutoScanPollAction_WhenFallingBackAndTargetsExist_OpensTheMigrationWindow() + { + _migrationPort.HasTargets = true; + + _flow.ApplyMigrationAutoScanPollAction(MigrationAutoScanPollAction.FallBackToFullScan); + + Assert.That( + _migrationPort.HasTargetsProjectRoots, + Is.EqualTo(new List { UnityCliLoopPathResolver.GetProjectRoot() })); + Assert.That(_migrationPort.DetectionProjectRoots, Is.Empty); + Assert.That(_sessionFlagsRepository.ShouldAutoScanValues, Is.EqualTo(new List { true })); + Assert.That(_showAutoScanCount, Is.EqualTo(1)); + } + + /// + /// Verifies a fallback full scan without targets keeps the migration window closed. + /// + [Test] + public async Task RunThirdPartyToolMigrationFallbackFullScanAsync_WithoutTargets_KeepsTheWindowClosed() + { + await UncanceledAwaits.AwaitCompletionAsync( + _flow.RunThirdPartyToolMigrationFallbackFullScanAsync("")); + + Assert.That(_migrationPort.HasTargetsProjectRoots, Is.EqualTo(new List { "" })); + Assert.That( + _migrationPort.HasTargetsTokens, + Is.EqualTo(new List { CancellationToken.None })); + Assert.That(_sessionFlagsRepository.ShouldAutoScanValues, Is.Empty); + Assert.That(_showAutoScanCount, Is.EqualTo(0)); + } + private sealed class RecordingEditorSettingsPort : IUnityCliLoopEditorSettingsPort { internal UnityCliLoopEditorSettingsData Settings { get; set; } = new UnityCliLoopEditorSettingsData(); @@ -350,8 +495,15 @@ public NativeCliInstallCommandLoadResult GetGlobalCliInstallCommand( } } - private sealed class UnusedSessionFlagsRepository : ISessionFlagsRepository + private sealed class RecordingSessionFlagsRepository : ISessionFlagsRepository { + internal List ShouldAutoScanValues { get; } = new List(); + + public void SetShouldAutoScanThirdPartyToolMigration(bool shouldAutoScanThirdPartyToolMigration) + { + ShouldAutoScanValues.Add(shouldAutoScanThirdPartyToolMigration); + } + public bool GetIsServerRunning() => throw new NotSupportedException(); public bool GetIsServerManuallyStopped() => throw new NotSupportedException(); public bool GetIsAfterCompile() => throw new NotSupportedException(); @@ -362,7 +514,6 @@ private sealed class UnusedSessionFlagsRepository : ISessionFlagsRepository public void SetIsReconnecting(bool isReconnecting) => throw new NotSupportedException(); public void SetShowReconnectingUI(bool showReconnectingUI) => throw new NotSupportedException(); public void SetShowPostCompileReconnectingUI(bool showPostCompileReconnectingUI) => throw new NotSupportedException(); - public void SetShouldAutoScanThirdPartyToolMigration(bool shouldAutoScanThirdPartyToolMigration) => throw new NotSupportedException(); public bool ConsumeShouldAutoScanThirdPartyToolMigration() => throw new NotSupportedException(); public void MarkServerStarted() => throw new NotSupportedException(); public void MarkServerManuallyStopped() => throw new NotSupportedException(); @@ -374,21 +525,45 @@ private sealed class UnusedSessionFlagsRepository : ISessionFlagsRepository public void ClearDomainReloadRecoveryFlags() => throw new NotSupportedException(); } - private sealed class UnusedAutoScanSeedRepository : IThirdPartyToolMigrationAutoScanSeedRepository + private sealed class RecordingAutoScanSeedRepository : IThirdPartyToolMigrationAutoScanSeedRepository { - public void StoreSeedFilePaths(string[] filePaths) => throw new NotSupportedException(); + internal List StoredSeedFilePaths { get; } = new List(); + + public void StoreSeedFilePaths(string[] filePaths) + { + StoredSeedFilePaths.Add(filePaths); + } + public string[] GetSeedFilePaths() => throw new NotSupportedException(); public void ClearSeedFilePaths() => throw new NotSupportedException(); } - private sealed class UnusedSkillSetupPort : ISkillSetupPort + private sealed class RecordingSkillSetupPort : ISkillSetupPort { + internal List DetectProjectRoots { get; } = new List(); + internal List DetectGroupFlags { get; } = new List(); + public void RemoveSkillFiles(string toolName) => throw new NotSupportedException(); public bool IsSkillInstalled(string toolName) => throw new NotSupportedException(); public List DetectSkillTargetsForLayoutAtProjectRoot( string projectRoot, - bool groupSkillsUnderUnityCliLoop) => throw new NotSupportedException(); + bool groupSkillsUnderUnityCliLoop) + { + DetectProjectRoots.Add(projectRoot); + DetectGroupFlags.Add(groupSkillsUnderUnityCliLoop); + return new List + { + new SkillSetupTargetInfo( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: true, + hasDifferentLayoutSkills: false, + SkillInstallState.Installed) + }; + } public List DetectSkillTargetsForLayoutFastAtProjectRoot( string projectRoot, @@ -422,8 +597,35 @@ public Task RemoveV3MigrationSkillFilesAsync( CancellationToken ct) => throw new NotSupportedException(); } - private sealed class UnusedMigrationPort : IThirdPartyToolMigrationPort + private sealed class RecordingMigrationPort : IThirdPartyToolMigrationPort { + internal bool DetectionFound { get; set; } + internal List DetectedFilePaths { get; set; } = new List(); + internal Exception DetectionFailure { get; set; } + internal List DetectionProjectRoots { get; } = new List(); + internal bool HasTargets { get; set; } + internal List HasTargetsProjectRoots { get; } = new List(); + internal List HasTargetsTokens { get; } = new List(); + + public (bool Found, List TargetFilePaths) TryDetectAutoScanTargetsFromCompileErrors( + string projectRoot) + { + DetectionProjectRoots.Add(projectRoot); + if (DetectionFailure != null) + { + throw DetectionFailure; + } + + return (DetectionFound, DetectedFilePaths); + } + + public Task HasMigrationTargetsAsync(string projectRoot, CancellationToken ct) + { + HasTargetsProjectRoots.Add(projectRoot); + HasTargetsTokens.Add(ct); + return Task.FromResult(HasTargets); + } + public ThirdPartyToolMigrationPreview PreviewMigration(string projectRoot) => throw new NotSupportedException(); public Task PreviewMigrationAsync( @@ -431,11 +633,6 @@ public Task PreviewMigrationAsync( IProgress progress, CancellationToken ct) => throw new NotSupportedException(); - public (bool Found, List TargetFilePaths) TryDetectAutoScanTargetsFromCompileErrors( - string projectRoot) => throw new NotSupportedException(); - - public Task HasMigrationTargetsAsync(string projectRoot, CancellationToken ct) => - throw new NotSupportedException(); public ThirdPartyToolMigrationResult ApplyMigration(string projectRoot) => throw new NotSupportedException(); diff --git a/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs b/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs index c80d204ec1..598fb7fde7 100644 --- a/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs +++ b/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs @@ -26,10 +26,13 @@ public sealed class SetupWizardWorkflowControllersTests private RecordingEditorSettingsPort _editorSettingsPort; private StubCliInstallationDetector _cliDetector; private StubNativeCliInstaller _nativeCliInstaller; + private StubCliPinReader _pinReader; private RecordingSkillSetupPort _skillPort; private CliSetupApplicationService _cliSetupApplicationService; private int _resizeCount; private List _refreshUiCalls; + private RecordingPresentationDialogs _dialogs; + private InlineBackgroundWorkRunner _backgroundWorkRunner; [SetUp] public void SetUp() @@ -38,13 +41,16 @@ public void SetUp() _editorSettingsPort = new RecordingEditorSettingsPort(); _cliDetector = new StubCliInstallationDetector(); _nativeCliInstaller = new StubNativeCliInstaller(); + _pinReader = new StubCliPinReader(); _skillPort = new RecordingSkillSetupPort(); _cliSetupApplicationService = new CliSetupApplicationService( _cliDetector, _nativeCliInstaller, - new StubCliPinReader()); + _pinReader); _resizeCount = 0; _refreshUiCalls = new List(); + _dialogs = new RecordingPresentationDialogs(); + _backgroundWorkRunner = new InlineBackgroundWorkRunner(); } /// @@ -103,6 +109,213 @@ public async Task CliRefreshAndUpdate_WithAPackageOwnedInstallHiddenFromTheShell Is.EqualTo(isWindowsEditor ? "Installed" : "Fix PATH")); } + /// + /// Verifies a click on a package-manager-owned CLI that the shell can see re-checks the state with the + /// caller's token and only redraws, without installing or repairing PATH. + /// + [Test] + public async Task CliHandleInstall_WithAManagedCli_OnlyRefreshesTheUi() + { + _cliDetector.CliVersion = "3.1.0"; + _cliDetector.IsDispatcher = true; + _nativeCliInstaller.ManagedKind = ManagedCliKind.Homebrew; + // A loadable pin lets a wrongly reached install call the installer instead of stopping at the pin. + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + using CancellationTokenSource cts = new CancellationTokenSource(); + + await controller.HandleInstallCliAsync(cts.Token); + + Assert.That(_cliDetector.ForceRefreshTokens, Is.EqualTo(new List { cts.Token })); + Assert.That(_nativeCliInstaller.InstallCalls, Is.Empty); + Assert.That(_dialogs.CliPathSetupCount, Is.EqualTo(0)); + Assert.That(_dialogs.MessageTitles, Is.Empty); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { true })); + } + + /// + /// Verifies a package-manager-owned CLI that the shell cannot see gets the PATH repair, which writes no + /// binary, instead of an install. + /// + [Test] + public async Task CliHandleInstall_WithAManagedCliHiddenFromTheShell_RepairsPathWithoutInstalling() + { + AssumePathCheckRuns(); + _cliDetector.CliVersion = "3.1.0"; + _cliDetector.IsDispatcher = true; + _cliDetector.IsVisibleFromShell = false; + _nativeCliInstaller.HasPackageOwnedInstall = true; + _nativeCliInstaller.ManagedKind = ManagedCliKind.Homebrew; + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + using CancellationTokenSource cts = new CancellationTokenSource(); + + await controller.HandleInstallCliAsync(cts.Token); + + AssertPathSetupRanOnceWith(cts.Token); + Assert.That(_nativeCliInstaller.InstallCalls, Is.Empty); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { true })); + } + + /// + /// Verifies an up-to-date package-owned CLI that the shell cannot see gets the PATH repair, re-checks + /// the shell afterwards with the caller's token, and is not reinstalled. + /// + [Test] + public async Task CliHandleInstall_WithACurrentCliHiddenFromTheShell_RepairsPathWithoutInstalling() + { + AssumePathCheckRuns(); + _cliDetector.CliVersion = "3.1.0"; + _cliDetector.IsDispatcher = true; + _cliDetector.IsVisibleFromShell = false; + _nativeCliInstaller.HasPackageOwnedInstall = true; + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + using CancellationTokenSource cts = new CancellationTokenSource(); + + await controller.HandleInstallCliAsync(cts.Token); + + AssertPathSetupRanOnceWith(cts.Token); + Assert.That( + _cliDetector.ShellVisibilityTokens, + Is.EqualTo(new List { cts.Token, cts.Token })); + Assert.That(_nativeCliInstaller.InstallCalls, Is.Empty); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { true })); + } + + /// + /// Verifies a first install uses the pinned release with the caller's token, runs the PATH setup once, + /// hides the progress, and refreshes the UI including skills. + /// + [Test] + public async Task CliHandleInstall_WithoutACli_InstallsThenRunsThePathSetup() + { + _cliDetector.CliVersion = string.Empty; + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + using CancellationTokenSource cts = new CancellationTokenSource(); + + await controller.HandleInstallCliAsync(cts.Token); + + Assert.That(_nativeCliInstaller.InstallCalls, Is.EqualTo(new List { "dispatcher-v3.1.0|" })); + Assert.That(_nativeCliInstaller.InstallTokens, Is.EqualTo(new List { cts.Token })); + AssertPathSetupRanOnceWith(cts.Token); + Assert.That(_dialogs.MessageTitles, Is.Empty); + Assert.That(_root.Q("cli-install-progress").style.display.value, Is.EqualTo(DisplayStyle.None)); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { true })); + } + + /// + /// Verifies a first install into the package-owned location checks the shell again afterwards with the + /// caller's token. + /// + [Test] + public async Task CliHandleInstall_IntoAPackageOwnedLocation_RechecksTheShellAfterInstalling() + { + AssumePathCheckRuns(); + _cliDetector.CliVersion = string.Empty; + _nativeCliInstaller.HasPackageOwnedInstall = true; + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + using CancellationTokenSource cts = new CancellationTokenSource(); + + await controller.HandleInstallCliAsync(cts.Token); + + Assert.That(_nativeCliInstaller.InstallCalls.Count, Is.EqualTo(1)); + Assert.That( + _cliDetector.ShellVisibilityTokens, + Is.EqualTo(new List { cts.Token, cts.Token })); + } + + /// + /// Verifies an update over an installed CLI refreshes the UI without the skills. + /// + [Test] + public async Task CliHandleInstall_OverAnInstalledCli_RefreshesWithoutTheSkills() + { + _cliDetector.CliVersion = "2.0.0"; + _cliDetector.IsDispatcher = true; + _cliDetector.IsCliInstalledValue = true; + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + + await controller.HandleInstallCliAsync(CancellationToken.None); + + Assert.That(_nativeCliInstaller.InstallCalls.Count, Is.EqualTo(1)); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { false })); + } + + /// + /// Verifies a failed install shows the installer error with the manual install command, skips the PATH + /// setup, and still hides the progress and refreshes the UI. + /// + [Test] + public async Task CliHandleInstall_WhenTheInstallFails_ShowsTheErrorWithTheManualCommand() + { + _cliDetector.CliVersion = string.Empty; + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + _nativeCliInstaller.InstallResult = new CliInstallResult(false, ""); + _nativeCliInstaller.InstallCommandResult = NativeCliInstallCommandLoadResult.FromSuccess( + new NativeCliInstallCommand("", "", "")); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + + await controller.HandleInstallCliAsync(CancellationToken.None); + + Assert.That(_dialogs.MessageTitles, Is.EqualTo(new List { "Installation Failed" })); + Assert.That( + _dialogs.Messages, + Is.EqualTo(new List { "Failed to install uloop CLI.\n\n\n\n" })); + Assert.That(_dialogs.CliPathSetupCount, Is.EqualTo(0)); + Assert.That(_root.Q("cli-install-progress").style.display.value, Is.EqualTo(DisplayStyle.None)); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { true })); + } + + /// + /// Verifies a failed install whose manual command cannot be built shows the command error instead. + /// + [Test] + public async Task CliHandleInstall_WhenTheManualCommandIsUnavailable_ShowsTheCommandError() + { + _cliDetector.CliVersion = string.Empty; + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + _nativeCliInstaller.InstallResult = new CliInstallResult(false, ""); + _nativeCliInstaller.InstallCommandResult = NativeCliInstallCommandLoadResult.FromFailure(""); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + + await controller.HandleInstallCliAsync(CancellationToken.None); + + Assert.That( + _dialogs.Messages, + Is.EqualTo(new List { "Failed to install uloop CLI.\n\n\n\n" })); + } + + /// + /// Verifies an install that throws still hides the progress, clears the installing state, and refreshes + /// the UI before the exception reaches the caller. + /// + [Test] + public async Task CliHandleInstall_WhenTheInstallThrows_ClearsTheInstallingStateAndRethrows() + { + _cliDetector.CliVersion = string.Empty; + _pinReader.BootstrapPin = DispatcherBootstrapPinLoadResult.FromSuccess("dispatcher-v3.1.0", ""); + _nativeCliInstaller.InstallFailure = new InvalidOperationException(""); + SetupWizardCliWorkflowController controller = CreateCliWorkflow(); + + try + { + await controller.HandleInstallCliAsync(CancellationToken.None); + Assert.Fail("The install failure should reach the caller."); + } + catch (InvalidOperationException exception) + { + Assert.That(exception.Message, Is.EqualTo("")); + } + + Assert.That(_dialogs.CliPathSetupCount, Is.EqualTo(0)); + Assert.That(_root.Q("cli-install-progress").style.display.value, Is.EqualTo(DisplayStyle.None)); + Assert.That(_refreshUiCalls, Is.EqualTo(new List { true })); + await controller.RefreshAndUpdateAsync(CancellationToken.None); + Assert.That(_root.Q public sealed class UnityCliLoopSettingsSkillsPresenterStateTests { + private const string CliNotFoundMessage = "uloop CLI is not installed. Please install the CLI first."; + private const string InstallFailureMessage = "install failed in this test"; + private RecordingSkillSetupPort _skillPort; private StubCliInstallationDetector _cliDetector; private RecordingEditorSettingsPort _editorSettingsPort; private List _sectionRefreshCalls; private bool _isRefreshingVersion; + private RecordingPresentationDialogs _dialogs; + private InlineBackgroundWorkRunner _backgroundWorkRunner; private UnityCliLoopSettingsSkillsPresenter _presenter; [SetUp] @@ -37,10 +42,14 @@ public void SetUp() _editorSettingsPort = new RecordingEditorSettingsPort(); _sectionRefreshCalls = new List(); _isRefreshingVersion = false; + _dialogs = new RecordingPresentationDialogs(); + _backgroundWorkRunner = new InlineBackgroundWorkRunner(); _presenter = new UnityCliLoopSettingsSkillsPresenter( new SkillSetupUseCase(_skillPort), new CliSetupApplicationService(_cliDetector, new UnusedNativeCliInstaller(), new UnusedCliPinReader()), - _editorSettingsPort); + _editorSettingsPort, + _dialogs, + _backgroundWorkRunner); _presenter.BindCoordination( includeSkillDirectoryChecks => _sectionRefreshCalls.Add(includeSkillDirectoryChecks), () => _isRefreshingVersion); @@ -265,6 +274,311 @@ public async Task ApplyToolToggleSideEffects_WhenTheSkillIsStillMissing_WarnsAbo Assert.That(_skillPort.InstalledToolSkills.Count, Is.EqualTo(1)); } + /// + /// Verifies a background refresh with a CLI applies the full scan run through the background runner. + /// + [Test] + public void RefreshSelectedTargetInstallStateInBackground_WithACli_AppliesTheFullScan() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Outdated, hasSkillsDirectory: true), + CreateTarget(".agents", SkillInstallState.Missing, hasSkillsDirectory: false) + }; + _presenter.MarkSelectedTargetInstallStateChecking(); + + _presenter.RefreshSelectedTargetInstallStateInBackground(); + + UnityCliLoopSettingsSkillsSnapshot snapshot = _presenter.GetSnapshot(); + Assert.That(snapshot.SelectedTargetInstallState, Is.EqualTo(SkillInstallState.Outdated)); + Assert.That( + snapshot.InstallableSkillTargets.Select(target => target.DirName).ToArray(), + Is.EqualTo(new[] { ".claude" })); + Assert.That(snapshot.HasSkillTargetScanResult, Is.True); + Assert.That(_skillPort.FullScanCount, Is.EqualTo(1)); + Assert.That(_backgroundWorkRunner.RunCount, Is.EqualTo(1)); + Assert.That(_sectionRefreshCalls, Is.EqualTo(new List { true })); + } + + /// + /// Verifies a background refresh cancelled while scanning leaves the previous state untouched. + /// + [Test] + public void RefreshSelectedTargetInstallStateInBackground_WhenCancelledDuringTheScan_KeepsThePreviousState() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Installed, hasSkillsDirectory: true) + }; + _skillPort.OnFullScan = () => _presenter.CancelSkillInstallStateRefresh(); + _presenter.MarkSelectedTargetInstallStateChecking(); + + _presenter.RefreshSelectedTargetInstallStateInBackground(); + + UnityCliLoopSettingsSkillsSnapshot snapshot = _presenter.GetSnapshot(); + Assert.That(snapshot.SelectedTargetInstallState, Is.EqualTo(SkillInstallState.Checking)); + Assert.That(snapshot.HasSkillTargetScanResult, Is.False); + Assert.That(_sectionRefreshCalls, Is.Empty); + } + + /// + /// Verifies installing the selected skills without a CLI shows the missing-CLI message and installs nothing. + /// + [Test] + public async Task HandleInstallSkills_WithoutACli_ShowsCliNotFound() + { + _cliDetector.IsCliInstalledValue = false; + + await _presenter.HandleInstallSkills(); + + Assert.That(_dialogs.MessageTitles, Is.EqualTo(new List { "CLI Not Found" })); + Assert.That(_dialogs.Messages, Is.EqualTo(new List { CliNotFoundMessage })); + Assert.That(_skillPort.InstalledTargetDirs, Is.Empty); + Assert.That(_sectionRefreshCalls, Is.Empty); + } + + /// + /// Verifies installing the selected skills installs the flat Claude target while marked as installing, + /// shows the installed dialog, and refreshes the install state afterwards. + /// + [Test] + public async Task HandleInstallSkills_WithACli_InstallsTheSelectedTargetAndShowsTheInstalledDialog() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true) + }; + bool installingDuringInstall = false; + _skillPort.OnInstall = () => installingDuringInstall = _presenter.GetSnapshot().IsInstallingSkills; + + await _presenter.HandleInstallSkills(); + + Assert.That(_skillPort.InstalledTargetDirs, Is.EqualTo(new List { ".claude" })); + Assert.That(_skillPort.InstallGroupFlags, Is.EqualTo(new List { false })); + Assert.That(installingDuringInstall, Is.True); + Assert.That(_presenter.GetSnapshot().IsInstallingSkills, Is.False); + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(1)); + Assert.That(_skillPort.FastScanGroupFlags.Count, Is.EqualTo(1)); + Assert.That(_skillPort.FullScanCount, Is.EqualTo(2)); + Assert.That(_presenter.GetSnapshot().HasSkillTargetScanResult, Is.True); + } + + /// + /// Verifies a failed selected install propagates the error without the installed dialog, clears the + /// installing flag, and still refreshes the install state. + /// + [Test] + public async Task HandleInstallSkills_WhenTheInstallFails_ClearsTheInstallingFlagAndRefreshes() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true) + }; + _skillPort.InstallFailure = new InvalidOperationException(InstallFailureMessage); + + // Awaited in try / catch instead of Assert.ThrowsAsync, which blocks the main thread in this NUnit. + try + { + await _presenter.HandleInstallSkills(); + Assert.Fail("Expected the install failure to propagate."); + } + catch (InvalidOperationException exception) + { + Assert.That(exception.Message, Is.EqualTo(InstallFailureMessage)); + } + + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(0)); + Assert.That(_presenter.GetSnapshot().IsInstallingSkills, Is.False); + Assert.That(_skillPort.FastScanGroupFlags.Count, Is.EqualTo(1)); + Assert.That(_skillPort.FullScanCount, Is.EqualTo(2)); + } + + /// + /// Verifies updating an outdated selected target installs it without the installed dialog. + /// + [Test] + public async Task HandleInstallSkills_WhenTheSelectedTargetIsOutdated_SkipsTheInstalledDialog() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Outdated, hasSkillsDirectory: true) + }; + + await _presenter.HandleInstallSkills(); + + Assert.That(_skillPort.InstalledTargetDirs, Is.EqualTo(new List { ".claude" })); + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(0)); + } + + /// + /// Verifies installing all skills without a CLI shows the missing-CLI message and installs nothing. + /// + [Test] + public async Task HandleInstallAllSkills_WithoutACli_ShowsCliNotFound() + { + _cliDetector.IsCliInstalledValue = false; + + await _presenter.HandleInstallAllSkills(CancellationToken.None); + + Assert.That(_dialogs.MessageTitles, Is.EqualTo(new List { "CLI Not Found" })); + Assert.That(_dialogs.Messages, Is.EqualTo(new List { CliNotFoundMessage })); + Assert.That(_skillPort.FullScanCount, Is.EqualTo(0)); + Assert.That(_sectionRefreshCalls, Is.Empty); + } + + /// + /// Verifies installing all skills installs every target with a skills directory with the caller's token + /// and shows the installed dialog. + /// + [Test] + public async Task HandleInstallAllSkills_WithInstallableTargets_InstallsThemAndShowsTheInstalledDialog() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true), + CreateTarget(".codex", SkillInstallState.Installed, hasSkillsDirectory: true), + CreateTarget(".agents", SkillInstallState.Missing, hasSkillsDirectory: false) + }; + bool installingDuringInstall = false; + _skillPort.OnInstall = () => installingDuringInstall = _presenter.GetSnapshot().IsInstallingSkills; + using CancellationTokenSource cancellation = new CancellationTokenSource(); + + await _presenter.HandleInstallAllSkills(cancellation.Token); + + Assert.That(_skillPort.InstalledTargetDirs, Is.EqualTo(new List { ".claude", ".codex" })); + Assert.That(_skillPort.InstallTokens, Is.EqualTo(new[] { cancellation.Token })); + Assert.That(installingDuringInstall, Is.True); + Assert.That(_presenter.GetSnapshot().IsInstallingSkills, Is.False); + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(1)); + } + + /// + /// Verifies a failed install-all propagates the error without the installed dialog, clears the installing + /// flag, and still refreshes the install state. + /// + [Test] + public async Task HandleInstallAllSkills_WhenTheInstallFails_ClearsTheInstallingFlagAndRefreshes() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true) + }; + _skillPort.InstallFailure = new InvalidOperationException(InstallFailureMessage); + + // Awaited in try / catch instead of Assert.ThrowsAsync, which blocks the main thread in this NUnit. + try + { + await _presenter.HandleInstallAllSkills(CancellationToken.None); + Assert.Fail("Expected the install failure to propagate."); + } + catch (InvalidOperationException exception) + { + Assert.That(exception.Message, Is.EqualTo(InstallFailureMessage)); + } + + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(0)); + Assert.That(_presenter.GetSnapshot().IsInstallingSkills, Is.False); + Assert.That(_skillPort.FastScanGroupFlags.Count, Is.EqualTo(1)); + Assert.That(_skillPort.FullScanCount, Is.EqualTo(2)); + } + + /// + /// Verifies installing all skills with an outdated target skips the installed dialog. + /// + [Test] + public async Task HandleInstallAllSkills_WhenATargetIsOutdated_SkipsTheInstalledDialog() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true), + CreateTarget(".codex", SkillInstallState.Outdated, hasSkillsDirectory: true) + }; + + await _presenter.HandleInstallAllSkills(CancellationToken.None); + + Assert.That(_skillPort.InstalledTargetDirs.Count, Is.EqualTo(2)); + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(0)); + } + + /// + /// Verifies installing all skills with no target that has a skills directory installs nothing and clears the installing flag. + /// + [Test] + public async Task HandleInstallAllSkills_WithoutInstallableTargets_InstallsNothing() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".agents", SkillInstallState.Missing, hasSkillsDirectory: false) + }; + + await _presenter.HandleInstallAllSkills(CancellationToken.None); + + Assert.That(_skillPort.InstalledTargetDirs, Is.Empty); + Assert.That(_dialogs.SkillsInstalledCount, Is.EqualTo(0)); + Assert.That(_presenter.GetSnapshot().IsInstallingSkills, Is.False); + } + + /// + /// Verifies installing all skills cancelled while detecting targets installs nothing. + /// + [Test] + public async Task HandleInstallAllSkills_WhenCancelledDuringDetection_InstallsNothing() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true) + }; + using CancellationTokenSource cancellation = new CancellationTokenSource(); + _skillPort.OnFullScan = () => cancellation.Cancel(); + + await _presenter.HandleInstallAllSkills(cancellation.Token); + + Assert.That(_skillPort.InstalledTargetDirs, Is.Empty); + Assert.That(_presenter.GetSnapshot().IsInstallingSkills, Is.False); + } + + /// + /// Verifies a second install-all request during an install returns without starting another install. + /// + [Test] + public async Task HandleInstallAllSkills_WhileInstalling_DoesNotStartASecondInstall() + { + _cliDetector.IsCliInstalledValue = true; + _skillPort.FullTargets = new List + { + CreateTarget(".claude", SkillInstallState.Missing, hasSkillsDirectory: true) + }; + Task nestedInstall = null; + bool nestedRequested = false; + // Re-enters only once so a missing latch fails the test instead of recursing without end. + _skillPort.OnInstall = () => + { + if (nestedRequested) + { + return; + } + + nestedRequested = true; + nestedInstall = _presenter.HandleInstallAllSkills(CancellationToken.None); + }; + + await _presenter.HandleInstallAllSkills(CancellationToken.None); + + Assert.That(nestedInstall.IsCompleted, Is.True); + Assert.That(_skillPort.InstalledTargetDirs, Is.EqualTo(new List { ".claude" })); + Assert.That(_skillPort.FullScanCount, Is.EqualTo(2)); + } + private static SkillSetupTargetInfo CreateTarget(string dirName, SkillInstallState installState, bool hasSkillsDirectory) { return new SkillSetupTargetInfo( @@ -283,6 +597,13 @@ private sealed class RecordingSkillSetupPort : ISkillSetupPort internal List FastScanProjectRoots { get; } = new List(); internal List FastScanGroupFlags { get; } = new List(); internal int FullScanCount { get; private set; } + internal List FullTargets { get; set; } = new List(); + internal Action OnFullScan { get; set; } + internal List InstalledTargetDirs { get; } = new List(); + internal List InstallGroupFlags { get; } = new List(); + internal Action OnInstall { get; set; } + internal List InstallTokens { get; } = new List(); + internal Exception InstallFailure { get; set; } internal HashSet InstalledToolNames { get; } = new HashSet(); internal List RemovedTools { get; } = new List(); internal List InstalledToolSkills { get; } = new List(); @@ -301,7 +622,8 @@ public List DetectSkillTargetsForLayoutAtProjectRoot( bool groupSkillsUnderUnityCliLoop) { FullScanCount++; - return new List(); + OnFullScan?.Invoke(); + return FullTargets; } public void RemoveSkillFiles(string toolName) @@ -326,7 +648,23 @@ public Task InstallSkillFilesForToolAsync( public Task InstallSkillFilesAsync( List targets, bool groupSkillsUnderUnityCliLoop, - CancellationToken ct) => throw new NotSupportedException(); + CancellationToken ct) + { + if (InstallFailure != null) + { + return Task.FromException(InstallFailure); + } + + foreach (SkillSetupTargetInfo target in targets) + { + InstalledTargetDirs.Add(target.DirName); + } + + InstallGroupFlags.Add(groupSkillsUnderUnityCliLoop); + InstallTokens.Add(ct); + OnInstall?.Invoke(); + return Task.CompletedTask; + } public SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( string projectRoot, diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpLegacyAssemblyMigrationContext.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpLegacyAssemblyMigrationContext.cs index 683b8ead11..7e3b4dddf9 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpLegacyAssemblyMigrationContext.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpLegacyAssemblyMigrationContext.cs @@ -374,11 +374,14 @@ private void RemovePlayerLoopTimingParameters() ApplyReplacementResult(migratedContent, replacementCount); _removedPlayerLoopTimingSignatures.AddRange(localRemovedTimingSignatures); + // This pass sees one file, so it cannot follow inheritance; the cross-file pass revisits every file + // with the project's type hierarchy and rewrites the inherited calls left here. (string timingCallerMigratedContent, int timingCallerReplacementCount) = RemoveLegacyPlayerLoopTimingCallerArgumentsInCode( _migratedContent, localRemovedTimingSignatures, - _legacyNamespaceAliases); + _legacyNamespaceAliases, + ThirdPartyToolMigrationTypeHierarchyIndex.Empty); ApplyReplacementResult(timingCallerMigratedContent, timingCallerReplacementCount); migratedCalleeMethodNames = localRemovedTimingSignatures .Select(signature => signature.MethodName) diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationDeclarationNameRules.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationDeclarationNameRules.cs new file mode 100644 index 0000000000..85f51ed93d --- /dev/null +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationDeclarationNameRules.cs @@ -0,0 +1,349 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; + +using CodeTextMask = io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationParsingRules.CodeTextMask; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationParsingRules; + +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Decides whether an identifier in a class body declares a name there or uses one, for the type hierarchy index. + /// + public static class ThirdPartyToolMigrationDeclarationNameRules + { + // Words after which an identifier is in an expression, so it is a use of a name rather than a declaration. + // Type keywords and modifiers (var, void, int, static, override, ...) are left out on purpose: a name after + // them is declared, and an unknown word must count as a declaration so the walk stays on the unchanged side. + private static readonly HashSet ExpressionKeywords = new(StringComparer.Ordinal) + { + "return", "await", "new", "else", "throw", "yield", "in", "is", "as", "case", "when", + "out", "ref", "typeof", "sizeof", "nameof", "default", "not", "and", "or", "goto", "using", "lock", "fixed", + "checked", "unchecked", "this", "base", "select", "where", "orderby", "by", "on", "equals", "ascending", + "descending", + }; + + /// + /// Decides whether the identifier at the index is declared there. Only positions that are certainly + /// expressions count as uses; anything unclear counts as a declaration so an inherited call is left unchanged. + /// + public static bool IsDeclarationName( + string source, + CodeTextMask codeTextMask, + int identifierStartIndex, + int parenDepth) + { + Debug.Assert(source != null, "source must not be null"); + Debug.Assert(identifierStartIndex >= 0, "identifierStartIndex must not be negative"); + + int previousIndex = ReadPreviousCodeIndex(source, codeTextMask, identifierStartIndex - 1); + if (previousIndex < 0) + { + return true; + } + + char previous = source[previousIndex]; + if (previous == '.') + { + // x.Run or an explicit interface implementation IFoo.Run: not a name this type can call unqualified. + return false; + } + + // After the '.' check: a lambda parameter never follows '.', and "int IFoo.Run => 0;" is not a lambda. + if (IsLambdaParameterName(source, codeTextMask, identifierStartIndex) || + IsDesignationName(source, codeTextMask, identifierStartIndex, previousIndex)) + { + return true; + } + + if (IsIdentifierCharacter(previous)) + { + return !ExpressionKeywords.Contains(ReadIdentifierEndingAt(source, codeTextMask, previousIndex)); + } + + return IsDeclarationAfterPunctuation(source, codeTextMask, previousIndex, parenDepth); + } + + private static bool IsDeclarationAfterPunctuation( + string source, + CodeTextMask codeTextMask, + int previousIndex, + int parenDepth) + { + char previous = source[previousIndex]; + switch (previous) + { + case '>': + // "=> Run" is an expression; "Task Run" is a type. A comparison cannot be told apart. + return !(previousIndex > 0 && source[previousIndex - 1] == '='); + case ']': + return true; + case '?': + // "int? Run" and "(int, int)? Run" are types; the conditional "x ? Run" has a space before '?'. + return previousIndex > 0 && IsNullableTypeSuffixOwner(source[previousIndex - 1]); + case ')': + return IsTupleTypeClose(source, codeTextMask, previousIndex); + case ',': + // Outside brackets a comma separates declarators; inside them it separates arguments. + return parenDepth == 0; + default: + return !IsExpressionPunctuation(previous); + } + } + + // "Run => ..", or "(Run, x) => .." where the name opens or continues a parenthesized parameter list. + // Foo(Run, x); is still a use: its ')' is not followed by "=>". + private static bool IsLambdaParameterName(string source, CodeTextMask codeTextMask, int identifierStartIndex) + { + int identifierEndIndex = ReadIdentifierEndIndex(source, identifierStartIndex); + if (IsArrowAt(source, codeTextMask, ReadNextCodeIndex(source, codeTextMask, identifierEndIndex))) + { + return true; + } + + int previousIndex = ReadPreviousCodeIndex(source, codeTextMask, identifierStartIndex - 1); + if (previousIndex < 0 || (source[previousIndex] != '(' && source[previousIndex] != ',')) + { + return false; + } + + int closeIndex = FindListCloseParenthesisIndex(source, codeTextMask, identifierEndIndex); + return closeIndex >= 0 && + IsArrowAt(source, codeTextMask, ReadNextCodeIndex(source, codeTextMask, closeIndex + 1)); + } + + // A name introduced after punctuation by a pattern or a deconstruction: "var (Run, n)" (also in foreach and + // "is var"), and the designation after a property or positional pattern ("is { } Run", "is Holder(1) Run"). + private static bool IsDesignationName( + string source, + CodeTextMask codeTextMask, + int identifierStartIndex, + int previousIndex) + { + char previous = source[previousIndex]; + if (previous == '(' || previous == ',') + { + return IsVarDeconstructionElement(source, codeTextMask, previousIndex); + } + + if (previous == '}' || previous == ')') + { + return !IsFollowedByCall(source, codeTextMask, ReadIdentifierEndIndex(source, identifierStartIndex)); + } + + return false; + } + + // The list the name is in opens with a '(' right after "var". A nested "var ((a, b), c)" is not recognized. + private static bool IsVarDeconstructionElement(string source, CodeTextMask codeTextMask, int previousIndex) + { + int openIndex = source[previousIndex] == '(' + ? previousIndex + : FindListOpenParenthesisIndex(source, codeTextMask, previousIndex - 1); + if (openIndex < 0) + { + return false; + } + + int beforeOpenIndex = ReadPreviousCodeIndex(source, codeTextMask, openIndex - 1); + return beforeOpenIndex >= 0 && + IsIdentifierCharacter(source[beforeOpenIndex]) && + ReadIdentifierEndingAt(source, codeTextMask, beforeOpenIndex) == "var"; + } + + // After '}' or ')' a name is a designation unless it is called: "} Run(..)" and "} Run(..)" start a + // statement. Inverting the check covers designations nested in patterns ("{ Callback: { } Run }") and + // designations followed by an operator without listing every token that may follow one. + private static bool IsFollowedByCall(string source, CodeTextMask codeTextMask, int identifierEndIndex) + { + int nextIndex = ReadNextCodeIndex(source, codeTextMask, identifierEndIndex); + return nextIndex >= 0 && (source[nextIndex] == '(' || source[nextIndex] == '<'); + } + + // Finds the '(' that opens the list the index is in, or -1 when the statement or block starts first. + private static int FindListOpenParenthesisIndex(string source, CodeTextMask codeTextMask, int startIndex) + { + int depth = 0; + for (int index = startIndex; index >= 0; index--) + { + if (!codeTextMask.IsCodeAt(index)) + { + continue; + } + + char character = source[index]; + if (character == ')' || character == ']') + { + depth++; + } + else if (character == '(' || character == '[') + { + if (depth == 0) + { + return character == '(' ? index : -1; + } + + depth--; + } + else if (character == ';' || character == '{' || character == '}') + { + return -1; + } + } + + return -1; + } + + // Finds the ')' that closes the list the index is in, or -1 when the statement or block ends first. + private static int FindListCloseParenthesisIndex(string source, CodeTextMask codeTextMask, int startIndex) + { + int depth = 0; + for (int index = startIndex; index < source.Length; index++) + { + if (!codeTextMask.IsCodeAt(index)) + { + continue; + } + + char character = source[index]; + if (character == '(' || character == '[') + { + depth++; + } + else if (character == ')' || character == ']') + { + if (depth == 0) + { + return character == ')' ? index : -1; + } + + depth--; + } + else if (character == ';' || character == '{' || character == '}') + { + return -1; + } + } + + return -1; + } + + private static bool IsArrowAt(string source, CodeTextMask codeTextMask, int index) + { + return index >= 0 && + index + 1 < source.Length && + source[index] == '=' && + source[index + 1] == '>' && + codeTextMask.IsCodeAt(index + 1); + } + + internal static bool IsNullableTypeSuffixOwner(char character) + { + return IsIdentifierCharacter(character) || character == '>' || character == ']' || character == ')'; + } + + private static bool IsExpressionPunctuation(char character) + { + return "([=;{}!&|^+-/%~:<".IndexOf(character) >= 0; + } + + // A ')' closes a tuple type when its parentheses hold a top-level comma and no top-level ';' or '='. + // "if (x) Run(..)" and "(T)Run(..)" have no comma, so they stay expressions. + private static bool IsTupleTypeClose(string source, CodeTextMask codeTextMask, int closeIndex) + { + int depth = 0; + bool hasTopLevelComma = false; + for (int index = closeIndex - 1; index >= 0; index--) + { + if (!codeTextMask.IsCodeAt(index)) + { + continue; + } + + char character = source[index]; + if (character == ')' || character == ']') + { + depth++; + continue; + } + + if (character == '(' || character == '[') + { + if (depth == 0) + { + return hasTopLevelComma; + } + + depth--; + continue; + } + + if (depth > 0) + { + continue; + } + + if (character == ';' || character == '=') + { + return false; + } + + if (character == '{' || character == '}') + { + // The open parenthesis cannot be found within the statement, so the shape is unknown. + return true; + } + + hasTopLevelComma |= character == ','; + } + + return true; + } + + internal static int ReadPreviousCodeIndex(string source, CodeTextMask codeTextMask, int startIndex) + { + int index = startIndex; + while (index >= 0 && (!codeTextMask.IsCodeAt(index) || char.IsWhiteSpace(source[index]))) + { + index--; + } + + return index; + } + + private static int ReadNextCodeIndex(string source, CodeTextMask codeTextMask, int startIndex) + { + int index = startIndex; + while (index < source.Length && (!codeTextMask.IsCodeAt(index) || char.IsWhiteSpace(source[index]))) + { + index++; + } + + return index < source.Length ? index : -1; + } + + private static string ReadIdentifierEndingAt(string source, CodeTextMask codeTextMask, int endIndex) + { + int startIndex = endIndex; + while (startIndex > 0 && + codeTextMask.IsCodeAt(startIndex - 1) && + IsIdentifierCharacter(source[startIndex - 1])) + { + startIndex--; + } + + return source.Substring(startIndex, endIndex - startIndex + 1); + } + + internal static int ReadIdentifierEndIndex(string source, int identifierStartIndex) + { + int index = identifierStartIndex; + while (index < source.Length && IsIdentifierCharacter(source[index])) + { + index++; + } + + return index; + } + } +} diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationDeclarationNameRules.cs.meta b/Packages/src/Editor/Domain/ThirdPartyToolMigrationDeclarationNameRules.cs.meta new file mode 100644 index 0000000000..49cd758fbb --- /dev/null +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationDeclarationNameRules.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 00a9953dffeb74c4095f853b70217051 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingCallerRules.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingCallerRules.cs index 9a75a3af82..ceea7acba1 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingCallerRules.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingCallerRules.cs @@ -53,12 +53,14 @@ public static ThirdPartyToolMigrationContentResult RemoveLegacyPlayerLoopTimingC string source, string originalSource, RemovedLegacyPlayerLoopTimingSignature[] removedSignatures, - string[] legacyAssemblyAliases) + string[] legacyAssemblyAliases, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex) { Debug.Assert(source != null, "source must not be null"); Debug.Assert(originalSource != null, "originalSource must not be null"); Debug.Assert(removedSignatures != null, "removedSignatures must not be null"); Debug.Assert(legacyAssemblyAliases != null, "legacyAssemblyAliases must not be null"); + Debug.Assert(typeHierarchyIndex != null, "typeHierarchyIndex must not be null"); string[] legacyNamespaceAliases = GetCombinedLegacyNamespaceAliases( originalSource, @@ -66,7 +68,8 @@ public static ThirdPartyToolMigrationContentResult RemoveLegacyPlayerLoopTimingC (string migratedContent, int replacementCount) = RemoveLegacyPlayerLoopTimingCallerArgumentsInCode( source, removedSignatures, - legacyNamespaceAliases); + legacyNamespaceAliases, + typeHierarchyIndex); return new ThirdPartyToolMigrationContentResult( migratedContent, replacementCount, @@ -106,11 +109,13 @@ public static ThirdPartyToolMigrationContentResult RemoveLegacyPlayerLoopTimingP public static (string Content, int ReplacementCount) RemoveLegacyPlayerLoopTimingCallerArgumentsInCode( string source, RemovedLegacyPlayerLoopTimingSignature[] removedSignatures, - string[] legacyNamespaceAliases) + string[] legacyNamespaceAliases, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex) { Debug.Assert(source != null, "source must not be null"); Debug.Assert(removedSignatures != null, "removedSignatures must not be null"); Debug.Assert(legacyNamespaceAliases != null, "legacyNamespaceAliases must not be null"); + Debug.Assert(typeHierarchyIndex != null, "typeHierarchyIndex must not be null"); string migratedContent = source; int replacementCount = 0; @@ -120,7 +125,8 @@ public static (string Content, int ReplacementCount) RemoveLegacyPlayerLoopTimin RemoveLegacyPlayerLoopTimingCallerArgumentsForMethodInCode( migratedContent, removedSignature, - legacyNamespaceAliases); + legacyNamespaceAliases, + typeHierarchyIndex); migratedContent = signatureMigratedContent; replacementCount += signatureReplacementCount; } @@ -132,11 +138,13 @@ public static (string Content, int ReplacementCount) RemoveLegacyPlayerLoopTimingCallerArgumentsForMethodInCode( string source, RemovedLegacyPlayerLoopTimingSignature removedSignature, - string[] legacyNamespaceAliases) + string[] legacyNamespaceAliases, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex) { Debug.Assert(source != null, "source must not be null"); Debug.Assert(!string.IsNullOrEmpty(removedSignature.MethodName), "MethodName must not be null or empty"); Debug.Assert(legacyNamespaceAliases != null, "legacyNamespaceAliases must not be null"); + Debug.Assert(typeHierarchyIndex != null, "typeHierarchyIndex must not be null"); Regex invocationRegex = new( $@"(?= 0, "methodNameIndex must not be negative"); Debug.Assert(arguments != null, "arguments must not be null"); + Debug.Assert(typeHierarchyIndex != null, "typeHierarchyIndex must not be null"); string[] trimmedArguments = GetTrimmedInvocationArguments(arguments); if (trimmedArguments.Length > removedSignature.OriginalParameters.Length) @@ -135,7 +137,8 @@ public static bool ShouldMigrateLegacyPlayerLoopTimingCaller( source, codeTextMask, methodNameIndex, - removedSignature)) + removedSignature, + typeHierarchyIndex)) { return false; } @@ -147,10 +150,12 @@ public static bool DoesPlayerLoopTimingCallerTargetRemovedSignature( string source, CodeTextMask codeTextMask, int methodNameIndex, - RemovedLegacyPlayerLoopTimingSignature removedSignature) + RemovedLegacyPlayerLoopTimingSignature removedSignature, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex) { Debug.Assert(source != null, "source must not be null"); Debug.Assert(methodNameIndex >= 0, "methodNameIndex must not be negative"); + Debug.Assert(typeHierarchyIndex != null, "typeHierarchyIndex must not be null"); if (removedSignature.DeclaringTypeName.Length == 0) { @@ -158,27 +163,26 @@ public static bool DoesPlayerLoopTimingCallerTargetRemovedSignature( } string targetExpression = ReadMemberTargetExpressionBeforeMethodName(source, methodNameIndex); - if (targetExpression.Length == 0) + bool isThisCall = string.Equals(targetExpression, "this", StringComparison.Ordinal); + if (targetExpression.Length == 0 || isThisCall) { - string containingTypeName = ReadContainingTypeName(source, codeTextMask, methodNameIndex); - return string.Equals( - containingTypeName, - removedSignature.DeclaringTypeName, - StringComparison.Ordinal); + return DoesImplicitInstanceCallTargetRemovedSignature( + source, + codeTextMask, + methodNameIndex, + removedSignature, + typeHierarchyIndex, + isThisCall); } if (string.Equals(targetExpression, "base", StringComparison.Ordinal)) { - return DoesBaseCallTargetRemovedSignature(source, codeTextMask, methodNameIndex, removedSignature); - } - - if (string.Equals(targetExpression, "this", StringComparison.Ordinal)) - { - string containingTypeName = ReadContainingTypeName(source, codeTextMask, methodNameIndex); - return string.Equals( - containingTypeName, - removedSignature.DeclaringTypeName, - StringComparison.Ordinal); + return DoesBaseCallTargetRemovedSignature( + source, + codeTextMask, + methodNameIndex, + removedSignature, + typeHierarchyIndex); } if (IsQualifiedMemberTargetExpression(targetExpression)) @@ -231,9 +235,89 @@ public static bool DoesPlayerLoopTimingCallerTargetRemovedSignature( removedSignature.DeclaringTypeName); } + // An unqualified or this. call binds to a member of the containing class or, when that class does not + // declare the name, to one inherited from its base classes, which only the project-wide index can follow. + private static bool DoesImplicitInstanceCallTargetRemovedSignature( + string source, + CodeTextMask codeTextMask, + int methodNameIndex, + RemovedLegacyPlayerLoopTimingSignature removedSignature, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex, + bool isThisCall) + { + // An empty target also comes back for a receiver that is an expression (GetOther().Run(..)); that call + // binds to whatever the expression returns, not to the containing class, even inside the declaring type. + if (!isThisCall && IsPrecededByMemberAccess(source, methodNameIndex)) + { + return false; + } + + string containingTypeName = ReadContainingTypeName(source, codeTextMask, methodNameIndex); + if (string.Equals(containingTypeName, removedSignature.DeclaringTypeName, StringComparison.Ordinal)) + { + return true; + } + + // new Runner(..) calls a constructor, which is never inherited. + if (IsPrecededByNewKeyword(source, codeTextMask, methodNameIndex)) + { + return false; + } + + return typeHierarchyIndex.IsInheritedMemberReachable( + containingTypeName, + removedSignature.MethodName, + removedSignature.DeclaringTypeName); + } + + // The ?. and !. accessors also end with '.' right before the method name. + private static bool IsPrecededByMemberAccess(string source, int methodNameIndex) + { + int index = SkipWhitespaceBackward(source, methodNameIndex - 1); + return index >= 0 && source[index] == '.'; + } + + private static bool IsPrecededByNewKeyword(string source, CodeTextMask codeTextMask, int methodNameIndex) + { + int index = methodNameIndex - 1; + while (index >= 0 && (!codeTextMask.IsCodeAt(index) || char.IsWhiteSpace(source[index]))) + { + index--; + } + + const string newKeyword = "new"; + int keywordStartIndex = index - newKeyword.Length + 1; + if (keywordStartIndex < 0 || + string.CompareOrdinal(source, keywordStartIndex, newKeyword, 0, newKeyword.Length) != 0) + { + return false; + } + + return keywordStartIndex == 0 || !IsIdentifierCharacter(source[keywordStartIndex - 1]); + } + // A base call resolves against the base class of the containing class, not the containing class itself. - // When the source cannot name that base class, the call is left unchanged rather than guessed. + // The direct base class named in this source is checked first; the project-wide index then follows + // intermediate classes in other sources. When neither can settle it, the call is left unchanged. private static bool DoesBaseCallTargetRemovedSignature( + string source, + CodeTextMask codeTextMask, + int methodNameIndex, + RemovedLegacyPlayerLoopTimingSignature removedSignature, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex) + { + if (IsDirectBaseClassDeclaringType(source, codeTextMask, methodNameIndex, removedSignature)) + { + return true; + } + + return typeHierarchyIndex.IsBaseMemberReachable( + ReadContainingTypeName(source, codeTextMask, methodNameIndex), + removedSignature.MethodName, + removedSignature.DeclaringTypeName); + } + + private static bool IsDirectBaseClassDeclaringType( string source, CodeTextMask codeTextMask, int methodNameIndex, diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingTypeScopeRules.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingTypeScopeRules.cs index 9ab538278a..e214cbb356 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingTypeScopeRules.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTimingTypeScopeRules.cs @@ -178,8 +178,11 @@ public static string ReadContainingClassBaseTypeName( } // Comments and strings are blanked out so a colon or comma inside them is not read as a delimiter. - private static string ReadCodeOnlyText(string source, CodeTextMask codeTextMask, int startIndex, int endIndex) + public static string ReadCodeOnlyText(string source, CodeTextMask codeTextMask, int startIndex, int endIndex) { + Debug.Assert(source != null, "source must not be null"); + Debug.Assert(startIndex >= 0 && startIndex <= endIndex, "startIndex must not be negative or past endIndex"); + StringBuilder builder = new(endIndex - startIndex); for (int index = startIndex; index < endIndex; index++) { @@ -189,7 +192,7 @@ private static string ReadCodeOnlyText(string source, CodeTextMask codeTextMask, return builder.ToString(); } - private static bool IsClassDeclarationKeyword(string declaration) + public static bool IsClassDeclarationKeyword(string declaration) { if (declaration.StartsWith("class", StringComparison.Ordinal)) { diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchyIndex.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchyIndex.cs new file mode 100644 index 0000000000..70db58201f --- /dev/null +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchyIndex.cs @@ -0,0 +1,257 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; + +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationTimingTypeNameRules; + +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Index of the classes in a project's C# sources, their base classes, and the names they declare, used to + /// decide whether an unqualified, this. or base. call inside a derived class reaches the class that declares + /// a removed timing signature. Every question that the text cannot settle is answered "not reachable". + /// + public sealed class ThirdPartyToolMigrationTypeHierarchyIndex + { + public static readonly ThirdPartyToolMigrationTypeHierarchyIndex Empty = + new(new Dictionary(StringComparer.Ordinal)); + + private readonly Dictionary _classesByQualifiedName; + + private ThirdPartyToolMigrationTypeHierarchyIndex(Dictionary classesByQualifiedName) + { + _classesByQualifiedName = classesByQualifiedName; + } + + public static ThirdPartyToolMigrationTypeHierarchyIndex Build(IReadOnlyList sources) + { + Debug.Assert(sources != null, "sources must not be null"); + + Dictionary> partsByQualifiedName = + new(StringComparer.Ordinal); + foreach (string source in sources) + { + Debug.Assert(source != null, "sources must not contain null"); + + foreach (ThirdPartyToolMigrationTypeHierarchyClassPart part in + ThirdPartyToolMigrationTypeHierarchySourceReader.ReadClassParts(source)) + { + if (!partsByQualifiedName.TryGetValue(part.QualifiedName, out List parts)) + { + parts = new List(); + partsByQualifiedName.Add(part.QualifiedName, parts); + } + + parts.Add(part); + } + } + + Dictionary classesByQualifiedName = new(StringComparer.Ordinal); + foreach (KeyValuePair> entry in partsByQualifiedName) + { + classesByQualifiedName.Add(entry.Key, new IndexedClass(entry.Value)); + } + + return new ThirdPartyToolMigrationTypeHierarchyIndex(classesByQualifiedName); + } + + /// + /// Decides whether an unqualified or this. call of the member inside the containing class binds to the member + /// that the declaring class declares. A name the containing class uses for anything (members, locals, + /// parameters) could take the call instead, so it keeps the call unresolved. + /// + public bool IsInheritedMemberReachable(string containingTypeName, string memberName, string declaringTypeName) + { + Debug.Assert(containingTypeName != null, "containingTypeName must not be null"); + Debug.Assert(!string.IsNullOrEmpty(memberName), "memberName must not be null or empty"); + Debug.Assert(declaringTypeName != null, "declaringTypeName must not be null"); + + if (!TryGetUnambiguous(containingTypeName, out IndexedClass containingClass)) + { + return false; + } + + if (containingClass.ScopeNames.Contains(memberName)) + { + return false; + } + + return IsBaseMemberReachable(containingTypeName, memberName, declaringTypeName); + } + + /// + /// Decides whether a base. call of the member inside the containing class binds to the member that the + /// declaring class declares, walking the base classes up from the containing class. + /// + public bool IsBaseMemberReachable(string containingTypeName, string memberName, string declaringTypeName) + { + Debug.Assert(containingTypeName != null, "containingTypeName must not be null"); + Debug.Assert(!string.IsNullOrEmpty(memberName), "memberName must not be null or empty"); + Debug.Assert(declaringTypeName != null, "declaringTypeName must not be null"); + + HashSet visited = new(StringComparer.Ordinal) { containingTypeName }; + string current = ResolveBaseTypeName(containingTypeName); + while (current.Length > 0) + { + if (!visited.Add(current)) + { + return false; + } + + if (!TryGetUnambiguous(current, out IndexedClass currentClass)) + { + return false; + } + + // The first class up the chain that declares the name owns the call, whatever overload it picks. + if (currentClass.MemberNames.Contains(memberName)) + { + return string.Equals(current, declaringTypeName, StringComparison.Ordinal) && + currentClass.NonPrivateMemberNames.Contains(memberName); + } + + current = ResolveBaseTypeName(current); + } + + return false; + } + + // Partial parts may disagree on their first base list entry because interfaces can be listed in any part. + // The base class is the single indexed class the parts resolve to; none means no base class in the index, + // and more than one cannot be decided. + private string ResolveBaseTypeName(string qualifiedName) + { + if (!TryGetUnambiguous(qualifiedName, out IndexedClass indexedClass)) + { + return string.Empty; + } + + HashSet resolved = new(StringComparer.Ordinal); + foreach (ThirdPartyToolMigrationTypeHierarchyClassPart part in indexedClass.Parts) + { + if (part.WrittenBaseName.Length == 0) + { + continue; + } + + List hits = ResolvePartCandidates(part); + if (hits.Count >= 2) + { + return string.Empty; + } + + if (hits.Count == 1) + { + resolved.Add(hits[0]); + } + } + + if (resolved.Count != 1) + { + return string.Empty; + } + + foreach (string baseTypeName in resolved) + { + return baseTypeName; + } + + return string.Empty; + } + + // Instead of replaying C#'s lookup order, every qualified name the written base name could mean in the part's + // context is collected, and the name is resolved only when exactly one of them is an indexed class. + private List ResolvePartCandidates(ThirdPartyToolMigrationTypeHierarchyClassPart part) + { + string writtenBaseName = part.WrittenBaseName; + string name = NormalizeTypeNameForComparison(writtenBaseName); + List candidates = new(); + if (writtenBaseName.StartsWith("global::", StringComparison.Ordinal)) + { + candidates.Add(name); + } + else + { + foreach (string enclosingTypeName in part.EnclosingTypeNames) + { + candidates.Add(enclosingTypeName + "." + name); + } + + AddNamespaceCandidates(candidates, part.NamespaceName, name); + foreach (string usingNamespace in part.UsingNamespaces) + { + candidates.Add(usingNamespace + "." + name); + } + } + + List hits = new(); + foreach (string candidate in candidates) + { + if (_classesByQualifiedName.ContainsKey(candidate) && !hits.Contains(candidate)) + { + hits.Add(candidate); + } + } + + return hits; + } + + // The name may be written relative to the namespace of the declaration or any namespace that encloses it. + private static void AddNamespaceCandidates(List candidates, string namespaceName, string name) + { + string current = namespaceName; + while (true) + { + candidates.Add(current.Length == 0 ? name : current + "." + name); + if (current.Length == 0) + { + return; + } + + int lastDotIndex = current.LastIndexOf('.'); + current = lastDotIndex < 0 ? string.Empty : current.Substring(0, lastDotIndex); + } + } + + private bool TryGetUnambiguous(string qualifiedName, out IndexedClass indexedClass) + { + if (!_classesByQualifiedName.TryGetValue(qualifiedName, out indexedClass)) + { + return false; + } + + return !indexedClass.IsAmbiguous; + } + + /// + /// All parts of one qualified class name merged together. + /// + private sealed class IndexedClass + { + public IndexedClass(List parts) + { + Debug.Assert(parts != null && parts.Count > 0, "parts must not be null or empty"); + + Parts = parts; + bool hasNonPartialPart = false; + foreach (ThirdPartyToolMigrationTypeHierarchyClassPart part in parts) + { + hasNonPartialPart |= !part.IsPartial; + ScopeNames.UnionWith(part.ScopeNames); + MemberNames.UnionWith(part.MemberNames); + NonPrivateMemberNames.UnionWith(part.NonPrivateMemberNames); + } + + // Two declarations of one name are only legal as partial parts; anything else is either a duplicate + // in another assembly or text this reader misread, and neither can be resolved safely. + IsAmbiguous = parts.Count >= 2 && hasNonPartialPart; + } + + public List Parts { get; } + public HashSet ScopeNames { get; } = new(StringComparer.Ordinal); + public HashSet MemberNames { get; } = new(StringComparer.Ordinal); + public HashSet NonPrivateMemberNames { get; } = new(StringComparer.Ordinal); + public bool IsAmbiguous { get; } + } + } +} diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchyIndex.cs.meta b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchyIndex.cs.meta new file mode 100644 index 0000000000..dee666c85b --- /dev/null +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchyIndex.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d5c380c0565c149aa97cd5a129bcef40 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchySourceReader.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchySourceReader.cs new file mode 100644 index 0000000000..cadf79e286 --- /dev/null +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchySourceReader.cs @@ -0,0 +1,395 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Text.RegularExpressions; + +using CodeTextMask = io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationParsingRules.CodeTextMask; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationDeclarationNameRules; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationParsingRules; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationRuleCatalog; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationTimingMethodBodyRules; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationTimingTypeNameRules; +using static io.github.hatayama.UnityCliLoop.Domain.ThirdPartyToolMigrationTimingTypeScopeRules; + +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// One class declaration (or one part of a partial class) read from a source: the names it declares and the + /// context needed to resolve the base name it writes. + /// + public sealed class ThirdPartyToolMigrationTypeHierarchyClassPart + { + public ThirdPartyToolMigrationTypeHierarchyClassPart( + string qualifiedName, + bool isPartial, + string writtenBaseName, + string namespaceName, + IReadOnlyList enclosingTypeNames, + IReadOnlyList usingNamespaces, + IReadOnlyCollection scopeNames, + IReadOnlyCollection memberNames, + IReadOnlyCollection nonPrivateMemberNames) + { + Debug.Assert(!string.IsNullOrEmpty(qualifiedName), "qualifiedName must not be null or empty"); + Debug.Assert(writtenBaseName != null, "writtenBaseName must not be null"); + Debug.Assert(namespaceName != null, "namespaceName must not be null"); + Debug.Assert(enclosingTypeNames != null, "enclosingTypeNames must not be null"); + Debug.Assert(usingNamespaces != null, "usingNamespaces must not be null"); + Debug.Assert(scopeNames != null, "scopeNames must not be null"); + Debug.Assert(memberNames != null, "memberNames must not be null"); + Debug.Assert(nonPrivateMemberNames != null, "nonPrivateMemberNames must not be null"); + + QualifiedName = qualifiedName; + IsPartial = isPartial; + WrittenBaseName = writtenBaseName; + NamespaceName = namespaceName; + EnclosingTypeNames = enclosingTypeNames; + UsingNamespaces = usingNamespaces; + ScopeNames = scopeNames; + MemberNames = memberNames; + NonPrivateMemberNames = nonPrivateMemberNames; + } + + public string QualifiedName { get; } + public bool IsPartial { get; } + + // The first base list entry as written, or empty when the declaration has no base list. + public string WrittenBaseName { get; } + public string NamespaceName { get; } + + // Qualified names of the enclosing types, from the innermost to the outermost. + public IReadOnlyList EnclosingTypeNames { get; } + public IReadOnlyList UsingNamespaces { get; } + + // Names declared anywhere in the class body outside nested type bodies, including locals and parameters. + public IReadOnlyCollection ScopeNames { get; } + + // Names declared directly in the class body. + public IReadOnlyCollection MemberNames { get; } + public IReadOnlyCollection NonPrivateMemberNames { get; } + } + + /// + /// Reads the class declarations of one C# source for the type hierarchy index. + /// + public static class ThirdPartyToolMigrationTypeHierarchySourceReader + { + private static readonly Regex AccessModifierRegex = + new(@"\b(?:public|protected|internal)\b", RegexOptions.Compiled); + + private static readonly Regex PartialModifierRegex = new(@"\bpartial\b", RegexOptions.Compiled); + + public static List ReadClassParts(string source) + { + Debug.Assert(source != null, "source must not be null"); + + CodeTextMask codeTextMask = CodeTextMask.Create(source); + List typeBodies = ReadTypeBodies(source, codeTextMask); + List parts = new(); + foreach (TypeBody typeBody in typeBodies) + { + if (typeBody.IsClass) + { + parts.Add(ReadClassPart(source, codeTextMask, typeBody, typeBodies)); + } + } + + return parts; + } + + private static List ReadTypeBodies(string source, CodeTextMask codeTextMask) + { + List typeBodies = new(); + foreach (Match match in TypeDeclarationNameRegex.Matches(source)) + { + if (!codeTextMask.IsCodeAt(match.Index)) + { + continue; + } + + int openBraceIndex = FindTypeBodyOpenBraceIndex(source, codeTextMask, match.Index + match.Length); + if (openBraceIndex < 0) + { + continue; + } + + int closeBraceIndex = FindBlockClosingBraceIndex(source, codeTextMask, openBraceIndex); + if (closeBraceIndex < 0) + { + continue; + } + + typeBodies.Add(new TypeBody(match, openBraceIndex, closeBraceIndex)); + } + + return typeBodies; + } + + private static ThirdPartyToolMigrationTypeHierarchyClassPart ReadClassPart( + string source, + CodeTextMask codeTextMask, + TypeBody typeBody, + List typeBodies) + { + // The qualified name must come from ReadContainingTypeName, the same function that names the declaring + // type of a removed signature and the type that contains a call; any other spelling never matches them. + int bodyStartIndex = typeBody.OpenBraceIndex + 1; + string qualifiedName = ReadContainingTypeName(source, codeTextMask, bodyStartIndex); + int enclosingTypeCount = ReadContainingTypeDeclarations(source, codeTextMask, bodyStartIndex).Count - 1; + int declarationIndex = typeBody.Declaration.Index; + string header = ReadCodeOnlyText( + source, + codeTextMask, + declarationIndex + typeBody.Declaration.Length, + typeBody.OpenBraceIndex); + DeclaredNames declaredNames = ReadDeclaredNames(source, codeTextMask, typeBody, typeBodies); + + return new ThirdPartyToolMigrationTypeHierarchyClassPart( + qualifiedName, + PartialModifierRegex.IsMatch(ReadModifierText(source, codeTextMask, declarationIndex)), + ReadFirstBaseListTypeName(header), + ReadNamespaceName(source, codeTextMask, declarationIndex), + ReadEnclosingTypeNames(qualifiedName, enclosingTypeCount), + ReadUsingNamespaces(source, codeTextMask, declarationIndex), + declaredNames.ScopeNames, + declaredNames.MemberNames, + declaredNames.NonPrivateMemberNames); + } + + // Each enclosing type's qualified name is the class's qualified name with trailing segments dropped. + private static List ReadEnclosingTypeNames(string qualifiedName, int enclosingTypeCount) + { + List enclosingTypeNames = new(); + string current = qualifiedName; + for (int count = 0; count < enclosingTypeCount; count++) + { + int lastDotIndex = current.LastIndexOf('.'); + if (lastDotIndex < 0) + { + break; + } + + current = current.Substring(0, lastDotIndex); + enclosingTypeNames.Add(current); + } + + return enclosingTypeNames; + } + + private static List ReadUsingNamespaces(string source, CodeTextMask codeTextMask, int declarationIndex) + { + List usingNamespaces = new(); + foreach (string importedNamespace in ReadImportedNamespaceNames(source, codeTextMask, declarationIndex)) + { + usingNamespaces.Add(NormalizeTypeNameForComparison(importedNamespace)); + } + + return usingNamespaces; + } + + // The modifiers of a declaration run back to the previous statement end, block brace, or attribute bracket. + // An array rank specifier in the return type ("int[] Run", "Task Run") is skipped, not a boundary. + private static string ReadModifierText(string source, CodeTextMask codeTextMask, int declarationIndex) + { + int startIndex = declarationIndex - 1; + while (startIndex >= 0) + { + if (codeTextMask.IsCodeAt(startIndex) && IsModifierBoundary(source[startIndex])) + { + int rankOpenIndex = ReadRankSpecifierOpenIndex(source, codeTextMask, startIndex); + if (rankOpenIndex < 0) + { + break; + } + + startIndex = rankOpenIndex; + } + + startIndex--; + } + + return ReadCodeOnlyText(source, codeTextMask, startIndex + 1, declarationIndex); + } + + private static bool IsModifierBoundary(char character) + { + return character == ';' || character == '{' || character == '}' || character == ']'; + } + + // Returns the '[' of a rank specifier ("[]", "[,]") closed at the index, or -1 for any other bracket such as + // an attribute. A rank specifier holds only commas and spaces and follows a type: a name, '>', ']', '?' or ')'. + private static int ReadRankSpecifierOpenIndex(string source, CodeTextMask codeTextMask, int closeIndex) + { + if (source[closeIndex] != ']') + { + return -1; + } + + int index = closeIndex - 1; + while (index >= 0 && codeTextMask.IsCodeAt(index) && (source[index] == ',' || char.IsWhiteSpace(source[index]))) + { + index--; + } + + if (index < 0 || !codeTextMask.IsCodeAt(index) || source[index] != '[') + { + return -1; + } + + int ownerIndex = ReadPreviousCodeIndex(source, codeTextMask, index - 1); + // '?' is allowed here only: in a declaration's modifiers and type, "?[" can only be a nullable element type. + return ownerIndex >= 0 && (IsNullableTypeSuffixOwner(source[ownerIndex]) || source[ownerIndex] == '?') + ? index + : -1; + } + + // Scans the class body once, front to back, skipping nested type bodies, so building the index stays linear. + private static DeclaredNames ReadDeclaredNames( + string source, + CodeTextMask codeTextMask, + TypeBody typeBody, + List typeBodies) + { + Dictionary nestedBodyCloseIndexByOpenIndex = new(); + foreach (TypeBody candidate in typeBodies) + { + if (candidate.OpenBraceIndex > typeBody.OpenBraceIndex && + candidate.CloseBraceIndex < typeBody.CloseBraceIndex) + { + nestedBodyCloseIndexByOpenIndex[candidate.OpenBraceIndex] = candidate.CloseBraceIndex; + } + } + + DeclaredNames declaredNames = new(); + BodyScanState state = new(); + for (int index = typeBody.OpenBraceIndex + 1; index < typeBody.CloseBraceIndex; index++) + { + if (!codeTextMask.IsCodeAt(index)) + { + continue; + } + + if (nestedBodyCloseIndexByOpenIndex.TryGetValue(index, out int nestedCloseIndex)) + { + index = nestedCloseIndex; + continue; + } + + if (state.TryTrackBracket(source[index])) + { + continue; + } + + if (!IsIdentifierStartAt(source, codeTextMask, index)) + { + continue; + } + + int identifierEndIndex = ReadIdentifierEndIndex(source, index); + AddDeclaredName(source, codeTextMask, index, identifierEndIndex, state, declaredNames); + index = identifierEndIndex - 1; + } + + return declaredNames; + } + + private static void AddDeclaredName( + string source, + CodeTextMask codeTextMask, + int identifierStartIndex, + int identifierEndIndex, + BodyScanState state, + DeclaredNames declaredNames) + { + if (!IsDeclarationName(source, codeTextMask, identifierStartIndex, state.ParenDepth)) + { + return; + } + + string name = source.Substring(identifierStartIndex, identifierEndIndex - identifierStartIndex); + declaredNames.ScopeNames.Add(name); + + // Parameters sit at brace depth 1 too, but inside the parameter list; they are not members. + if (state.BraceDepth != 1 || state.ParenDepth != 0) + { + return; + } + + declaredNames.MemberNames.Add(name); + if (AccessModifierRegex.IsMatch(ReadModifierText(source, codeTextMask, identifierStartIndex))) + { + declaredNames.NonPrivateMemberNames.Add(name); + } + } + + private static bool IsIdentifierStartAt(string source, CodeTextMask codeTextMask, int index) + { + if (!IsIdentifierStartCharacter(source[index])) + { + return false; + } + + return index == 0 || !codeTextMask.IsCodeAt(index - 1) || !IsIdentifierCharacter(source[index - 1]); + } + + private readonly struct TypeBody + { + public TypeBody(Match declaration, int openBraceIndex, int closeBraceIndex) + { + Declaration = declaration; + OpenBraceIndex = openBraceIndex; + CloseBraceIndex = closeBraceIndex; + // Only classes and record classes have a base class; structs and interfaces list interfaces only. + IsClass = IsClassDeclarationKeyword(declaration.Value); + } + + public Match Declaration { get; } + public int OpenBraceIndex { get; } + public int CloseBraceIndex { get; } + public bool IsClass { get; } + } + + // Tracks brace and bracket depth while scanning a class body. The bracket depth restarts inside every brace + // block so a lambda body inside an argument list reads its own statements at depth 0. + private sealed class BodyScanState + { + private readonly Stack _savedParenDepths = new(); + + public int BraceDepth { get; private set; } = 1; + public int ParenDepth { get; private set; } + + public bool TryTrackBracket(char character) + { + switch (character) + { + case '{': + _savedParenDepths.Push(ParenDepth); + ParenDepth = 0; + BraceDepth++; + return true; + case '}': + BraceDepth--; + ParenDepth = _savedParenDepths.Count > 0 ? _savedParenDepths.Pop() : 0; + return true; + case '(': + case '[': + ParenDepth++; + return true; + case ')': + case ']': + ParenDepth = Math.Max(0, ParenDepth - 1); + return true; + default: + return false; + } + } + } + + private sealed class DeclaredNames + { + public HashSet ScopeNames { get; } = new(StringComparer.Ordinal); + public HashSet MemberNames { get; } = new(StringComparer.Ordinal); + public HashSet NonPrivateMemberNames { get; } = new(StringComparer.Ordinal); + } + } +} diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchySourceReader.cs.meta b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchySourceReader.cs.meta new file mode 100644 index 0000000000..a4b1e2703e --- /dev/null +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeHierarchySourceReader.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 96e5b59a41e5c41da8bc09ee05f515be +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs index fc847ba934..bb6a947891 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs @@ -30,6 +30,7 @@ public class CompileController : IDisposable private int _assemblyFinishedCount; private int _consoleErrorCountAtCompileStart; private readonly CompileLifecycleRecoveryCoordinator _recoveryCoordinator; + private ICompilePipelinePort _pipeline = new UnityCompilePipelinePort(); public CompileController( ICompileResultSessionRepository compileResultSessionRepository, @@ -47,7 +48,9 @@ public CompileController( IsCompileRequestCompleted, () => _currentCompileTask, entries => new AssemblyDefinitionConsoleErrorValidationService().FindErrors(entries), - ReadConsoleErrorEntries, + // Why a lambda instead of the method group: the coordinator must read through the + // port that is current at recovery time, not the one captured at construction. + () => _pipeline.ReadConsoleErrorEntries(), () => _consoleErrorCountAtCompileStart, () => new AssemblyDefinitionDuplicationValidationService().ValidateNoDuplicateAsmdefNames(), () => _isForceCompile, @@ -114,6 +117,24 @@ internal void SetExternalSceneChangeResolutionForTesting( throw new ArgumentNullException(nameof(resolveExternalSceneChanges)); } + /// + /// Replaces the Unity compile pipeline so tests can drive a compile request without + /// refreshing assets or starting a real compilation. + /// + internal void SetCompilePipelineForTesting(ICompilePipelinePort pipeline) + { + UnityEngine.Debug.Assert(pipeline != null, "pipeline must not be null"); + ICompilePipelinePort validatedPipeline = pipeline ?? throw new ArgumentNullException(nameof(pipeline)); + // Why: subscribe and unsubscribe both go through the current port, so swapping it mid-compile + // would send the unsubscribe to the new port and leave the old subscription holding this controller. + if (_isCompiling) + { + throw new InvalidOperationException("The compile pipeline cannot be replaced while a compile is in flight."); + } + + _pipeline = validatedPipeline; + } + private (bool CanProceed, string Message, string[] ScenePaths) ResolveExternalSceneChanges() { if (_resolveExternalSceneChangesForTesting != null) @@ -183,7 +204,7 @@ public async Task TryCompileAsync(bool forceRecompile, string pla _isForceCompile = forceRecompile; // Why before Refresh: the asmdef import errors that abort a compile are logged during // AssetDatabase.Refresh, so the boundary must precede it to keep them in the summary. - _consoleErrorCountAtCompileStart = ReadConsoleErrorEntries().Length; + _consoleErrorCountAtCompileStart = _pipeline.ReadConsoleErrorEntries().Length; bool eventsRegistered = false; bool compileTaskTransferred = false; @@ -193,11 +214,10 @@ public async Task TryCompileAsync(bool forceRecompile, string pla // and that compile can raise Script Updating Consent. Begin must be set first // or the prefix lets the modal through on the typical "edit then uloop compile" path. CompileApiUpdaterConsentState.BeginCliCompile(); - AssetDatabase.Refresh(); + _pipeline.RefreshAssets(); - AssemblyDefinitionConsoleErrorValidationService assemblyDefinitionValidationService = new(); AssemblyDefinitionConsoleErrorResult assemblyDefinitionErrors = - assemblyDefinitionValidationService.FindCurrentErrors(); + _pipeline.FindCurrentAssemblyDefinitionErrors(); if (assemblyDefinitionErrors.HasErrors) { CompileResult result = @@ -215,23 +235,15 @@ public async Task TryCompileAsync(bool forceRecompile, string pla } // Register events. - CompilationPipeline.compilationFinished += HandleCompileFinished; - CompilationPipeline.assemblyCompilationFinished += HandleAssemblyFinished; + _pipeline.SubscribeCompilationEvents(HandleCompileFinished, HandleAssemblyFinished); eventsRegistered = true; string startMessage = forceRecompile ? "Forced recompile started after asset refresh..." : "Compilation started after asset refresh..."; OnCompileStarted?.Invoke(startMessage); - if (forceRecompile) - { - CompilationPipeline.RequestScriptCompilation(RequestScriptCompilationOptions.CleanBuildCache); - } - else - { - CompilationPipeline.RequestScriptCompilation(); - } + _pipeline.RequestScriptCompilation(forceRecompile); - _recoveryCoordinator.StartWatchdog(compileTask, ct); + _pipeline.StartWatchdog(_recoveryCoordinator, compileTask, ct); compileTaskTransferred = true; return await compileTask.Task.ConfigureAwait(false); } @@ -251,16 +263,6 @@ private bool IsCompileRequestCompleted() return _currentCompileTask == null || _currentCompileTask.Task.IsCompleted; } - /// - /// Snapshots the current Unity Console error entries for indeterminate-result diagnosis. - /// - private static UnityCliLoopConsoleLogEntry[] ReadConsoleErrorEntries() - { - IUnityCliLoopConsoleLogService consoleLogs = new LogRetrievalService(); - UnityCliLoopConsoleLogResult errorLogs = consoleLogs.GetLogs(UnityCliLoopLogType.Error); - return errorLogs.LogEntries; - } - private Dictionary BuildCompileControllerStateContext( Dictionary extraContext) { @@ -455,8 +457,7 @@ private void RecordCompileResultIfNeeded(CompileResult result, string playModeSt /// private void UnregisterCompilationEvents() { - CompilationPipeline.compilationFinished -= HandleCompileFinished; - CompilationPipeline.assemblyCompilationFinished -= HandleAssemblyFinished; + _pipeline.UnsubscribeCompilationEvents(HandleCompileFinished, HandleAssemblyFinished); } /// @@ -544,8 +545,7 @@ private void HandleAssemblyFinished(string asmPath, CompilerMessage[] messages) public void Cleanup() { // Unregister events just in case. - CompilationPipeline.compilationFinished -= HandleCompileFinished; - CompilationPipeline.assemblyCompilationFinished -= HandleAssemblyFinished; + UnregisterCompilationEvents(); // If there is an incomplete task, cancel it. if (_currentCompileTask != null && !_currentCompileTask.Task.IsCompleted) diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompilePipelinePort.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompilePipelinePort.cs new file mode 100644 index 0000000000..d4198ccd33 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompilePipelinePort.cs @@ -0,0 +1,98 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using UnityEditor; +using UnityEditor.Compilation; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Unity Editor operations CompileController needs to start and observe a compile request. + /// Kept behind a port so tests can drive the controller without refreshing assets or compiling. + /// + internal interface ICompilePipelinePort + { + void RefreshAssets(); + + AssemblyDefinitionConsoleErrorResult FindCurrentAssemblyDefinitionErrors(); + + UnityCliLoopConsoleLogEntry[] ReadConsoleErrorEntries(); + + void SubscribeCompilationEvents( + Action compilationFinished, + Action assemblyFinished); + + void UnsubscribeCompilationEvents( + Action compilationFinished, + Action assemblyFinished); + + void RequestScriptCompilation(bool cleanBuildCache); + + void StartWatchdog( + CompileLifecycleRecoveryCoordinator coordinator, + TaskCompletionSource compileTask, + CancellationToken ct); + } + + /// + /// Production compile pipeline that forwards to AssetDatabase, the Unity Console, and CompilationPipeline. + /// + internal sealed class UnityCompilePipelinePort : ICompilePipelinePort + { + public void RefreshAssets() + { + AssetDatabase.Refresh(); + } + + public AssemblyDefinitionConsoleErrorResult FindCurrentAssemblyDefinitionErrors() + { + AssemblyDefinitionConsoleErrorValidationService assemblyDefinitionValidationService = new(); + return assemblyDefinitionValidationService.FindCurrentErrors(); + } + + /// + /// Snapshots the current Unity Console error entries for indeterminate-result diagnosis. + /// + public UnityCliLoopConsoleLogEntry[] ReadConsoleErrorEntries() + { + IUnityCliLoopConsoleLogService consoleLogs = new LogRetrievalService(); + UnityCliLoopConsoleLogResult errorLogs = consoleLogs.GetLogs(UnityCliLoopLogType.Error); + return errorLogs.LogEntries; + } + + public void SubscribeCompilationEvents( + Action compilationFinished, + Action assemblyFinished) + { + CompilationPipeline.compilationFinished += compilationFinished; + CompilationPipeline.assemblyCompilationFinished += assemblyFinished; + } + + public void UnsubscribeCompilationEvents( + Action compilationFinished, + Action assemblyFinished) + { + CompilationPipeline.compilationFinished -= compilationFinished; + CompilationPipeline.assemblyCompilationFinished -= assemblyFinished; + } + + public void RequestScriptCompilation(bool cleanBuildCache) + { + if (cleanBuildCache) + { + CompilationPipeline.RequestScriptCompilation(RequestScriptCompilationOptions.CleanBuildCache); + return; + } + + CompilationPipeline.RequestScriptCompilation(); + } + + public void StartWatchdog( + CompileLifecycleRecoveryCoordinator coordinator, + TaskCompletionSource compileTask, + CancellationToken ct) + { + coordinator.StartWatchdog(compileTask, ct); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompilePipelinePort.cs.meta b/Packages/src/Editor/FirstPartyTools/Compile/CompilePipelinePort.cs.meta new file mode 100644 index 0000000000..90f972afaa --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompilePipelinePort.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 635807854df844d38a2ac0e1ce91c882 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/CliPlayModeRunInBackgroundService.cs b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/CliPlayModeRunInBackgroundService.cs index a1f73f7f09..682a25ef49 100644 --- a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/CliPlayModeRunInBackgroundService.cs +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/CliPlayModeRunInBackgroundService.cs @@ -1,4 +1,5 @@ #nullable enable +using System; using UnityEditor; using UnityEngine; @@ -18,12 +19,19 @@ public interface ICliPlayModeRunInBackgroundStarter internal sealed class CliPlayModeRunInBackgroundService : ICliPlayModeRunInBackgroundStarter { private readonly CliPlayModeRunInBackgroundController _controller; + private readonly Action _setRunInBackground; + private readonly Func _getRunInBackground; private bool _isPlayModeCallbackRegistered; - public CliPlayModeRunInBackgroundService(CliPlayModeRunInBackgroundController controller) + public CliPlayModeRunInBackgroundService( + CliPlayModeRunInBackgroundController controller, + Action? setRunInBackground = null, + Func? getRunInBackground = null) { System.Diagnostics.Debug.Assert(controller != null, "controller must not be null"); _controller = controller!; + _setRunInBackground = setRunInBackground ?? (value => Application.runInBackground = value); + _getRunInBackground = getRunInBackground ?? (() => Application.runInBackground); } /// @@ -36,7 +44,7 @@ public void InitializeForEditorStartup() bool? desiredRunInBackground = _controller.OnEditorStartup(EditorApplication.isPlaying); if (desiredRunInBackground.HasValue) { - Application.runInBackground = desiredRunInBackground.Value; + _setRunInBackground(desiredRunInBackground.Value); } } @@ -45,8 +53,8 @@ public void InitializeForEditorStartup() /// public void EnableForCliPlayStart() { - bool desiredRunInBackground = _controller.OnCliPlayStarting(Application.runInBackground); - Application.runInBackground = desiredRunInBackground; + bool desiredRunInBackground = _controller.OnCliPlayStarting(_getRunInBackground()); + _setRunInBackground(desiredRunInBackground); } private void RegisterPlayModeCallback() @@ -62,7 +70,7 @@ private void RegisterPlayModeCallback() _isPlayModeCallbackRegistered = true; } - private void OnPlayModeStateChanged(PlayModeStateChange state) + internal void OnPlayModeStateChanged(PlayModeStateChange state) { // Why: ExitingPlayMode covers CLI Stop and the toolbar Stop button, but Unity may // overwrite runInBackground during the transition or domain-reload afterward. @@ -72,7 +80,7 @@ private void OnPlayModeStateChanged(PlayModeStateChange state) bool? originalRunInBackground = _controller.PeekOriginalIfActive(); if (originalRunInBackground.HasValue) { - Application.runInBackground = originalRunInBackground.Value; + _setRunInBackground(originalRunInBackground.Value); } return; @@ -86,7 +94,7 @@ private void OnPlayModeStateChanged(PlayModeStateChange state) bool? restoredRunInBackground = _controller.CommitRestoreAfterPlayModeExit(); if (restoredRunInBackground.HasValue) { - Application.runInBackground = restoredRunInBackground.Value; + _setRunInBackground(restoredRunInBackground.Value); } } } diff --git a/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoService.cs b/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoService.cs index 862ff21491..c074899101 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoService.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoService.cs @@ -1,7 +1,5 @@ -using System; using System.IO; using UnityEditor; -using UnityEngine; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -12,20 +10,29 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal static class RecordVideoService { - private static VideoRecordingSession _session; - private static bool _usedDefaultOutputPath; - private static bool _stopOnPlayModeExit; + private static readonly RecordVideoSessionHost ServiceValue = new RecordVideoSessionHost( + (outputPath, width, height, frameRate, useVp8, quality) => new MediaEncoderVideoFrameEncoder( + outputPath, + width, + height, + frameRate, + useVp8, + quality), + () => EditorApplication.timeSinceStartup, + callback => EditorApplication.update += callback, + callback => EditorApplication.update -= callback, + ApplyDefaultDirectoryRetention); - internal static bool IsRecording => _session != null && _session.Snapshot().IsRecording; + internal static bool IsRecording => ServiceValue.IsRecording; internal static void InitializeForEditorStartup() { - EditorApplication.playModeStateChanged -= OnPlayModeStateChanged; - EditorApplication.playModeStateChanged += OnPlayModeStateChanged; - AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload; - AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload; - EditorApplication.quitting -= OnEditorQuitting; - EditorApplication.quitting += OnEditorQuitting; + EditorApplication.playModeStateChanged -= ServiceValue.OnPlayModeStateChanged; + EditorApplication.playModeStateChanged += ServiceValue.OnPlayModeStateChanged; + AssemblyReloadEvents.beforeAssemblyReload -= ServiceValue.OnBeforeAssemblyReload; + AssemblyReloadEvents.beforeAssemblyReload += ServiceValue.OnBeforeAssemblyReload; + EditorApplication.quitting -= ServiceValue.OnEditorQuitting; + EditorApplication.quitting += ServiceValue.OnEditorQuitting; } internal static VideoRecordingSnapshot Start( @@ -39,117 +46,26 @@ internal static VideoRecordingSnapshot Start( bool stopOnPlayModeExit, RecordVideoQuality quality) { - Debug.Assert(!IsRecording, "Start must not run while a recording is already active."); - Debug.Assert(!string.IsNullOrEmpty(outputPath), "outputPath must not be empty."); - Debug.Assert(width > 0, "encoder width must be a positive even size."); - Debug.Assert(height > 0, "encoder height must be a positive even size."); - Debug.Assert((width & 1) == 0, "encoder width must be even."); - Debug.Assert((height & 1) == 0, "encoder height must be even."); - - string directory = Path.GetDirectoryName(outputPath); - Debug.Assert(!string.IsNullOrEmpty(directory), "outputPath must include a directory."); - Directory.CreateDirectory(directory); - - bool useVp8 = string.Equals( - Path.GetExtension(outputPath), - RecordVideoConstants.WebmExtension, - StringComparison.OrdinalIgnoreCase); - MediaEncoderVideoFrameEncoder encoder = new MediaEncoderVideoFrameEncoder( + return ServiceValue.Start( + frameRate, + maxDurationSeconds, outputPath, + usedDefaultOutputPath, width, height, - frameRate, - useVp8, + frameSource, + stopOnPlayModeExit, quality); - try - { - _session = new VideoRecordingSession( - encoder, - frameSource, - () => EditorApplication.timeSinceStartup, - frameRate, - maxDurationSeconds, - outputPath, - quality); - _usedDefaultOutputPath = usedDefaultOutputPath; - _stopOnPlayModeExit = stopOnPlayModeExit; - LastCompletedRecordingStore.Clear(); - EditorApplication.update -= OnEditorUpdate; - EditorApplication.update += OnEditorUpdate; - return _session.Snapshot(); - } - finally - { - if (_session == null) - { - encoder.Dispose(); - } - } } internal static VideoRecordingSnapshot Stop(string reason) { - if (_session == null) - { - return default; - } - - _session.Stop(reason); - return FinishStoppedSession(reason); + return ServiceValue.Stop(reason); } internal static VideoRecordingSnapshot GetSnapshot() { - if (_session == null) - { - return default; - } - - return _session.Snapshot(); - } - - private static void OnEditorUpdate() - { - if (_session == null) - { - return; - } - - _session.Tick(); - if (_session.Snapshot().IsRecording) - { - return; - } - - FinishStoppedSession(_session.Snapshot().StoppedBy); - } - - private static VideoRecordingSnapshot FinishStoppedSession(string reason) - { - EditorApplication.update -= OnEditorUpdate; - VideoRecordingSnapshot snapshot = _session.Snapshot(); - try - { - // SessionState does not survive an Editor quit, so saving there on quit is a dead write. - if (reason != RecordVideoConstants.StoppedByCli - && reason != RecordVideoConstants.StoppedByEditorQuit) - { - LastCompletedRecordingStore.Save(snapshot); - } - - if (_usedDefaultOutputPath) - { - ApplyDefaultDirectoryRetention(); - } - - return snapshot; - } - finally - { - _session = null; - _usedDefaultOutputPath = false; - _stopOnPlayModeExit = false; - } + return ServiceValue.GetSnapshot(); } private static void ApplyDefaultDirectoryRetention() @@ -161,47 +77,5 @@ private static void ApplyDefaultDirectoryRetention() OutputFileRetention.DeleteOldestBeyondLimit(directory, RecordVideoConstants.Mp4SearchPattern); OutputFileRetention.DeleteOldestBeyondLimit(directory, RecordVideoConstants.WebmSearchPattern); } - - private static void OnPlayModeStateChanged(PlayModeStateChange state) - { - if (state != PlayModeStateChange.ExitingPlayMode) - { - return; - } - - if (_session == null) - { - return; - } - - // A window recording is independent of Play Mode, so only a Game View recording - // stops when Play Mode ends. - if (!_stopOnPlayModeExit) - { - return; - } - - Stop(RecordVideoConstants.StoppedByPlayModeExit); - } - - private static void OnBeforeAssemblyReload() - { - if (_session == null) - { - return; - } - - Stop(RecordVideoConstants.StoppedByAssemblyReload); - } - - private static void OnEditorQuitting() - { - if (_session == null) - { - return; - } - - Stop(RecordVideoConstants.StoppedByEditorQuit); - } } } diff --git a/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoSessionHost.cs b/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoSessionHost.cs new file mode 100644 index 0000000000..11a7a89cad --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoSessionHost.cs @@ -0,0 +1,212 @@ +using System; +using System.IO; +using UnityEditor; +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Owns the single active video recording and its Editor lifecycle reactions. + /// The encoder, clock, update subscription, and default-folder retention are injected + /// so the lifecycle can be tested without encoding video or registering real Editor callbacks. + /// + internal sealed class RecordVideoSessionHost + { + private readonly Func _createEncoder; + private readonly Func _clock; + private readonly Action _subscribeUpdate; + private readonly Action _unsubscribeUpdate; + private readonly Action _applyDefaultDirectoryRetention; + private VideoRecordingSession _session; + private bool _usedDefaultOutputPath; + private bool _stopOnPlayModeExit; + + internal RecordVideoSessionHost( + Func createEncoder, + Func clock, + Action subscribeUpdate, + Action unsubscribeUpdate, + Action applyDefaultDirectoryRetention) + { + Debug.Assert(createEncoder != null, "createEncoder must not be null"); + Debug.Assert(clock != null, "clock must not be null"); + Debug.Assert(subscribeUpdate != null, "subscribeUpdate must not be null"); + Debug.Assert(unsubscribeUpdate != null, "unsubscribeUpdate must not be null"); + Debug.Assert(applyDefaultDirectoryRetention != null, "applyDefaultDirectoryRetention must not be null"); + + _createEncoder = createEncoder; + _clock = clock; + _subscribeUpdate = subscribeUpdate; + _unsubscribeUpdate = unsubscribeUpdate; + _applyDefaultDirectoryRetention = applyDefaultDirectoryRetention; + } + + internal bool IsRecording => _session != null && _session.Snapshot().IsRecording; + + internal VideoRecordingSnapshot Start( + int frameRate, + int maxDurationSeconds, + string outputPath, + bool usedDefaultOutputPath, + int width, + int height, + IGameViewFrameSource frameSource, + bool stopOnPlayModeExit, + RecordVideoQuality quality) + { + Debug.Assert(!IsRecording, "Start must not run while a recording is already active."); + Debug.Assert(!string.IsNullOrEmpty(outputPath), "outputPath must not be empty."); + Debug.Assert(width > 0, "encoder width must be a positive even size."); + Debug.Assert(height > 0, "encoder height must be a positive even size."); + Debug.Assert((width & 1) == 0, "encoder width must be even."); + Debug.Assert((height & 1) == 0, "encoder height must be even."); + + string directory = Path.GetDirectoryName(outputPath); + Debug.Assert(!string.IsNullOrEmpty(directory), "outputPath must include a directory."); + Directory.CreateDirectory(directory); + + bool useVp8 = string.Equals( + Path.GetExtension(outputPath), + RecordVideoConstants.WebmExtension, + StringComparison.OrdinalIgnoreCase); + IVideoFrameEncoder encoder = _createEncoder( + outputPath, + width, + height, + frameRate, + useVp8, + quality); + try + { + _session = new VideoRecordingSession( + encoder, + frameSource, + _clock, + frameRate, + maxDurationSeconds, + outputPath, + quality); + _usedDefaultOutputPath = usedDefaultOutputPath; + _stopOnPlayModeExit = stopOnPlayModeExit; + LastCompletedRecordingStore.Clear(); + _unsubscribeUpdate(OnEditorUpdate); + _subscribeUpdate(OnEditorUpdate); + return _session.Snapshot(); + } + finally + { + if (_session == null) + { + encoder.Dispose(); + } + } + } + + internal VideoRecordingSnapshot Stop(string reason) + { + if (_session == null) + { + return default; + } + + _session.Stop(reason); + return FinishStoppedSession(reason); + } + + internal VideoRecordingSnapshot GetSnapshot() + { + if (_session == null) + { + return default; + } + + return _session.Snapshot(); + } + + internal void OnEditorUpdate() + { + if (_session == null) + { + return; + } + + _session.Tick(); + if (_session.Snapshot().IsRecording) + { + return; + } + + FinishStoppedSession(_session.Snapshot().StoppedBy); + } + + private VideoRecordingSnapshot FinishStoppedSession(string reason) + { + _unsubscribeUpdate(OnEditorUpdate); + VideoRecordingSnapshot snapshot = _session.Snapshot(); + try + { + // SessionState does not survive an Editor quit, so saving there on quit is a dead write. + if (reason != RecordVideoConstants.StoppedByCli + && reason != RecordVideoConstants.StoppedByEditorQuit) + { + LastCompletedRecordingStore.Save(snapshot); + } + + if (_usedDefaultOutputPath) + { + _applyDefaultDirectoryRetention(); + } + + return snapshot; + } + finally + { + _session = null; + _usedDefaultOutputPath = false; + _stopOnPlayModeExit = false; + } + } + + internal void OnPlayModeStateChanged(PlayModeStateChange state) + { + if (state != PlayModeStateChange.ExitingPlayMode) + { + return; + } + + if (_session == null) + { + return; + } + + // A window recording is independent of Play Mode, so only a Game View recording + // stops when Play Mode ends. + if (!_stopOnPlayModeExit) + { + return; + } + + Stop(RecordVideoConstants.StoppedByPlayModeExit); + } + + internal void OnBeforeAssemblyReload() + { + if (_session == null) + { + return; + } + + Stop(RecordVideoConstants.StoppedByAssemblyReload); + } + + internal void OnEditorQuitting() + { + if (_session == null) + { + return; + } + + Stop(RecordVideoConstants.StoppedByEditorQuit); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoSessionHost.cs.meta b/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoSessionHost.cs.meta new file mode 100644 index 0000000000..2e2ad729ea --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/RecordVideo/RecordVideoSessionHost.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2b9163c69ba9c4a2dbec30516d383e78 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs index 277a39009f..d73d6795cc 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs @@ -18,10 +18,14 @@ public class ScreenshotUseCase private const int ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES = 2; private readonly IScreenshotEditorStateReader _editorStateReader; + private readonly IEditorWindowCaptureService _windowCaptureService; - internal ScreenshotUseCase(IScreenshotEditorStateReader editorStateReader = null) + internal ScreenshotUseCase( + IScreenshotEditorStateReader editorStateReader = null, + IEditorWindowCaptureService windowCaptureService = null) { _editorStateReader = editorStateReader ?? new ScreenshotEditorStateReader(); + _windowCaptureService = windowCaptureService ?? new EditorWindowCaptureService(); } public async Task CaptureAsync( @@ -416,7 +420,7 @@ private async Task CaptureWindowsAsync( { SynchronizationContext editorContext = CapturedEditorSynchronizationContext.RequireCurrent("window screenshot use case"); - EditorWindow[] windows = EditorWindowCaptureUtility.FindWindowsByName(request.WindowName, request.MatchMode); + EditorWindow[] windows = _windowCaptureService.FindWindowsByName(request.WindowName, request.MatchMode); string captureWindowName = ScreenshotWindowNameResolver.ResolveCaptureWindowName( request.WindowName, request.MatchMode, @@ -428,7 +432,7 @@ private async Task CaptureWindowsAsync( if (usedSimulatorFallback) { // why: Device Simulator replaces the Game tab, so the default "Game" title miss should retry Simulator - windows = EditorWindowCaptureUtility.FindWindowsByName(captureWindowName, request.MatchMode); + windows = _windowCaptureService.FindWindowsByName(captureWindowName, request.MatchMode); if (windows.Length > 0) { VibeLogger.LogInfo( @@ -498,7 +502,7 @@ private async Task CaptureWindowsAsync( for (int i = 0; i < windows.Length; i++) { EditorWindow window = windows[i]; - (Texture2D texture, bool timedOut) = await EditorWindowCaptureUtility.CaptureWindowAsync( + (Texture2D texture, bool timedOut) = await _windowCaptureService.CaptureWindowAsync( window, request.ResolutionScale, UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index c0ab2c177a..84ac3defa5 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -37,6 +37,7 @@ public sealed class CliInstallationDetector : ICliInstallationDetector private string _cachedCliExecutablePath; private bool _cacheInitialized; private bool _isRefreshing; + private Func> _detect = DetectCliInstallationAsync; public CliInstallationDetector(ICliPinReader cliPinReader) { @@ -45,6 +46,14 @@ public CliInstallationDetector(ICliPinReader cliPinReader) _cliPinReader = cliPinReader ?? throw new ArgumentNullException(nameof(cliPinReader)); } + // Replaces the process-backed detection so tests can drive the cache without running the CLI. + internal void SetDetectionForTesting(Func> detect) + { + UnityEngine.Debug.Assert(detect != null, "detect must not be null"); + + _detect = detect; + } + public bool IsCliInstalled() { return GetCachedCliVersion() != null; @@ -80,7 +89,7 @@ public async Task RefreshCliVersionAsync(CancellationToken ct) _isRefreshing = true; try { - CliInstallationDetection detection = await DetectCliInstallationAsync(ct); + CliInstallationDetection detection = await _detect(ct); _cachedCliVersion = detection.Version; _cachedCliIsDispatcher = detection.IsDispatcher; _cachedCliExecutablePath = detection.ExecutablePath; @@ -94,7 +103,7 @@ public async Task RefreshCliVersionAsync(CancellationToken ct) public async Task ForceRefreshCliVersionAsync(CancellationToken ct) { - CliInstallationDetection detection = await DetectCliInstallationAsync(ct); + CliInstallationDetection detection = await _detect(ct); _cachedCliVersion = detection.Version; _cachedCliIsDispatcher = detection.IsDispatcher; _cachedCliExecutablePath = detection.ExecutablePath; @@ -167,7 +176,7 @@ private static CliInstallationDetection DetectPackageOwnedCliInstallationBlockin return new CliInstallationDetection(null, executablePath); } - return DetectCliInstallationAtExecutablePath(executablePath, ct); + return DetectCliInstallationAtExecutablePath(executablePath, ct, CliDetectionCommandRunner.Execute); } private static CliInstallationDetection DetectShellCliInstallationBlocking(RuntimePlatform platform, CancellationToken ct) @@ -180,7 +189,7 @@ private static CliInstallationDetection DetectShellCliInstallationBlocking(Runti string executablePath = NodeEnvironmentResolver.FindExecutablePathAtPlatform( CliConstants.EXECUTABLE_NAME, platform); - return DetectCliInstallationAtExecutablePath(executablePath, ct); + return DetectCliInstallationAtExecutablePath(executablePath, ct, CliDetectionCommandRunner.Execute); } private static CliInstallationDetection DetectShellCliInstallationFromLoginShell(RuntimePlatform platform, CancellationToken ct) @@ -200,28 +209,36 @@ private static CliInstallationDetection DetectShellCliInstallationFromLoginShell return CliShellInstallationProbe.ParseShellCliInstallationOutput(output); } - private static CliInstallationDetection DetectCliInstallationAtExecutablePath( + internal static CliInstallationDetection DetectCliInstallationAtExecutablePath( string executablePath, - CancellationToken ct) + CancellationToken ct, + Func runCommand) { string fileName = executablePath ?? CliConstants.EXECUTABLE_NAME; - string contractOutput = ExecuteCliVersionCommand(fileName, CliConstants.VERSION_FLAG + " " + CliConstants.JSON_FLAG, ct); + string contractOutput = ExecuteCliVersionCommand( + fileName, + CliConstants.VERSION_FLAG + " " + CliConstants.JSON_FLAG, + ct, + runCommand); CliInstallationDetection contractDetection = CliShellInstallationProbe.ParseCliContractOutput(contractOutput, executablePath); if (!string.IsNullOrEmpty(contractDetection.Version)) { return contractDetection; } - string versionOutput = ExecuteCliVersionCommand(fileName, CliConstants.VERSION_FLAG, ct); + string versionOutput = ExecuteCliVersionCommand(fileName, CliConstants.VERSION_FLAG, ct, runCommand); string version = string.IsNullOrEmpty(versionOutput) ? null : versionOutput; return new CliInstallationDetection(version, executablePath); } - private static string ExecuteCliVersionCommand( + internal static string ExecuteCliVersionCommand( string fileName, string arguments, - CancellationToken ct) + CancellationToken ct, + Func runCommand) { + UnityEngine.Debug.Assert(runCommand != null, "runCommand must not be null"); + ProcessStartInfo startInfo = new() { FileName = fileName, @@ -234,7 +251,7 @@ private static string ExecuteCliVersionCommand( try { - CliDetectionCommandResult commandResult = CliDetectionCommandRunner.Execute(startInfo, ct); + CliDetectionCommandResult commandResult = runCommand(startInfo, ct); if (commandResult == null) { return null; diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs index 9800e10525..8422ff58d7 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationCrossFileTimingMigrationPlanner.cs @@ -75,6 +75,8 @@ internal static int ApplyCrossFilePlayerLoopTimingCallerArgumentMigrations( return 0; } + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex = + BuildTypeHierarchyIndex(csharpFilePaths, changes, readAllText); int replacementCount = 0; while (HasRemovedPlayerLoopTimingSignatures(activeRemovedSignaturesByAssemblyDirectory)) { @@ -110,7 +112,8 @@ internal static int ApplyCrossFilePlayerLoopTimingCallerArgumentMigrations( source, originalSource, activeRemovedSignatures.ToArray(), - legacyAssemblyAliases); + legacyAssemblyAliases, + typeHierarchyIndex); if (!callerResult.Changed) { continue; @@ -152,6 +155,23 @@ internal static int ApplyCrossFilePlayerLoopTimingCallerArgumentMigrations( return replacementCount; } + // Built once from the pending content so it sees the same version of each file as the caller rewrite does. + // Later rounds change only argument and parameter text, never class names, base lists, namespaces, usings, + // or member names, so the index is not rebuilt between rounds. + private static ThirdPartyToolMigrationTypeHierarchyIndex BuildTypeHierarchyIndex( + List csharpFilePaths, + List changes, + Func readAllText) + { + List sources = new(csharpFilePaths.Count); + foreach (string csharpFilePath in csharpFilePaths) + { + sources.Add(GetPendingMigrationFileContent(csharpFilePath, changes, readAllText(csharpFilePath))); + } + + return ThirdPartyToolMigrationTypeHierarchyIndex.Build(sources); + } + internal static bool CanMigrateBareLegacyPlayerLoopTimingForAssembly( MigrationAssemblyUsage assemblyUsage, string assemblyDirectory, diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRules.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRules.cs index 313c832abb..4154771378 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRules.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationRules.cs @@ -338,13 +338,15 @@ internal static ThirdPartyToolMigrationContentResult RemoveLegacyPlayerLoopTimin string source, string originalSource, RemovedLegacyPlayerLoopTimingSignature[] removedSignatures, - string[] legacyAssemblyAliases) + string[] legacyAssemblyAliases, + ThirdPartyToolMigrationTypeHierarchyIndex typeHierarchyIndex) { return ThirdPartyToolMigrationTimingCallerRules.RemoveLegacyPlayerLoopTimingCallerArgumentsForLegacyAssembly( source, originalSource, removedSignatures, - legacyAssemblyAliases); + legacyAssemblyAliases, + typeHierarchyIndex); } internal static ThirdPartyToolMigrationContentResult RemoveLegacyPlayerLoopTimingParametersForLegacyAssembly( diff --git a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs index d3894afe97..c533f34ee5 100644 --- a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs +++ b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs @@ -23,6 +23,8 @@ public class UnityCliLoopBridgeServer : IUnityCliLoopServerInstance public event Action ServerLoopExited; private readonly IDomainReloadDetectionService _domainReloadDetectionService; private readonly UnityCliLoopBridgeClientSessionManager _clientSessionManager; + private readonly Func _createListener; + private readonly Func> _acceptClient; private IBridgeTransportListener _transportListener; private CancellationTokenSource _cancellationTokenSource; @@ -37,7 +39,9 @@ internal UnityCliLoopBridgeServer( IDomainReloadDetectionService domainReloadDetectionService, JsonRpcRequestProcessor jsonRpcRequestProcessor, UnityCliLoopBridgeHeartbeatService heartbeatService, - UnityCliLoopBridgeClientDisconnectMonitor clientDisconnectMonitor) + UnityCliLoopBridgeClientDisconnectMonitor clientDisconnectMonitor, + Func createListener = null, + Func> acceptClient = null) { System.Diagnostics.Debug.Assert(domainReloadDetectionService != null, "domainReloadDetectionService must not be null"); System.Diagnostics.Debug.Assert(jsonRpcRequestProcessor != null, "jsonRpcRequestProcessor must not be null"); @@ -56,6 +60,8 @@ internal UnityCliLoopBridgeServer( validatedJsonRpcRequestProcessor, validatedHeartbeatService, validatedClientDisconnectMonitor); + _createListener = createListener ?? BridgeTransportListenerFactory.Create; + _acceptClient = acceptClient ?? AcceptClientAsync; } /// @@ -78,7 +84,7 @@ public void StartServer() try { - _transportListener = BridgeTransportListenerFactory.Create(endpoint); + _transportListener = _createListener(endpoint); _transportListener.Start(); _isRunning = true; @@ -148,6 +154,27 @@ public void StopServer() TimeSpan.FromSeconds(UnityCliLoopServerConfig.SHUTDOWN_TIMEOUT_SECONDS)).Forget(); } + /// + /// Puts the server into the running state with the given listener without starting the accept loop, + /// so tests can drive ServerLoopAsync and the stop paths without binding an endpoint or using the thread pool. + /// + internal void AttachListenerForTesting(IBridgeTransportListener listener) + { + System.Diagnostics.Debug.Assert(listener != null, "listener must not be null"); + + IBridgeTransportListener validatedListener = listener ?? throw new ArgumentNullException(nameof(listener)); + // Why: attaching over a running server would drop the live listener and token source without stopping them. + if (_isRunning) + { + throw new InvalidOperationException("A listener cannot be attached while the server is running."); + } + + _transportListener = validatedListener; + _cancellationTokenSource = new CancellationTokenSource(); + _unexpectedExitCleanupStarted = 0; + _isRunning = true; + } + private CancellationTokenSource TakeCancellationTokenSource() { return Interlocked.Exchange(ref _cancellationTokenSource, null); @@ -231,7 +258,7 @@ private void CleanupAfterUnexpectedLoopExit() /// /// The server's main loop. /// - private async Task ServerLoopAsync(CancellationToken cancellationToken) + internal async Task ServerLoopAsync(CancellationToken cancellationToken) { try { @@ -239,7 +266,7 @@ private async Task ServerLoopAsync(CancellationToken cancellationToken) { try { - BridgeClientConnection client = await AcceptClientAsync(_transportListener, cancellationToken); + BridgeClientConnection client = await _acceptClient(_transportListener, cancellationToken); if (client != null) { _clientSessionManager.StartClientHandler(client, cancellationToken); diff --git a/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs b/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs index 4fb85acdb2..e964b37d93 100644 --- a/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs +++ b/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs @@ -58,11 +58,18 @@ private static string TryWhichCommand(string executableName) /// /// Finds the first executable path for the given name using the Windows 'where' command. - /// Prioritizes .cmd/.exe over extensionless entries because native Windows shims must be launched directly. /// private static string TryWhereCommand(string executableName) { - string[] paths = TryWhereCommandAll(executableName); + return SelectWindowsExecutable(TryWhereCommandAll(executableName)); + } + + /// + /// Picks the path to launch from 'where' results. + /// Prioritizes .cmd/.exe over extensionless entries because native Windows shims must be launched directly. + /// + internal static string SelectWindowsExecutable(string[] paths) + { if (paths == null || paths.Length == 0) { return null; @@ -92,7 +99,12 @@ private static string[] TryWhereCommandAll(string executableName) CreateNoWindow = true }; - string output = ExecuteAndGetOutput(startInfo); + return ParseWhereOutput(ExecuteAndGetOutput(startInfo)); + } + + // Splits on LF and trims each line so CRLF output from cmd.exe leaves no carriage returns behind. + internal static string[] ParseWhereOutput(string output) + { if (!string.IsNullOrEmpty(output)) { string[] lines = output.Split('\n'); @@ -300,7 +312,7 @@ private static string TryReadDirectoryServiceUserShell(string userName) return ExtractDirectoryServiceUserShell(ExecuteAndGetOutput(startInfo)); } - private static bool IsSafeDirectoryServiceUserName(string userName) + internal static bool IsSafeDirectoryServiceUserName(string userName) { if (string.IsNullOrEmpty(userName)) { diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.cs index 3344368caa..7efadb84f8 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.cs @@ -2,7 +2,6 @@ using System.Threading; using System.Threading.Tasks; -using UnityEditor; using UnityEngine; using UnityEngine.UIElements; @@ -21,6 +20,7 @@ internal sealed class SetupWizardCliWorkflowController private readonly CliInstallProgressView _installProgressView; private readonly CliSetupApplicationService _cliSetupApplicationService; private readonly Action _refreshUi; + private readonly IPresentationDialogs _dialogs; private bool _isInstallingCli; private bool _needsCliPathSetup; @@ -33,7 +33,8 @@ internal SetupWizardCliWorkflowController( VisualElement installProgressContainer, Label installProgressLabel, CliSetupApplicationService cliSetupApplicationService, - Action refreshUi) + Action refreshUi, + IPresentationDialogs dialogs = null) { Debug.Assert(cliSetupApplicationService != null, "cliSetupApplicationService must not be null"); Debug.Assert(refreshUi != null, "refreshUi must not be null"); @@ -42,6 +43,7 @@ internal SetupWizardCliWorkflowController( ?? throw new ArgumentNullException(nameof(cliSetupApplicationService)); _refreshUi = refreshUi ?? throw new ArgumentNullException(nameof(refreshUi)); + _dialogs = dialogs ?? new EditorPresentationDialogs(); _installProgressView = new CliInstallProgressView( installProgressContainer, installCliButton, @@ -101,7 +103,7 @@ private void HandleInstallCli() HandleInstallCliAsync(CancellationToken.None).Forget(); } - private async Task HandleInstallCliAsync(CancellationToken ct) + internal async Task HandleInstallCliAsync(CancellationToken ct) { await RefreshCliPrimaryActionStateAsync(ct); @@ -162,15 +164,14 @@ private async Task HandleInstallCliAsync(CancellationToken ct) string manualInstallGuidance = commandResult.Success ? commandResult.Command.ManualCommand : commandResult.ErrorOutput; - EditorUtility.DisplayDialog( + _dialogs.ShowMessage( "Installation Failed", $"Failed to install uloop CLI.\n\n{result.ErrorOutput}\n\n" - + manualInstallGuidance, - "OK"); + + manualInstallGuidance); return; } - await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + await _dialogs.EnsureCliVisibleAndShowResultAsync( UnityEngine.Application.platform, _cliSetupApplicationService, ct); @@ -220,7 +221,7 @@ private async Task HandleRepairCliPathSetup(CancellationToken ct) try { - await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + await _dialogs.EnsureCliVisibleAndShowResultAsync( UnityEngine.Application.platform, _cliSetupApplicationService, ct); diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardSkillsWorkflowController.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardSkillsWorkflowController.cs index 573eca9521..523f1af18b 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardSkillsWorkflowController.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardSkillsWorkflowController.cs @@ -3,7 +3,6 @@ using System.Threading; using System.Threading.Tasks; -using UnityEditor; using UnityEngine; using UnityEngine.UIElements; @@ -23,6 +22,8 @@ internal sealed class SetupWizardSkillsWorkflowController private readonly IUnityCliLoopEditorSettingsPort _editorSettingsPort; private readonly CliSetupApplicationService _cliSetupApplicationService; private readonly Action _scheduleResizeToContent; + private readonly IPresentationDialogs _dialogs; + private readonly IBackgroundWorkRunner _backgroundWorkRunner; private bool _isInstallingSkills; private bool _installSkillsFlat; @@ -35,7 +36,9 @@ internal SetupWizardSkillsWorkflowController( SkillSetupUseCase skillSetupUseCase, IUnityCliLoopEditorSettingsPort editorSettingsPort, CliSetupApplicationService cliSetupApplicationService, - Action scheduleResizeToContent) + Action scheduleResizeToContent, + IPresentationDialogs dialogs = null, + IBackgroundWorkRunner backgroundWorkRunner = null) { Debug.Assert(skillsSetupPanelView != null, "skillsSetupPanelView must not be null"); Debug.Assert(skillSetupUseCase != null, "skillSetupUseCase must not be null"); @@ -53,6 +56,8 @@ internal SetupWizardSkillsWorkflowController( ?? throw new ArgumentNullException(nameof(cliSetupApplicationService)); _scheduleResizeToContent = scheduleResizeToContent ?? throw new ArgumentNullException(nameof(scheduleResizeToContent)); + _dialogs = dialogs ?? new EditorPresentationDialogs(); + _backgroundWorkRunner = backgroundWorkRunner ?? new ThreadPoolBackgroundWorkRunner(); _skillsSetupPanelView.OnInstallAllClicked += HandleInstallAllSkills; _skillsSetupPanelView.OnInstallSelectedClicked += HandleInstallSelectedSkills; @@ -137,7 +142,7 @@ private async Task RefreshDisplayedSkillTargetsAsync(CancellationToken ct) { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); List targets = - await Task.Run(() => DetectDisplayedSkillTargets(projectRoot)); + await _backgroundWorkRunner.RunAsync(() => DetectDisplayedSkillTargets(projectRoot)); if (ct.IsCancellationRequested) { return; @@ -185,7 +190,7 @@ private void HandleInstallSelectedSkills() HandleInstallSkillsAsync(isBulkInstall: false, CancellationToken.None).Forget(); } - private async Task HandleInstallSkillsAsync(bool isBulkInstall, CancellationToken ct) + internal async Task HandleInstallSkillsAsync(bool isBulkInstall, CancellationToken ct) { if (_isInstallingSkills) { @@ -201,7 +206,7 @@ private async Task HandleInstallSkillsAsync(bool isBulkInstall, CancellationToke { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); List targets = - await Task.Run(() => DetectDisplayedSkillTargets(projectRoot)); + await _backgroundWorkRunner.RunAsync(() => DetectDisplayedSkillTargets(projectRoot)); if (ct.IsCancellationRequested) { return; @@ -229,7 +234,7 @@ await _skillSetupUseCase.InstallSkillFilesAsync( ct); if (shouldShowSkillsInstalledDialog) { - EditorDialogHelper.ShowSkillsInstalledDialog(); + _dialogs.ShowSkillsInstalled(); } } finally @@ -239,7 +244,7 @@ await _skillSetupUseCase.InstallSkillFilesAsync( } } - private void HandleTargetChanged(SkillsTarget newTarget) + internal void HandleTargetChanged(SkillsTarget newTarget) { _skillsTarget = newTarget; string cachedCliVersion = _cliSetupApplicationService.GetCachedCliVersion(); @@ -248,7 +253,7 @@ private void HandleTargetChanged(SkillsTarget newTarget) _scheduleResizeToContent(); } - private void HandleGroupSkillsChanged(bool _) + internal void HandleGroupSkillsChanged(bool _) { ApplyFlatSkillInstallPreference(); RefreshSkillsSection(); diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs index ba0e4d69c9..1e9995a0f5 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs @@ -40,6 +40,7 @@ internal sealed class SetupWizardStartupFlow private readonly ThirdPartyToolMigrationUseCase _thirdPartyToolMigrationUseCase; private readonly System.Action _showWindowOnVersionChange; private readonly System.Action _showThirdPartyMigrationAutoScan; + private readonly IBackgroundWorkRunner _backgroundWorkRunner; private bool _migrationAutoScanPollingActive; private double _migrationAutoScanPollStartTime; @@ -52,7 +53,8 @@ internal SetupWizardStartupFlow( SkillSetupUseCase skillSetupUseCase, ThirdPartyToolMigrationUseCase thirdPartyToolMigrationUseCase, System.Action showWindowOnVersionChange, - System.Action showThirdPartyMigrationAutoScan) + System.Action showThirdPartyMigrationAutoScan, + IBackgroundWorkRunner backgroundWorkRunner = null) { Debug.Assert(editorSettingsPort != null, "editorSettingsPort must not be null"); Debug.Assert(projectSettingsPort != null, "projectSettingsPort must not be null"); @@ -83,6 +85,7 @@ internal SetupWizardStartupFlow( ?? throw new System.ArgumentNullException(nameof(showWindowOnVersionChange)); _showThirdPartyMigrationAutoScan = showThirdPartyMigrationAutoScan ?? throw new System.ArgumentNullException(nameof(showThirdPartyMigrationAutoScan)); + _backgroundWorkRunner = backgroundWorkRunner ?? new ThreadPoolBackgroundWorkRunner(); } /// @@ -345,11 +348,12 @@ private CliSetupCompatibilityState EvaluateCliSetupCompatibility( private async Task HasSkillUpdateForSetupWizardAsync(CancellationToken ct) { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); - List targets = await Task.Run( + // The runner takes no token, so cancellation is checked after the scan; the only caller passes + // CancellationToken.None, so the scan never had a canceled start to skip. + List targets = await _backgroundWorkRunner.RunAsync( () => _skillSetupUseCase.DetectSkillTargetsForLayoutAtProjectRoot( projectRoot, - !SetupWizardWindow.ForceFlatSkillInstall), - ct); + !SetupWizardWindow.ForceFlatSkillInstall)); if (ct.IsCancellationRequested) { return false; @@ -389,7 +393,11 @@ private void PollThirdPartyToolMigrationAutoScan() EditorUtility.scriptCompilationFailed, elapsedSeconds, SetupWizardStartupFlowConstants.MigrationAutoScanPollTimeoutSeconds); + ApplyMigrationAutoScanPollAction(action); + } + internal void ApplyMigrationAutoScanPollAction(MigrationAutoScanPollAction action) + { switch (action) { case MigrationAutoScanPollAction.ContinueWaiting: @@ -460,7 +468,7 @@ private void ScheduleThirdPartyToolMigrationFallbackFullScan() RunThirdPartyToolMigrationFallbackFullScanAsync(projectRoot).Forget(); } - private async Task RunThirdPartyToolMigrationFallbackFullScanAsync(string projectRoot) + internal async Task RunThirdPartyToolMigrationFallbackFullScanAsync(string projectRoot) { bool hasTargets = await _thirdPartyToolMigrationUseCase.HasMigrationTargetsAsync( projectRoot, diff --git a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWorkflowController.cs b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWorkflowController.cs index ecb04e9193..ad279fdcb9 100644 --- a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWorkflowController.cs +++ b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWorkflowController.cs @@ -23,6 +23,8 @@ internal sealed class ThirdPartyToolMigrationWizardWorkflowController private readonly SkillSetupUseCase _skillSetupUseCase; private readonly ThirdPartyToolMigrationUseCase _thirdPartyToolMigrationUseCase; private readonly Action _scheduleResize; + private readonly IPresentationDialogs _dialogs; + private readonly IBackgroundWorkRunner _backgroundWorkRunner; // Auto-scan seed files (compile-error-matched migration targets) are only used to render the // initial detected-state list without scanning; RefreshUI (manual Check / re-check) always @@ -46,7 +48,9 @@ internal ThirdPartyToolMigrationWizardWorkflowController( SkillSetupUseCase skillSetupUseCase, ThirdPartyToolMigrationUseCase thirdPartyToolMigrationUseCase, List autoScanSeedFilePaths, - Action scheduleResize) + Action scheduleResize, + IPresentationDialogs dialogs = null, + IBackgroundWorkRunner backgroundWorkRunner = null) { Debug.Assert(view != null, "view must not be null"); Debug.Assert(skillSetupUseCase != null, "skillSetupUseCase must not be null"); @@ -65,6 +69,8 @@ internal ThirdPartyToolMigrationWizardWorkflowController( ?? throw new ArgumentNullException(nameof(autoScanSeedFilePaths)); _scheduleResize = scheduleResize ?? throw new ArgumentNullException(nameof(scheduleResize)); + _dialogs = dialogs ?? new EditorPresentationDialogs(); + _backgroundWorkRunner = backgroundWorkRunner ?? new ThreadPoolBackgroundWorkRunner(); } internal void ShowInitialState(bool shouldShowAutoScanDetectedState) @@ -115,7 +121,7 @@ internal async Task RefreshUI() ThirdPartyToolMigrationPreview preview; try { - preview = await Task.Run(async () => + preview = await _backgroundWorkRunner.RunTaskAsync(async () => await _thirdPartyToolMigrationUseCase.PreviewMigrationAsync(projectRoot, progress, ct)); await MainThreadSwitcher.SwitchToMainThread(); } @@ -160,7 +166,7 @@ internal async Task HandleMigrateThirdPartyTools() _pendingMigrationFilePaths.Length); if (!ThirdPartyToolMigrationWizardWindow.ConfirmMigrationApply( confirmDialogFileCount, - (title, message, ok, cancel) => EditorUtility.DisplayDialog(title, message, ok, cancel))) + _dialogs.Confirm)) { return; } @@ -176,7 +182,7 @@ internal async Task HandleMigrateThirdPartyTools() try { IProgress progress = CreateProgressReporter(ct); - result = await Task.Run(async () => + result = await _backgroundWorkRunner.RunTaskAsync(async () => await _thirdPartyToolMigrationUseCase.ApplyMigrationAsync(projectRoot, progress, ct)); if (!ThirdPartyToolMigrationWizardWindow.ShouldFinishMigrationOnMainThread( ct.IsCancellationRequested, diff --git a/Packages/src/Editor/Presentation/Shared/EditorPresentationDialogs.cs b/Packages/src/Editor/Presentation/Shared/EditorPresentationDialogs.cs new file mode 100644 index 0000000000..7dc199e8ff --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/EditorPresentationDialogs.cs @@ -0,0 +1,45 @@ +using System.Threading; +using System.Threading.Tasks; + +using UnityEditor; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Presentation +{ + /// + /// Opens the real editor dialogs for the settings window and setup wizard workflows. + /// + internal sealed class EditorPresentationDialogs : IPresentationDialogs + { + public void ShowMessage(string title, string message) + { + EditorUtility.DisplayDialog(title, message, "OK"); + } + + public bool Confirm(string title, string message, string ok, string cancel) + { + return EditorUtility.DisplayDialog(title, message, ok, cancel); + } + + public void ShowSkillsInstalled() + { + EditorDialogHelper.ShowSkillsInstalledDialog(); + } + + public bool ConfirmCliUninstall() + { + return CliUninstallPrompt.ConfirmUninstall(); + } + + public Task EnsureCliVisibleAndShowResultAsync( + RuntimePlatform platform, + CliSetupApplicationService cliSetupApplicationService, + CancellationToken ct) + { + return CliPathSetupPrompt.EnsureVisibleAndShowResultAsync(platform, cliSetupApplicationService, ct); + } + } +} diff --git a/Packages/src/Editor/Presentation/Shared/EditorPresentationDialogs.cs.meta b/Packages/src/Editor/Presentation/Shared/EditorPresentationDialogs.cs.meta new file mode 100644 index 0000000000..19d99d1fe9 --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/EditorPresentationDialogs.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 66bf33431a7c746daa974c0ebbc2ef42 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Shared/IBackgroundWorkRunner.cs b/Packages/src/Editor/Presentation/Shared/IBackgroundWorkRunner.cs new file mode 100644 index 0000000000..c5da6d2cac --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/IBackgroundWorkRunner.cs @@ -0,0 +1,15 @@ +using System; +using System.Threading.Tasks; + +namespace io.github.hatayama.UnityCliLoop.Presentation +{ + /// + /// Runs blocking presentation work off the editor main thread. + /// + internal interface IBackgroundWorkRunner + { + Task RunAsync(Func work); + + Task RunTaskAsync(Func> work); + } +} diff --git a/Packages/src/Editor/Presentation/Shared/IBackgroundWorkRunner.cs.meta b/Packages/src/Editor/Presentation/Shared/IBackgroundWorkRunner.cs.meta new file mode 100644 index 0000000000..addd855b23 --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/IBackgroundWorkRunner.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 420d5d9484dae414199e2bc9450eae9b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Shared/IPresentationDialogs.cs b/Packages/src/Editor/Presentation/Shared/IPresentationDialogs.cs new file mode 100644 index 0000000000..6e7a831bb2 --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/IPresentationDialogs.cs @@ -0,0 +1,29 @@ +using System.Threading; +using System.Threading.Tasks; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Presentation +{ + /// + /// Shows the modal dialogs that the settings window and setup wizard workflows open. + /// + internal interface IPresentationDialogs + { + void ShowMessage(string title, string message); + + bool Confirm(string title, string message, string ok, string cancel); + + void ShowSkillsInstalled(); + + bool ConfirmCliUninstall(); + + Task EnsureCliVisibleAndShowResultAsync( + RuntimePlatform platform, + CliSetupApplicationService cliSetupApplicationService, + CancellationToken ct); + } +} diff --git a/Packages/src/Editor/Presentation/Shared/IPresentationDialogs.cs.meta b/Packages/src/Editor/Presentation/Shared/IPresentationDialogs.cs.meta new file mode 100644 index 0000000000..a2b3ff77d8 --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/IPresentationDialogs.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ed908e5324c74457dac23977c3d8befc +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Shared/ThreadPoolBackgroundWorkRunner.cs b/Packages/src/Editor/Presentation/Shared/ThreadPoolBackgroundWorkRunner.cs new file mode 100644 index 0000000000..41df9d0232 --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/ThreadPoolBackgroundWorkRunner.cs @@ -0,0 +1,23 @@ +using System; +using System.Threading.Tasks; + +namespace io.github.hatayama.UnityCliLoop.Presentation +{ + /// + /// Runs presentation background work on the thread pool. + /// + internal sealed class ThreadPoolBackgroundWorkRunner : IBackgroundWorkRunner + { + // No token is passed to Task.Run: callers check their token after the await and return quietly, + // whereas a cancelled Task.Run would throw and log from fire-and-forget refreshes. + public Task RunAsync(Func work) + { + return Task.Run(work); + } + + public Task RunTaskAsync(Func> work) + { + return Task.Run(work); + } + } +} diff --git a/Packages/src/Editor/Presentation/Shared/ThreadPoolBackgroundWorkRunner.cs.meta b/Packages/src/Editor/Presentation/Shared/ThreadPoolBackgroundWorkRunner.cs.meta new file mode 100644 index 0000000000..410153ffcb --- /dev/null +++ b/Packages/src/Editor/Presentation/Shared/ThreadPoolBackgroundWorkRunner.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7894ed508433b40ecbab6c15ee8d0df6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs index 8f661e68d9..9e8e05c719 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs @@ -1,7 +1,6 @@ using System; using System.Threading; using System.Threading.Tasks; -using UnityEditor; using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; @@ -18,6 +17,7 @@ internal sealed class UnityCliLoopSettingsCliSetupPresenter { private readonly UnityCliLoopSettingsWindowUI _view; private readonly CliSetupApplicationService _cliSetupApplicationService; + private readonly IPresentationDialogs _dialogs; private Func _getSkillsSnapshot; private Action _refreshSkillsInstallStateInBackground; @@ -30,7 +30,8 @@ internal sealed class UnityCliLoopSettingsCliSetupPresenter internal UnityCliLoopSettingsCliSetupPresenter( UnityCliLoopSettingsWindowUI view, - CliSetupApplicationService cliSetupApplicationService) + CliSetupApplicationService cliSetupApplicationService, + IPresentationDialogs dialogs = null) { Debug.Assert(view != null, "view must not be null"); Debug.Assert(cliSetupApplicationService != null, "cliSetupApplicationService must not be null"); @@ -38,6 +39,7 @@ internal UnityCliLoopSettingsCliSetupPresenter( _view = view ?? throw new ArgumentNullException(nameof(view)); _cliSetupApplicationService = cliSetupApplicationService ?? throw new ArgumentNullException(nameof(cliSetupApplicationService)); + _dialogs = dialogs ?? new EditorPresentationDialogs(); } internal bool IsRefreshingVersion => _isRefreshingVersion; @@ -313,14 +315,13 @@ internal async Task HandleInstallCli() string manualInstallGuidance = commandResult.Success ? commandResult.Command.ManualCommand : commandResult.ErrorOutput; - EditorUtility.DisplayDialog( + _dialogs.ShowMessage( "Installation Failed", - $"Failed to install uLoop CLI.\n\n{result.ErrorOutput}\n\n{manualInstallGuidance}", - "OK"); + $"Failed to install uLoop CLI.\n\n{result.ErrorOutput}\n\n{manualInstallGuidance}"); return; } - await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + await _dialogs.EnsureCliVisibleAndShowResultAsync( UnityEngine.Application.platform, _cliSetupApplicationService, CancellationToken.None); @@ -373,7 +374,7 @@ private async Task HandleRepairCliPathSetup() try { - await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + await _dialogs.EnsureCliVisibleAndShowResultAsync( UnityEngine.Application.platform, _cliSetupApplicationService, CancellationToken.None); @@ -388,7 +389,7 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( private async Task HandleUninstallCli() { - if (!CliUninstallPrompt.ConfirmUninstall()) + if (!_dialogs.ConfirmCliUninstall()) { return; } @@ -403,10 +404,9 @@ private async Task HandleUninstallCli() CancellationToken.None); if (!result.Success) { - EditorUtility.DisplayDialog( + _dialogs.ShowMessage( "Uninstallation Failed", - $"Failed to uninstall uloop CLI.\n\n{result.ErrorOutput}", - "OK"); + $"Failed to uninstall uloop CLI.\n\n{result.ErrorOutput}"); return; } } diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsSkillsPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsSkillsPresenter.cs index e2740f62ed..228425d19f 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsSkillsPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsSkillsPresenter.cs @@ -3,7 +3,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using UnityEditor; using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; @@ -51,6 +50,8 @@ internal sealed class UnityCliLoopSettingsSkillsPresenter private readonly SkillSetupUseCase _skillSetupUseCase; private readonly CliSetupApplicationService _cliSetupApplicationService; private readonly IUnityCliLoopEditorSettingsPort _editorSettingsPort; + private readonly IPresentationDialogs _dialogs; + private readonly IBackgroundWorkRunner _backgroundWorkRunner; private Action _refreshCliSetupSection; private Func _isRefreshingVersion; @@ -66,7 +67,9 @@ internal sealed class UnityCliLoopSettingsSkillsPresenter internal UnityCliLoopSettingsSkillsPresenter( SkillSetupUseCase skillSetupUseCase, CliSetupApplicationService cliSetupApplicationService, - IUnityCliLoopEditorSettingsPort editorSettingsPort) + IUnityCliLoopEditorSettingsPort editorSettingsPort, + IPresentationDialogs dialogs = null, + IBackgroundWorkRunner backgroundWorkRunner = null) { Debug.Assert(skillSetupUseCase != null, "skillSetupUseCase must not be null"); Debug.Assert(cliSetupApplicationService != null, "cliSetupApplicationService must not be null"); @@ -78,6 +81,8 @@ internal UnityCliLoopSettingsSkillsPresenter( ?? throw new ArgumentNullException(nameof(cliSetupApplicationService)); _editorSettingsPort = editorSettingsPort ?? throw new ArgumentNullException(nameof(editorSettingsPort)); + _dialogs = dialogs ?? new EditorPresentationDialogs(); + _backgroundWorkRunner = backgroundWorkRunner ?? new ThreadPoolBackgroundWorkRunner(); } internal void BindCoordination( @@ -200,10 +205,9 @@ internal async Task HandleInstallSkills() { if (!_cliSetupApplicationService.IsCliInstalled()) { - EditorUtility.DisplayDialog( + _dialogs.ShowMessage( "CLI Not Found", - "uloop CLI is not installed. Please install the CLI first.", - "OK"); + "uloop CLI is not installed. Please install the CLI first."); return; } @@ -235,7 +239,7 @@ await _skillSetupUseCase.InstallSkillFilesAsync( CancellationToken.None); if (shouldShowSkillsInstalledDialog) { - EditorDialogHelper.ShowSkillsInstalledDialog(); + _dialogs.ShowSkillsInstalled(); } } finally @@ -256,10 +260,9 @@ internal async Task HandleInstallAllSkills(CancellationToken ct) if (!_cliSetupApplicationService.IsCliInstalled()) { - EditorUtility.DisplayDialog( + _dialogs.ShowMessage( "CLI Not Found", - "uloop CLI is not installed. Please install the CLI first.", - "OK"); + "uloop CLI is not installed. Please install the CLI first."); return; } @@ -271,7 +274,7 @@ internal async Task HandleInstallAllSkills(CancellationToken ct) try { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); - List targets = await Task.Run( + List targets = await _backgroundWorkRunner.RunAsync( () => _skillSetupUseCase.DetectSkillTargetsForLayoutAtProjectRoot( projectRoot, !_installSkillsFlat)); @@ -296,7 +299,7 @@ await _skillSetupUseCase.InstallSkillFilesAsync( ct); if (shouldShowSkillsInstalledDialog) { - EditorDialogHelper.ShowSkillsInstalledDialog(); + _dialogs.ShowSkillsInstalled(); } } finally @@ -336,7 +339,7 @@ private async Task RefreshSelectedTargetInstallStateAsync(CancellationToken ct) { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); (SkillSetupTargetInfo selectedTargetInfo, List allTargets) = - await Task.Run(() => GetSelectedTargetInfo(projectRoot, includeFreshnessCheck: true)); + await _backgroundWorkRunner.RunAsync(() => GetSelectedTargetInfo(projectRoot, includeFreshnessCheck: true)); if (ct.IsCancellationRequested) { return; diff --git a/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs b/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs index 013b443d2a..6db560bc71 100644 --- a/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs +++ b/Packages/src/Editor/ToolContracts/EditorFrameWaiter.cs @@ -14,8 +14,18 @@ internal sealed class EditorFrameWaiterService { private readonly object _lockObject = new object(); private readonly List _requests = new List(); + private readonly Func _waitTimeout; private int _currentFrameCount; + /// + /// Creates a frame waiter whose timeout defaults to wall-clock TimerDelay; tests pass a fake + /// so a timeout can be completed without waiting on real time. + /// + internal EditorFrameWaiterService(Func waitTimeout = null) + { + _waitTimeout = waitTimeout ?? TimerDelay.Wait; + } + public int PendingWaitCount { get @@ -79,7 +89,7 @@ public async Task WaitFramesOrTimeoutAsync( Task frameTask = WaitFramesCoreAsync(frameCount, frameCancellationSource.Token); using CancellationTokenSource timeoutCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(ct); - Task timeoutTask = TimerDelay.Wait(timeoutMilliseconds, timeoutCancellationSource.Token); + Task timeoutTask = _waitTimeout(timeoutMilliseconds, timeoutCancellationSource.Token); Task completedTask = await Task.WhenAny(frameTask, timeoutTask).ConfigureAwait(false); if (completedTask == timeoutTask) @@ -109,7 +119,7 @@ public void ClearAllForTests() } } - private void UpdateRequests() + internal void UpdateRequests() { List completedRequests = new List(); lock (_lockObject) diff --git a/coverage-baseline.json b/coverage-baseline.json index 8c91a1cbe5..dc2e167c0a 100644 --- a/coverage-baseline.json +++ b/coverage-baseline.json @@ -21,6 +21,6 @@ "UnityCLILoop.FirstPartyTools.ReplayInput.Editor", "UnityCLILoop.FirstPartyTools.Common.Overlay.Editor" ], - "lineCoverage": 85.4 + "lineCoverage": 87.3 } } diff --git a/docs/unity-editmode-test-guardrails.md b/docs/unity-editmode-test-guardrails.md index 6fe87739d7..2abf18a105 100644 --- a/docs/unity-editmode-test-guardrails.md +++ b/docs/unity-editmode-test-guardrails.md @@ -13,6 +13,7 @@ async execution, cancellation, threads, or dynamic-code runtime paths. - Do not block the main thread inside Unity EditMode tests with `.Wait()`, `.Result`, `Task.WaitAll`, `Thread.Sleep`, or similar synchronous waiting APIs. - Do not add Unity EditMode tests that execute real dynamic-code compile-and-run flows through `ExecuteDynamicCodeTool`, `DynamicCodeCompiler`, or similar end-to-end runtime paths when a pure unit test or compile-only test can cover the behavior. - Do not add Unity EditMode tests that start nested test execution flows or any other long-running editor orchestration from inside a test body. +- Unity Test Framework (checked in 1.3.9 and 1.6.0) records an `async Task` test as passed when it ends Canceled. A leaked `OperationCanceledException` therefore makes the test pass silently. When a test awaits a value or a completion, make an unexpected cancellation fail the test: catch `OperationCanceledException` and call `Assert.Fail`, or check that the task's `Status` is `RanToCompletion`. Tests that expect a cancellation keep catching it with `try` / `catch` and then check the resulting state. ## High-risk patterns to avoid by default