From ddcac35417d5ddc3fc4acfcd2179284f05eeff4d Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 5 Jul 2026 19:07:53 +0900 Subject: [PATCH 1/3] Collapse the RunTests tool's redundant intermediate DTO layer The RunTests pipeline previously carried a UnityCliLoopTestExecutionRequest / UnityCliLoopTestExecutionResult pair and an IUnityCliLoopTestExecutionService interface between RunTestsSchema and RunTestsResponse. Every hop was a pure field-for-field copy into an identically shaped record, so the intermediate layer only added indirection without changing behavior. Drop the interface and the two DTOs and thread RunTestsSchema / RunTestsResponse through the pipeline directly. RunTestsResponse is still constructed with every derived field passed explicitly so the constructor's derivation branches stay skipped, matching the pre-collapse wire output byte-for-byte. The UnityCliLoopTestMode, TestFilterType, and RunTestsExecutionStatus enums are the real domain vocabulary shared by the schema and services (for example TestExecutionStateValidationService.Validate takes UnityCliLoopTestMode), so they stay in the same file to preserve the .meta GUID. --- .../Editor/OnionAssemblyDependencyTests.cs | 13 --- Assets/Tests/Editor/RunTestsUseCaseTests.cs | 24 +++--- .../FirstPartyTools/RunTests/RunTestsTool.cs | 41 +-------- .../RunTests/RunTestsUseCase.cs | 86 ++++++++++--------- .../UnityCliLoopTestExecutionTypes.cs | 43 +--------- 5 files changed, 61 insertions(+), 146 deletions(-) diff --git a/Assets/Tests/Editor/OnionAssemblyDependencyTests.cs b/Assets/Tests/Editor/OnionAssemblyDependencyTests.cs index 08a80c4a22..f83291f31f 100644 --- a/Assets/Tests/Editor/OnionAssemblyDependencyTests.cs +++ b/Assets/Tests/Editor/OnionAssemblyDependencyTests.cs @@ -337,19 +337,6 @@ public void GetHierarchyUseCase_WhenLoaded_CompilesUnderApplicationAssembly() Assert.That(useCaseAssemblyName, Does.StartWith(FirstPartyToolsAssemblyNamePrefix)); } - [Test] - public void TestExecutionTypes_WhenLoaded_CompileUnderFirstPartyToolsAssembly() - { - // Tests that bundled test-runner implementation types stay inside the first-party tool assembly. - string serviceAssemblyName = typeof(IUnityCliLoopTestExecutionService).Assembly.GetName().Name; - string requestAssemblyName = typeof(UnityCliLoopTestExecutionRequest).Assembly.GetName().Name; - string resultAssemblyName = typeof(UnityCliLoopTestExecutionResult).Assembly.GetName().Name; - - Assert.That(serviceAssemblyName, Does.StartWith(FirstPartyToolsAssemblyNamePrefix)); - Assert.That(requestAssemblyName, Does.StartWith(FirstPartyToolsAssemblyNamePrefix)); - Assert.That(resultAssemblyName, Does.StartWith(FirstPartyToolsAssemblyNamePrefix)); - } - [Test] public void RunTestsUseCase_WhenLoaded_CompilesUnderApplicationAssembly() { diff --git a/Assets/Tests/Editor/RunTestsUseCaseTests.cs b/Assets/Tests/Editor/RunTestsUseCaseTests.cs index 4f584bbf45..1e530dd451 100644 --- a/Assets/Tests/Editor/RunTestsUseCaseTests.cs +++ b/Assets/Tests/Editor/RunTestsUseCaseTests.cs @@ -25,13 +25,13 @@ public async Task ExecuteAsync_WithInvalidExecutionState_ShouldFailFastWithoutRu validationService, NoCleanupWait ); - UnityCliLoopTestExecutionRequest parameters = new() + RunTestsSchema parameters = new() { TestMode = UnityCliLoopTestMode.EditMode, SaveBeforeRun = true }; - UnityCliLoopTestExecutionResult response = await useCase.ExecuteAsync(parameters, CancellationToken.None); + RunTestsResponse response = await useCase.ExecuteAsync(parameters, CancellationToken.None); Assert.That(response.Success, Is.False); Assert.That(response.Status, Is.EqualTo(RunTestsExecutionStatus.ExecutionFailed)); @@ -60,12 +60,12 @@ public async Task ExecuteAsync_WithUnknownTestMode_ShouldFailFastWithoutRunningT validationService, NoCleanupWait ); - UnityCliLoopTestExecutionRequest parameters = new() + RunTestsSchema parameters = new() { TestMode = (UnityCliLoopTestMode)999 }; - UnityCliLoopTestExecutionResult response = await useCase.ExecuteAsync(parameters, CancellationToken.None); + RunTestsResponse response = await useCase.ExecuteAsync(parameters, CancellationToken.None); Assert.That(response.Success, Is.False); Assert.That(response.Message, Does.Contain("Unsupported test mode")); @@ -85,7 +85,7 @@ public async Task ExecuteAsync_WithDefaultRequest_ShouldSaveBeforeRun() validationService, NoCleanupWait ); - UnityCliLoopTestExecutionRequest parameters = new(); + RunTestsSchema parameters = new(); await useCase.ExecuteAsync(parameters, CancellationToken.None); @@ -109,13 +109,13 @@ public async Task ExecuteAsync_WhenTestFrameworkUnavailable_ShouldFailFastWithou validationService, NoCleanupWait ); - UnityCliLoopTestExecutionRequest parameters = new() + RunTestsSchema parameters = new() { TestMode = UnityCliLoopTestMode.PlayMode, SaveBeforeRun = true }; - UnityCliLoopTestExecutionResult response = await useCase.ExecuteAsync(parameters, CancellationToken.None); + RunTestsResponse response = await useCase.ExecuteAsync(parameters, CancellationToken.None); Assert.That(response.Success, Is.False); Assert.That(response.Status, Is.EqualTo(RunTestsExecutionStatus.ExecutionFailed)); @@ -157,14 +157,14 @@ public async Task ExecuteAsync_WhenNoTestsWereFound_ShouldExposeNoTestsFoundStat validationService, NoCleanupWait ); - UnityCliLoopTestExecutionRequest parameters = new() + RunTestsSchema parameters = new() { TestMode = UnityCliLoopTestMode.PlayMode, FilterType = TestFilterType.exact, FilterValue = "MissingTest" }; - UnityCliLoopTestExecutionResult response = await useCase.ExecuteAsync(parameters, CancellationToken.None); + RunTestsResponse response = await useCase.ExecuteAsync(parameters, CancellationToken.None); Assert.That(response.Success, Is.False); Assert.That(response.Status, Is.EqualTo(RunTestsExecutionStatus.NoTestsFound)); @@ -194,7 +194,7 @@ public async Task ExecuteAsync_AfterTestExecution_ShouldWaitForCleanup() return Task.CompletedTask; } ); - UnityCliLoopTestExecutionRequest parameters = new(); + RunTestsSchema parameters = new(); await useCase.ExecuteAsync(parameters, CancellationToken.None); @@ -221,7 +221,7 @@ public async Task ExecuteAsync_WhenValidationFails_ShouldNotWaitForCleanup() return Task.CompletedTask; } ); - UnityCliLoopTestExecutionRequest parameters = new(); + RunTestsSchema parameters = new(); await useCase.ExecuteAsync(parameters, CancellationToken.None); @@ -250,7 +250,7 @@ public async Task ExecuteAsync_WhenTestFrameworkUnavailable_ShouldNotWaitForClea return Task.CompletedTask; } ); - UnityCliLoopTestExecutionRequest parameters = new(); + RunTestsSchema parameters = new(); await useCase.ExecuteAsync(parameters, CancellationToken.None); diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs index 512a91191a..a7bbd5d99d 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs @@ -16,46 +16,7 @@ public class RunTestsTool : UnityCliLoopTool protected override async Task ExecuteAsync(RunTestsSchema parameters, CancellationToken ct) { RunTestsUseCase useCase = new(); - UnityCliLoopTestExecutionResult result = await useCase.RunTestsAsync(ToRequest(parameters), ct); - return ToResponse(result); - } - - private static UnityCliLoopTestExecutionRequest ToRequest(RunTestsSchema parameters) - { - if (parameters == null) - { - throw new System.ArgumentNullException(nameof(parameters)); - } - - return new UnityCliLoopTestExecutionRequest - { - TestMode = parameters.TestMode, - FilterType = parameters.FilterType, - FilterValue = parameters.FilterValue, - SaveBeforeRun = parameters.SaveBeforeRun, - }; - } - - private static RunTestsResponse ToResponse(UnityCliLoopTestExecutionResult result) - { - if (result == null) - { - throw new System.ArgumentNullException(nameof(result)); - } - - return new RunTestsResponse( - success: result.Success, - message: result.Message, - completedAt: result.CompletedAt, - testCount: result.TestCount, - passedCount: result.PassedCount, - failedCount: result.FailedCount, - skippedCount: result.SkippedCount, - xmlPath: result.XmlPath, - status: result.Status, - hasFailures: result.HasFailures, - noTestsFound: result.NoTestsFound, - noTestsFoundExplanation: result.NoTestsFoundExplanation); + return await useCase.RunTestsAsync(parameters, ct); } } } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs index a2a3743760..eb1700044a 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs @@ -12,7 +12,7 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// Processing sequence: 1. Test filter creation, 2. Test execution, 3. Result processing /// Related classes: RunTestsTool, TestFilterCreationService, TestExecutionService /// - public class RunTestsUseCase : IUnityCliLoopTestExecutionService + public class RunTestsUseCase { private readonly TestFilterCreationService _filterService; private readonly TestExecutionService _executionService; @@ -51,8 +51,10 @@ public RunTestsUseCase( /// Test execution parameters /// Cancellation control token /// Test execution result - public async Task ExecuteAsync(UnityCliLoopTestExecutionRequest parameters, CancellationToken ct) + public async Task ExecuteAsync(RunTestsSchema parameters, CancellationToken ct) { + Debug.Assert(parameters != null, "parameters must not be null"); + if (!IsSupportedTestMode(parameters.TestMode)) { return CreateFailureResponse("Unsupported test mode: " + parameters.TestMode); @@ -76,11 +78,11 @@ public async Task ExecuteAsync(UnityCliLoopTest { filter = _filterService.CreateFilter(parameters.FilterType, parameters.FilterValue); } - + // 2. Test execution ct.ThrowIfCancellationRequested(); SerializableTestResult result; - + try { if (parameters.TestMode == UnityCliLoopTestMode.PlayMode) @@ -102,33 +104,33 @@ public async Task ExecuteAsync(UnityCliLoopTest // Log full exception details for debugging UnityEngine.Debug.LogError($"Test execution failed: {ex}"); VibeLogger.LogError( - "test_execution_failed", - "Test execution encountered an error", + "test_execution_failed", + "Test execution encountered an error", new { testMode = parameters.TestMode, filterType = parameters.FilterType, filterValue = parameters.FilterValue, error = ex.Message } ); - + // Create a minimal error result throw new System.InvalidOperationException("Test execution failed. Please check the logs for details.", ex); } await _waitForTestRunnerCleanupAsync(ct); - - // 3. Response creation - UnityCliLoopTestExecutionResult response = new UnityCliLoopTestExecutionResult - { - Success = result.success, - Status = result.status, - HasFailures = result.hasFailures, - NoTestsFound = result.noTestsFound, - NoTestsFoundExplanation = result.noTestsFoundExplanation, - Message = result.message, - CompletedAt = result.completedAt, - TestCount = result.testCount, - PassedCount = result.passedCount, - FailedCount = result.failedCount, - SkippedCount = result.skippedCount, - XmlPath = result.xmlPath - }; + + // 3. Response creation. + // Why: pass every derived field explicitly so RunTestsResponse's derivation branches stay skipped + // and the wire output matches the pre-collapse pipeline byte-for-byte. + RunTestsResponse response = new( + success: result.success, + message: result.message, + completedAt: result.completedAt, + testCount: result.testCount, + passedCount: result.passedCount, + failedCount: result.failedCount, + skippedCount: result.skippedCount, + xmlPath: result.xmlPath, + status: result.status, + hasFailures: result.hasFailures, + noTestsFound: result.noTestsFound, + noTestsFoundExplanation: result.noTestsFoundExplanation); response.Message = RunTestsNoTestsDiagnosticService.AppendDiagnosticsOrOriginalMessage( response.Message, () => _noTestsDiagnosticService.AppendDiagnosticsIfNeeded( @@ -140,7 +142,7 @@ public async Task ExecuteAsync(UnityCliLoopTest return response; } - public Task RunTestsAsync(UnityCliLoopTestExecutionRequest request, CancellationToken ct) + public Task RunTestsAsync(RunTestsSchema request, CancellationToken ct) { return ExecuteAsync(request, ct); } @@ -150,28 +152,28 @@ private static bool IsSupportedTestMode(UnityCliLoopTestMode testMode) return Enum.IsDefined(typeof(UnityCliLoopTestMode), testMode); } - private static UnityCliLoopTestExecutionResult CreateTestFrameworkUnavailableResponse() + private static RunTestsResponse CreateTestFrameworkUnavailableResponse() { return CreateFailureResponse(RunTestsResponse.TestFrameworkUnavailableMessage); } - private static UnityCliLoopTestExecutionResult CreateFailureResponse(string message) + private static RunTestsResponse CreateFailureResponse(string message) { - return new UnityCliLoopTestExecutionResult - { - Success = false, - Status = RunTestsExecutionStatus.ExecutionFailed, - HasFailures = false, - NoTestsFound = false, - NoTestsFoundExplanation = string.Empty, - Message = message, - CompletedAt = DateTime.UtcNow.ToString("o"), - TestCount = 0, - PassedCount = 0, - FailedCount = 0, - SkippedCount = 0, - XmlPath = null - }; + // Why: supply every optional argument so RunTestsResponse skips its derivation branches + // and returns the same explicit failure shape the intermediate DTO used to build. + return new RunTestsResponse( + success: false, + message: message, + completedAt: DateTime.UtcNow.ToString("o"), + testCount: 0, + passedCount: 0, + failedCount: 0, + skippedCount: 0, + xmlPath: null, + status: RunTestsExecutionStatus.ExecutionFailed, + hasFailures: false, + noTestsFound: false, + noTestsFoundExplanation: string.Empty); } private static async Task WaitForTestRunnerCleanupAsync(CancellationToken ct) diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/UnityCliLoopTestExecutionTypes.cs b/Packages/src/Editor/FirstPartyTools/RunTests/UnityCliLoopTestExecutionTypes.cs index 4a1c700fda..5d739b139c 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/UnityCliLoopTestExecutionTypes.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/UnityCliLoopTestExecutionTypes.cs @@ -1,22 +1,17 @@ -using System.Threading; -using System.Threading.Tasks; - namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// - /// Defines the Unity CLI Loop Test Execution operations required by the owning workflow. + /// Test mode selector shared between the CLI schema and the run-tests pipeline. /// - public interface IUnityCliLoopTestExecutionService - { - Task RunTestsAsync(UnityCliLoopTestExecutionRequest request, CancellationToken ct); - } - public enum UnityCliLoopTestMode { EditMode = 0, PlayMode = 1 } + /// + /// Filter strategy that determines how the run-tests filter value is interpreted. + /// public enum TestFilterType { all = 0, @@ -35,34 +30,4 @@ internal static class RunTestsExecutionStatus public const string NoTestsFound = "NoTestsFound"; public const string ExecutionFailed = "ExecutionFailed"; } - - /// - /// Carries the request data needed for Unity CLI Loop Test Execution behavior. - /// - public sealed class UnityCliLoopTestExecutionRequest - { - public UnityCliLoopTestMode TestMode { get; set; } = UnityCliLoopTestMode.EditMode; - public TestFilterType FilterType { get; set; } = TestFilterType.all; - public string FilterValue { get; set; } = ""; - public bool SaveBeforeRun { get; set; } = true; - } - - /// - /// Carries the result data produced by Unity CLI Loop Test Execution behavior. - /// - public sealed class UnityCliLoopTestExecutionResult - { - public bool Success { get; set; } - public string Status { get; set; } = ""; - public bool HasFailures { get; set; } - public bool NoTestsFound { get; set; } - public string NoTestsFoundExplanation { get; set; } = ""; - public string Message { get; set; } = ""; - public string CompletedAt { get; set; } = ""; - public int TestCount { get; set; } - public int PassedCount { get; set; } - public int FailedCount { get; set; } - public int SkippedCount { get; set; } - public string XmlPath { get; set; } - } } From fb5a5c518d58381bd7c15ecb97a48cc069c25b1a Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 5 Jul 2026 19:10:03 +0900 Subject: [PATCH 2/3] Drop the leftover RunTestsAsync pass-through The wrapper only existed to satisfy the deleted IUnityCliLoopTestExecutionService interface; the tool and every test call ExecuteAsync, so keeping two public entry points invited drift. --- Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs | 2 +- .../src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs index a7bbd5d99d..ac20d40e58 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsTool.cs @@ -16,7 +16,7 @@ public class RunTestsTool : UnityCliLoopTool protected override async Task ExecuteAsync(RunTestsSchema parameters, CancellationToken ct) { RunTestsUseCase useCase = new(); - return await useCase.RunTestsAsync(parameters, ct); + return await useCase.ExecuteAsync(parameters, ct); } } } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs index eb1700044a..0eed563b2e 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs @@ -142,11 +142,6 @@ public async Task ExecuteAsync(RunTestsSchema parameters, Canc return response; } - public Task RunTestsAsync(RunTestsSchema request, CancellationToken ct) - { - return ExecuteAsync(request, ct); - } - private static bool IsSupportedTestMode(UnityCliLoopTestMode testMode) { return Enum.IsDefined(typeof(UnityCliLoopTestMode), testMode); From f93773d98400215a6cddded867ee51411e0cf7db Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 5 Jul 2026 19:18:58 +0900 Subject: [PATCH 3/3] Address review findings on the RunTests DTO collapse Restore the explicit ArgumentNullException guard to match the Compile pipeline's Fail Fast precedent, reuse the existing RunTestsResponse.CreateTestFrameworkUnavailable factory instead of rebuilding the same shape by hand, and reword comments that referenced the deleted intermediate DTO or contradicted the exception-based error path. --- .../RunTests/RunTestsUseCase.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs index 0eed563b2e..18be9adb83 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs @@ -53,7 +53,10 @@ public RunTestsUseCase( /// Test execution result public async Task ExecuteAsync(RunTestsSchema parameters, CancellationToken ct) { - Debug.Assert(parameters != null, "parameters must not be null"); + if (parameters == null) + { + throw new System.ArgumentNullException(nameof(parameters)); + } if (!IsSupportedTestMode(parameters.TestMode)) { @@ -63,7 +66,7 @@ public async Task ExecuteAsync(RunTestsSchema parameters, Canc ct.ThrowIfCancellationRequested(); if (!_executionService.IsTestFrameworkAvailable) { - return CreateTestFrameworkUnavailableResponse(); + return RunTestsResponse.CreateTestFrameworkUnavailable(); } ValidationResult validation = _validationService.Validate(parameters.TestMode, parameters.SaveBeforeRun); @@ -109,15 +112,16 @@ public async Task ExecuteAsync(RunTestsSchema parameters, Canc new { testMode = parameters.TestMode, filterType = parameters.FilterType, filterValue = parameters.FilterValue, error = ex.Message } ); - // Create a minimal error result + // Surface the failure; the tool layer converts it into an error response. throw new System.InvalidOperationException("Test execution failed. Please check the logs for details.", ex); } await _waitForTestRunnerCleanupAsync(ct); // 3. Response creation. - // Why: pass every derived field explicitly so RunTestsResponse's derivation branches stay skipped - // and the wire output matches the pre-collapse pipeline byte-for-byte. + // Why: pass the derived fields explicitly so the constructor does not re-derive them; + // the null-triggered derivation exists only as a source-compat fallback for callers + // that omit the optional arguments. RunTestsResponse response = new( success: result.success, message: result.message, @@ -147,15 +151,10 @@ private static bool IsSupportedTestMode(UnityCliLoopTestMode testMode) return Enum.IsDefined(typeof(UnityCliLoopTestMode), testMode); } - private static RunTestsResponse CreateTestFrameworkUnavailableResponse() - { - return CreateFailureResponse(RunTestsResponse.TestFrameworkUnavailableMessage); - } - private static RunTestsResponse CreateFailureResponse(string message) { - // Why: supply every optional argument so RunTestsResponse skips its derivation branches - // and returns the same explicit failure shape the intermediate DTO used to build. + // Why: supply every optional argument so the constructor does not re-derive + // status or explanation fields from the failure message. return new RunTestsResponse( success: false, message: message,