Yield before creating state messages, to accommodate Unity's async ca…

…ncellation

CHANGELOG.md

@@ -9,6 +9,13 @@ Experimental features are also **not** under semantic versioning.
 
 ## [Unreleased]
 
+### Changed
+- Breaking change: `RunAs(Simulated)Loop` is now an `async` method.
+  This had to be done to support Unity's async cancellation state to propagate to state strings. 
+
+### Fixed
+- Fix instructions that were canceled because of a failure not showing up as canceled on Unity.
+
 ## [4.5.0] - 2024-05-25
 
 ### Added

ResponsibleUnity/Assets/UnityTests/ToYieldInstructionTests.cs

@@ -100,8 +100,9 @@ public IEnumerator ToYieldInstruction_ErrorsAsExpected()
 
 			yield return yieldInstruction;
 
-			// Completes one frame "late", because of Update ordering
-			Assert.AreEqual(this.completedOnFrame + 1, Time.frameCount);
+			// Completes two frames "late", because of Update ordering
+			// and the async cancellation that Unity does
+			Assert.AreEqual(this.completedOnFrame + 2, Time.frameCount);
 
 			object unused;
 			Assert.IsFalse(yieldInstruction.WasCanceled);

ResponsibleUnity/Assets/UnityTests/UnityTestInstructionExecutorTests.cs

@@ -13,41 +13,42 @@ namespace Responsible.UnityTests
 {
 	public class UnityTestInstructionExecutorTests
 	{
-		[Test]
-		public void Errors_AreLogged_WhenLogErrorsIsTrue()
+		[UnityTest]
+		public IEnumerator Errors_AreLogged_WhenLogErrorsIsTrue()
 		{
 			using (var executor = new UnityTestInstructionExecutor(logErrors: true))
 			{
 				var message = "Should be in log";
-				Responsibly
+				yield return Responsibly
 					.Do("Throw exception", () => throw new Exception(message))
-					.ToYieldInstruction(executor, throwOnError: false); // Should complete synchronously
+					.ToYieldInstruction(executor, throwOnError: false);
 				LogAssert.Expect(LogType.Error, new Regex(message));
 			}
 		}
 
-		[Test]
-		public void UnhandledErrorLog_IsLoggedAsWarning_WhenLogErrorsIsTrue()
+		[UnityTest]
+		public IEnumerator UnhandledErrorLog_IsLoggedAsWarning_WhenLogErrorsIsTrue()
 		{
 			using (var executor = new UnityTestInstructionExecutor(logErrors: true))
 			{
 				var expected = "expected message";
-				Responsibly
+				yield return Responsibly
 					.Do("Throw exception", () => Debug.LogError(expected))
-					.ToYieldInstruction(executor, throwOnError: false); // Should complete synchronously
+					.ToYieldInstruction(executor, throwOnError: false);
 
 				LogAssert.Expect(LogType.Warning, new Regex(expected)); // The one from us
 			}
 		}
 
-		[Test]
-		public void Errors_AreNotLogged_WhenLogErrorsIsFalse()
+		[UnityTest]
+		public IEnumerator Errors_AreNotLogged_WhenLogErrorsIsFalse()
 		{
 			using (var executor = new UnityTestInstructionExecutor(logErrors: false))
 			{
 				var instruction = Responsibly
 					.Do("Throw exception", () => throw new Exception())
 					.ToYieldInstruction(executor, throwOnError: false);
+				yield return null;
 				Assert.IsTrue(instruction.CompletedWithError);
 				// Should not fail the test with logged errors
 			}
@@ -64,17 +65,17 @@ public IEnumerator AssertIgnore_DoesNotCauseTestFailure()
 			}
 		}
 
-		[Test]
-		public void GlobalContext_IsIncludedInErrors()
+		[UnityTest]
+		public IEnumerator GlobalContext_IsIncludedInErrors()
 		{
 			var globalContextProvider = Substitute.For<IGlobalContextProvider>();
 			globalContextProvider.BuildGlobalContext(Arg.Do(
 				(StateStringBuilder builder) => builder.AddDetails("Global details")));
 			using (var executor = new UnityTestInstructionExecutor(globalContextProvider: globalContextProvider))
 			{
-				Responsibly
+				yield return Responsibly
 					.Do("Throw exception", () => throw new Exception())
-					.ToYieldInstruction(executor, throwOnError: false); // Should complete synchronously
+					.ToYieldInstruction(executor, throwOnError: false);
 				LogAssert.Expect(LogType.Error, new Regex("Global details"));
 			}
 		}
@@ -128,8 +129,8 @@ public IEnumerator Polling_IsStopped_WhenDisposed()
 			Assert.AreEqual(1, pollCount, "Should not poll after being disposed");
 		}
 
-		[Test]
-		public void LoggingError_CausesFailureSynchronously()
+		[UnityTest]
+		public IEnumerator LoggingError_CausesFailureAtEndOfFrame()
 		{
 			using (var executor = new UnityTestInstructionExecutor(logErrors: false))
 			{
@@ -140,6 +141,7 @@ public void LoggingError_CausesFailureSynchronously()
 
 				Debug.LogError("Should fail the instruction");
 
+				yield return null;
 				Assert.IsTrue(yieldInstruction.CompletedWithError);
 			}
 		}

