Make interface members + downstream types internal (#249)

This will allow more flexible non-breaking changes in the future. Made possible by C#8 (and we no longer support older Unity versions).

CHANGELOG.md

@@ -11,7 +11,10 @@ Experimental features are also **not** under semantic versioning.
 
 ### 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. 
+  This had to be done to support Unity's async cancellation state to propagate to state strings.
+- Breaking change: Make interface members that were always only intended for internal use internal.
+  This was not possible before C#8, but now that we no longer support older Unity versions,
+  it can be done, allowing more flexibility to change things without breaking things in the future.
 
 ### Fixed
 - Fix instructions that were canceled because of a failure not showing up as canceled on Unity.

ResponsibleUnity/Assets/EditorTests/FakeOperationState.cs

@@ -18,8 +18,8 @@ public FakeOperationState(Exception exception)
 			this.Exception = exception;
 		}
 
-		public TestOperationStatus Status => throw new NotImplementedException();
-		public void BuildDescription(StateStringBuilder builder) => throw new NotImplementedException();
+		TestOperationStatus ITestOperationState.Status => throw new NotImplementedException();
+		void ITestOperationState.BuildDescription(StateStringBuilder builder) => throw new NotImplementedException();
 
 		public override string ToString() => this.Exception != null
 			? throw this.Exception

com.beatwaves.responsible/Runtime/AssemblyInfo.cs

@@ -0,0 +1,8 @@
+using System.Runtime.CompilerServices;
+
+// Testing the Unity Editor utilities is very tedious without
+// implementing fake operation states.
+// We don't want to test internals in the core tests,
+// but here it's ok, as long as the use is very limited.
+[assembly:InternalsVisibleTo("Responsible.EditorTests")]
+[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]

com.beatwaves.responsible/Runtime/Context/RunContext.cs

@@ -2,9 +2,8 @@ namespace Responsible.Context
 {
 	/// <summary>
 	/// Represents the context in which a test operation is run from.
-	/// Only for internal use.
 	/// </summary>
-	public readonly struct RunContext
+	internal readonly struct RunContext
 	{
 		internal readonly ITestScheduler Scheduler;
 		internal readonly SourceContext SourceContext;

com.beatwaves.responsible/Runtime/IExternalResultSource.cs

@@ -22,6 +22,6 @@ public interface IExternalResultSource
 		/// </param>
 		/// <typeparam name="T">Type of the test instruction being executed.</typeparam>
 		/// <returns>A task which will cause early failure or completion of the executed test instruction.</returns>
-		Task<T> GetExternalResult<T>(CancellationToken cancellationToken);
+		public Task<T> GetExternalResult<T>(CancellationToken cancellationToken);
 	}
 }

com.beatwaves.responsible/Runtime/IFailureListener.cs

@@ -14,6 +14,6 @@ public interface IFailureListener
 		/// <param name="failureMessage">
 		/// Message detailing the failure, including the state of the operation.
 		/// </param>
-		void OperationFailed(Exception exception, string failureMessage);
+		public void OperationFailed(Exception exception, string failureMessage);
 	}
 }

com.beatwaves.responsible/Runtime/IGlobalContextProvider.cs

@@ -11,6 +11,6 @@ public interface IGlobalContextProvider
 		/// Add any details that might be useful as global context to include when test operations fail.
 		/// </summary>
 		/// <param name="contextBuilder">The string builder to use to build the context.</param>
-		void BuildGlobalContext(StateStringBuilder contextBuilder);
+		public void BuildGlobalContext(StateStringBuilder contextBuilder);
 	}
 }

com.beatwaves.responsible/Runtime/ITestOperation.cs

@@ -34,6 +34,6 @@ public interface ITestOperation<out T>
 		/// </remarks>
 		[Pure]
 		[NotNull]
-		ITestOperationState<T> CreateState();
+		public ITestOperationState<T> CreateState();
 	}
 }

com.beatwaves.responsible/Runtime/ITestScheduler.cs

@@ -15,7 +15,7 @@ public interface ITestScheduler
 		/// Used for waiting for frames, and for providing state details for operations.
 		/// </summary>
 		/// <value>Current frame value.</value>
-		int FrameNow { get; }
+		public int FrameNow { get; }
 
 		/// <summary>
 		/// Returns the current time.