ResponsibleUnity/Assets/UnityTests/WaitForCoroutineTests.cs

@@ -75,26 +75,28 @@ public IEnumerator WaitForCoroutine_CancelsCoroutine_WhenOperationCanceled()
 			}
 		}
 
-		[Test]
-		public void WaitForCoroutine_ContainsCorrectDescription()
+		[UnityTest]
+		public IEnumerator WaitForCoroutine_ContainsCorrectDescription()
 		{
 			var instruction = Responsibly
 				.WaitForCoroutine("Manual", this.ThrowImmediately)
 				.ExpectWithinSeconds(1)
 				.ToYieldInstruction(this.Executor);
 
+			yield return null;
 			Assert.IsTrue(instruction.CompletedWithError);
 			StringAssert.Contains("Manual (Coroutine)", instruction.Error.Message);
 		}
 
-		[Test]
-		public void WaitForCoroutineMethod_ContainsCorrectDescription()
+		[UnityTest]
+		public IEnumerator WaitForCoroutineMethod_ContainsCorrectDescription()
 		{
 			var instruction = Responsibly
 				.WaitForCoroutineMethod(this.ThrowImmediately)
 				.ExpectWithinSeconds(1)
 				.ToYieldInstruction(this.Executor);
 
+			yield return null;
 			Assert.IsTrue(instruction.CompletedWithError);
 			StringAssert.Contains("ThrowImmediately (Coroutine)", instruction.Error.Message);
 		}
@@ -116,15 +118,16 @@ public IEnumerator CompoundCoroutineWait_ContainsCorrectDescription()
 				Does.Match(@"\[✓\] CompleteAfterOneFrame.*\n.*\[!\] ThrowImmediately"));
 		}
 
-		[Test]
-		public void WaitForCoroutine_ThrowsWithInvalidExecutor()
+		[UnityTest]
+		public IEnumerator WaitForCoroutine_ThrowsWithInvalidExecutor()
 		{
 			var nonUnityExecutor = new TestInstructionExecutor(new MockTestScheduler());
 			var instruction = Responsibly
 				.WaitForCoroutineMethod(this.Forever)
 				.ExpectWithinSeconds(1)
 				.ToYieldInstruction(nonUnityExecutor);
 
+			yield return null;
 			Assert.IsTrue(instruction.CompletedWithError);
 			StringAssert.Contains(nameof(MonoBehaviour), instruction.Error.Message);
 		}

com.beatwaves.responsible/Runtime/TestInstruction.cs