@@ -26,7 +26,7 @@ public interface ITestScheduler
 		/// <remarks>
 		/// This exists mostly so that it can be mocked in internal Responsible tests.
 		/// </remarks>
-		DateTimeOffset TimeNow { get; }
+		public DateTimeOffset TimeNow { get; }
 
 		/// <summary>
 		/// Registers a poll callback to be called at least once per frame.
@@ -35,6 +35,6 @@ public interface ITestScheduler
 		/// </summary>
 		/// <param name="action">Action to call at least once on every frame.</param>
 		/// <returns>A disposable instance, which must unregister the callback when disposed.</returns>
-		IDisposable RegisterPollCallback(Action action);
+		public IDisposable RegisterPollCallback(Action action);
 	}
 }

com.beatwaves.responsible/Runtime/State/BoxedOperationState.cs

@@ -26,7 +26,7 @@ protected override async Task<TResult> ExecuteInner(RunContext runContext, Cance
 			return this.boxingSelector(result);
 		}
 
-		public override void BuildDescription(StateStringBuilder builder) => this.state.BuildDescription(builder);
+		protected override void BuildDescription(StateStringBuilder builder) => this.state.BuildDescription(builder);
 	}
 
 	internal class BoxedOperationState<T> : BoxedOperationState<T, object>

com.beatwaves.responsible/Runtime/State/ITestOperationState.cs

@@ -11,16 +11,16 @@ namespace Responsible.State
 	public interface ITestOperationState
 	{
 		/// <summary>
-		/// Current status of the test operation. Intended for internal use only.
+		/// Current status of the test operation.
 		/// </summary>
 		/// <value>The current state of this test operation run.</value>
-		TestOperationStatus Status { get; }
+		internal TestOperationStatus Status { get; }
 
 		/// <summary>
 		/// Adds a detailed description of the operation to the builder.
 		/// </summary>
 		/// <param name="builder">State string builder to append the description of this operation state to.</param>
-		void BuildDescription(StateStringBuilder builder);
+		internal void BuildDescription(StateStringBuilder builder);
 	}
 
 	/// <summary>
@@ -40,8 +40,6 @@ public interface ITestOperationState<out T> : ITestOperationState
 	{
 		/// <summary>
 		/// Starts execution of the the operation this state was created from.
-		///
-		/// Intended for internal use only.
 		/// </summary>
 		/// <param name="runContext">The test operation run context this run is part of.</param>
 		/// <param name="cancellationToken">Cancellation token for canceling the run.</param>
@@ -53,7 +51,7 @@ public interface ITestOperationState<out T> : ITestOperationState
 		/// we have to use this unsafe method. However, it is used only from an extension method,
 		/// which does the correct type inference for us, so overall, things are safe.
 		/// </remarks>
-		Task<TResult> ExecuteUnsafe<TResult>(RunContext runContext, CancellationToken cancellationToken);
+		internal Task<TResult> ExecuteUnsafe<TResult>(RunContext runContext, CancellationToken cancellationToken);
 			// where T : TResult <-- not supported in C#
 	}
 }

com.beatwaves.responsible/Runtime/State/SelectOperationState.cs

@@ -35,7 +35,7 @@ protected override async Task<T2> ExecuteInner(RunContext runContext, Cancellati
 			return await this.selectState.Execute(runContext, cancellationToken);
 		}
 
-		public override void BuildDescription(StateStringBuilder builder) =>
+		protected override void BuildDescription(StateStringBuilder builder) =>
 			builder.AddSelect<T1, T2>(this.first, this.SelectStatus);
 	}
 }

com.beatwaves.responsible/Runtime/State/TestOperationStatus.cs

@@ -12,10 +12,8 @@ namespace Responsible.State
 	/// * Completed
 	/// * Failed
 	/// * Canceled
-	///
-	/// Not intended for public use.
 	/// </summary>
-	public abstract class TestOperationStatus
+	internal abstract class TestOperationStatus
 	{
 		internal abstract string MakeStatusLine(string description);
 

com.beatwaves.responsible/Runtime/State/TestOperationsState.cs

@@ -8,42 +8,44 @@ namespace Responsible.State
 	internal abstract class TestOperationState<T> : ITestOperationState<T>
 	{
 		private readonly SourceContext? sourceContext;
+		private TestOperationStatus status = TestOperationStatus.NotExecuted.Instance;
 
-		public TestOperationStatus Status { get; private set; } = TestOperationStatus.NotExecuted.Instance;
+		TestOperationStatus ITestOperationState.Status => this.status;
 
 		protected TestOperationState(SourceContext? sourceContext)
 		{
 			this.sourceContext = sourceContext;
 		}
 
-		public async Task<TResult> ExecuteUnsafe<TResult>(RunContext runContext, CancellationToken cancellationToken)
+		async Task<TResult> ITestOperationState<T>.ExecuteUnsafe<TResult>(RunContext runContext, CancellationToken cancellationToken)
 		{
 			var nestedRunContext = this.sourceContext != null
 				? runContext.MakeNested(this.sourceContext.Value)
 				: runContext;
-			this.Status = new TestOperationStatus.Waiting(this.Status, runContext.MakeWaitContext());
+			this.status = new TestOperationStatus.Waiting(this.status, runContext.MakeWaitContext());
 
 			try
 			{
 				var result = await this.ExecuteInner(nestedRunContext, cancellationToken);
-				this.Status = new TestOperationStatus.Completed(this.Status);
+				this.status = new TestOperationStatus.Completed(this.status);
 				return (TResult)(object)result; // See the Execute extension method for why this is safe.
 			}
 			catch (OperationCanceledException)
 			{
-				this.Status = new TestOperationStatus.Canceled(this.Status);
+				this.status = new TestOperationStatus.Canceled(this.status);
 				throw;
 			}
 			catch (Exception e)
 			{
-				this.Status = new TestOperationStatus.Failed(this.Status, e, nestedRunContext.SourceContext);
+				this.status = new TestOperationStatus.Failed(this.status, e, nestedRunContext.SourceContext);
 				throw;
 			}
 		}
 
 		protected abstract Task<T> ExecuteInner(RunContext runContext, CancellationToken cancellationToken);
 
-		public abstract void BuildDescription(StateStringBuilder builder);
+		void ITestOperationState.BuildDescription(StateStringBuilder builder) => this.BuildDescription(builder);
+		protected abstract void BuildDescription(StateStringBuilder builder);
 
 		public override string ToString() => StateStringBuilder.MakeState(this);
 	}

com.beatwaves.responsible/Runtime/State/UntilResponderState.cs

@@ -52,7 +52,7 @@ protected override async Task<T> ExecuteInner(RunContext runContext, Cancellatio
 			}
 		}
 
-		public override void BuildDescription(StateStringBuilder builder) =>
+		protected override void BuildDescription(StateStringBuilder builder) =>
 			builder.AddUntilResponder(
 				"RESPOND TO",
 				this.respondTo,

com.beatwaves.responsible/Runtime/TestInstructions/ContinuationTestInstruction.cs

@@ -37,7 +37,7 @@ protected override async Task<T2> ExecuteInner(RunContext runContext, Cancellati
 				return await this.continuation.Execute(firstResult, runContext, cancellationToken);
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddContinuation(this.first, this.continuation.State);
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/ExpectConditionInstruction.cs

@@ -35,7 +35,7 @@ protected override Task<T> ExecuteInner(RunContext runContext, CancellationToken
 					cancellationToken,
 					ct => this.condition.Execute(runContext, ct));
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddExpectWithin(this, this.timeout, this.condition);
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/ExpectTestResponse.cs

@@ -38,7 +38,7 @@ protected override async Task<T> ExecuteInner(RunContext runContext, Cancellatio
 				return await instruction.Execute(runContext, cancellationToken);
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddExpectWithin(this, this.timeout, this.responder);
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/GroupedAsInstruction.cs

@@ -31,7 +31,7 @@ public State(
 			protected override Task<T> ExecuteInner(RunContext runContext, CancellationToken cancellationToken) =>
 				this.state.Execute(runContext, cancellationToken);
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddParentWithChildren(this.description, this, new[] { this.state });
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/SequencedTestInstruction.cs

@@ -38,7 +38,7 @@ protected override async Task<T2> ExecuteInner(
 				return await this.second.Execute(runContext, cancellationToken);
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddContinuation(this.first, new ContinuationState.Available(this.second));
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/SynchronousTestInstruction.cs

@@ -28,7 +28,7 @@ public State(string description, Func<T> action, SourceContext sourceContext)
 			protected override Task<T> ExecuteInner(RunContext runContext, CancellationToken cancellationToken)
 				=> Task.FromResult(this.action());
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddInstruction(this, this.description);
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/WaitForFramesInstruction.cs

@@ -33,7 +33,7 @@ protected override Task<object> ExecuteInner(RunContext runContext, Cancellation
 					cancellationToken);
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddInstruction(this, $"WAIT FOR {this.wholeFramesToWaitFor} FRAME(S)");
 		}
 	}

com.beatwaves.responsible/Runtime/TestInstructions/WaitForInstruction.cs

@@ -35,7 +35,7 @@ protected override Task<object> ExecuteInner(
 					cancellationToken);
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddInstruction(this, $"WAIT FOR {this.waitTime:g}");
 		}
 	}

com.beatwaves.responsible/Runtime/TestResponders/AnyOfResponder.cs

@@ -36,7 +36,7 @@ DeferredTask<ITestOperationState<object>> MakeLauncher(
 				return Task.FromResult(MultipleTaskSource.Make(this.responders.Select(MakeLauncher)));
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddToPreviousLineWithChildren(" ANY OF", this.responders);
 		}
 	}