@@ -239,7 +239,7 @@ public static ITestWaitCondition<TWait> RepeatUntil<TWait, TInstruction>(
 		/// <typeparam name="T">Return type of the instruction to execute.</typeparam>
 		/// <returns>The result of the test instruction, if it completes successfully.</returns>
 		/// <inheritdoc cref="Docs.Inherit.CallerMember{T1,T2,T3,T4}"/>
-		public static T RunAsSimulatedUpdateLoop<T>(
+		public static Task<T> RunAsSimulatedUpdateLoop<T>(
 			this ITestInstruction<T> instruction,
 			double framesPerSecond,
 			Action<TimeSpan> tick,
@@ -269,7 +269,7 @@ public static T RunAsSimulatedUpdateLoop<T>(
 		/// <typeparam name="T">Return type of the instruction to execute.</typeparam>
 		/// <returns>The result of the test instruction, if it completes successfully.</returns>
 		/// <inheritdoc cref="Docs.Inherit.CallerMember{T1,T2,T3}"/>
-		public static T RunAsLoop<T>(
+		public static Task<T> RunAsLoop<T>(
 			this ITestInstruction<T> instruction,
 			Action tick,
 			CancellationToken cancellationToken = default,
@@ -282,7 +282,7 @@ public static T RunAsLoop<T>(
 				new GenericRunLoopScheduler(),
 				new SourceContext(nameof(RunAsLoop), memberName, sourceFilePath, sourceLineNumber));
 
-		private static T RunAsLoop<T, TTickArgument>(
+		private static async Task<T> RunAsLoop<T, TTickArgument>(
 			this ITestInstruction<T> instruction,
 			Action<TTickArgument> tick,
 			CancellationToken cancellationToken,
@@ -292,13 +292,14 @@ private static T RunAsLoop<T, TTickArgument>(
 			using (var executor = new TestInstructionExecutor(scheduler, scheduler.ExternalResultSource))
 			{
 				var task = executor.RunInstruction(instruction.CreateState(), sourceContext, cancellationToken);
-
-				while (!task.IsCompleted)
+				var cts = new CancellationTokenSource();
+				while (!task.IsFaulted && !task.IsCompleted && !cts.Token.IsCancellationRequested)
 				{
-					scheduler.Run(tick);
+					scheduler.Run(tick, cts);
+					await Task.Yield(); // Let the test instruction execution process errors
 				}
 
-				return task.GetAwaiter().GetResult();
+				return await task;
 			}
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructionExecutor.cs

@@ -140,6 +140,8 @@ internal async Task<T> RunInstruction<T>(
 						throw;
 					}
 
+					await Task.Yield();
+
 					var message = e is TimeoutException
 						? this.MakeTimeoutMessage(rootState)
 						: this.MakeErrorMessage(rootState, e);

com.beatwaves.responsible/Runtime/Utilities/RunLoopScheduler.cs

@@ -12,7 +12,7 @@ internal abstract class RunLoopScheduler<TTickArgument> : ITestScheduler
 
 		public IExternalResultSource ExternalResultSource => this.runLoopExceptionSource;
 
-		public void Run(Action<TTickArgument> tick)
+		public void Run(Action<TTickArgument> tick, CancellationTokenSource cts)
 		{
 			try
 			{
@@ -22,6 +22,7 @@ public void Run(Action<TTickArgument> tick)
 			}
 			catch (Exception e)
 			{
+				cts.Cancel();
 				this.runLoopExceptionSource.SetException(e);
 			}
 		}

src/Responsible.Tests/ExecutorFailureTests.cs

@@ -44,7 +44,7 @@ public async Task Executor_PropagatesAndNotifiesFailure_WhenOperationTimesOut()
 			StateAssert
 				.StringContainsInOrder(receivedMessage)
 				.Details("timed out")
-				.JustCanceled("NO")
+				.Canceled("NO")
 				.Completed("YES");
 		}
 
@@ -64,9 +64,9 @@ public async Task Executor_PropagatesAndNotifiesFailure_WhenWaitThrows()
 		}
 
 		[Test]
-		public void Executor_NotifiesWaitContext_WhenWaitTimesOut()
+		public async Task Executor_NotifiesWaitContext_WhenWaitTimesOut()
 		{
-			WaitForCondition(
+			_ = WaitForCondition(
 					"Never",
 					() => false,
 					builder => builder.AddNestedDetails(
@@ -76,6 +76,7 @@ public void Executor_NotifiesWaitContext_WhenWaitTimesOut()
 				.ToTask(this.Executor);
 
 			this.Scheduler.AdvanceFrame(TimeSpan.FromSeconds(2));
+			await Task.Yield(); // Let Unity handle cancellation
 
 			this.FailureListener.Received(1).OperationFailed(
 				Arg.Any<TimeoutException>(),
@@ -85,13 +86,19 @@ public void Executor_NotifiesWaitContext_WhenWaitTimesOut()
 		}
 
 		[Test]
-		public void Executor_RequestsGlobalContext_OnFailure()
+		public async Task Executor_RequestsGlobalContext_OnFailure()
 		{
 			this.GlobalContextProvider.BuildGlobalContext(Arg.Do<StateStringBuilder>(
 				b => b.AddDetails("Global details")));
 
-			Do("Throw", () => throw new Exception())
-				.ToTask(this.Executor);
+			try
+			{
+				await Do("Throw", () => throw new Exception()).ToTask(this.Executor);
+			}
+			catch
+			{
+				// Expected, ignore
+			}			
 
 			this.FailureListener.Received(1).OperationFailed(
 				Arg.Any<Exception>(),
@@ -101,16 +108,17 @@ public void Executor_RequestsGlobalContext_OnFailure()
 		}
 
 		[Test]
-		public void Executor_RequestsGlobalContext_OnTimeout()
+		public async Task Executor_RequestsGlobalContext_OnTimeout()
 		{
 			this.GlobalContextProvider.BuildGlobalContext(Arg.Do<StateStringBuilder>(
 				b => b.AddDetails("Global details")));
 
-			WaitForCondition("Never", () => false)
+			_ = WaitForCondition("Never", () => false)
 				.ExpectWithinSeconds(1)
 				.ToTask(this.Executor);
 
 			this.Scheduler.AdvanceFrame(TimeSpan.FromSeconds(2));
+			await Task.Yield(); // Let Unity handle cancellation
 
 			this.FailureListener.Received(1).OperationFailed(
 				Arg.Any<TimeoutException>(),

src/Responsible.Tests/OperationStateTests.cs

@@ -36,7 +36,7 @@ bool WaitForIt()
 			StateAssert.StringContainsInOrder(exception.Message)
 				.Failed("EXPECT WITHIN")
 				.Completed(description)
-				.JustCanceled(description);
+				.Canceled(description);
 		}
 
 	}

src/Responsible.Tests/ResponderTests.cs

@@ -109,7 +109,7 @@ public async Task BasicResponder_FailureDescription_IsAsExpected()
 			StateAssert.StringContainsInOrder(message)
 				.Failed("Response CONDITION EXPECTED WITHIN")
 				.Details("WAIT FOR")
-				.JustCanceled("Condition")
+				.Canceled("Condition")
 				.Details("THEN RESPOND WITH ...");
 		}
 	}