com.beatwaves.responsible/Runtime/TestResponders/RepeatedlyResponder.cs

@@ -53,7 +53,7 @@ protected override Task<IMultipleTaskSource<ITestOperationState<object>>> Execut
 					}));
 			}
 
-			public override void BuildDescription(StateStringBuilder builder)
+			protected override void BuildDescription(StateStringBuilder builder)
 			{
 				if (!this.states.Any())
 				{

com.beatwaves.responsible/Runtime/TestResponders/SelectResponder.cs

@@ -43,7 +43,7 @@ protected override async Task<ITestOperationState<T2>> ExecuteInner(
 				return this.selectState;
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddSelect<T1, T2>(
 					this.responder,
 					this.selectState?.SelectStatus ?? TestOperationStatus.NotExecuted.Instance);

com.beatwaves.responsible/Runtime/TestResponders/TestResponder.cs

@@ -48,7 +48,7 @@ protected override async Task<ITestOperationState<T>> ExecuteInner(
 				return instruction;
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) => builder.AddResponder(this);
+			protected override void BuildDescription(StateStringBuilder builder) => builder.AddResponder(this);
 		}
 	}
 }

com.beatwaves.responsible/Runtime/TestWaitConditions/AllOfWaitCondition.cs

@@ -38,7 +38,7 @@ protected override async Task<T[]> ExecuteInner(RunContext runContext, Cancellat
 			}
 
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddParentWithChildren(
 					"WAIT FOR ALL OF",
 					this,

com.beatwaves.responsible/Runtime/TestWaitConditions/ContinuedWaitCondition.cs

@@ -40,7 +40,7 @@ protected override async Task<TSecond> ExecuteInner(
 				return await this.continuation.Execute(firstResult, runContext, cancellationToken);
 			}
 
-			public override void BuildDescription(StateStringBuilder builder) =>
+			protected override void BuildDescription(StateStringBuilder builder) =>
 				builder.AddContinuation(this.first, this.continuation.State);
 		}
 	}

com.beatwaves.responsible/Runtime/TestWaitConditions/PollingWaitCondition.cs

@@ -48,7 +48,7 @@ protected override Task<T> ExecuteInner(RunContext runContext, CancellationToken
 					this.condition,
 					cancellationToken);
 
-			public override void BuildDescription(StateStringBuilder builder) => builder.AddWait(this);
+			protected override void BuildDescription(StateStringBuilder builder) => builder.AddWait(this);
 		}
 	}
 }

com.beatwaves.responsible/Runtime/TestWaitConditions/RepeatUntilCondition.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
 using Responsible.Context;
@@ -37,7 +37,7 @@ public State(
 				this.waitState = condition.CreateState();
 			}
 
-			public override void BuildDescription(StateStringBuilder builder)
+			protected override void BuildDescription(StateStringBuilder builder)
 			{
 				builder.AddNestedDetails("UNTIL", this.waitState.BuildDescription);
 				builder.AddNestedDetails("REPEATEDLY EXECUTING", b =>