I got annoyed with totally useless test failure messages and having to follow the steps to the failures.
A few important changes here:
I've removed StepButton constructors except where required (UntilStepButton, RepeatStepButton) to preserve behaviour. They now use properties including required and { get; init; } keywords to indicate expected uses.
Primary reason I did this is because I found ToggleStepButton missing the isSetupStep param. It feels like the contracts were too loose being optional ctor params.
UntilStepButton now has a callstack similar to AssertButton.
AddStep and AddLabelStep no longer return the step button. It's not used in o!f and osu!, and I'm not sure what practice use it could ever have.
Every method (e.g. AddUntilStep) calls the AddStep(StepButton) method which does the schedule + add to container. In particular, this is important for generating correct callstacks as they're currently generated inside the lambda and lose context. This is why our traces are terrible.
If deemed necessary, I can split this PR up into two. I kinda went down the rabbit hole to see where it ends, so it combines the refactoring.
Demo:
public class SomeTestScene : TestScene
{
[Test]
public void FailingAssertStep()
{
AddAssert("fail", () => false);
}
[Test]
public void PassingAssertStep()
{
AddAssert("pass", () => true);
}
[Test]
public void FailingUntilStep()
{
AddUntilStep("fail", () => false);
}
[Test]
public void PassingUntilStep()
{
AddUntilStep("pass", () => true);
}
[Test]
public void RepeatStep()
{
AddRepeatStep("repeat", () => { }, 10);
}
[Test]
public void WaitStep()
{
AddWaitStep("wait", 10);
}
}
Failed FailingAssertStep [81 ms]
Error Message:
fail
Stack Trace:
at osu.Framework.Threading.ScheduledDelegate.InvokeTask()
at osu.Framework.Threading.ScheduledDelegate.RunTaskInternal()
at osu.Framework.Threading.Scheduler.Update()
at osu.Framework.Graphics.Drawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Graphics.Containers.CompositeDrawable.updateChild(Drawable c)
at osu.Framework.Graphics.Containers.CompositeDrawable.UpdateSubTree()
at osu.Framework.Platform.GameHost.UpdateFrame()
at osu.Framework.Platform.HeadlessGameHost.UpdateFrame()
at osu.Framework.Threading.GameThread.processFrame()
at osu.Framework.Threading.GameThread.RunSingleFrame()
at osu.Framework.Threading.GameThread.<createThread>g__runWork|70_0()
at System.Threading.Thread.StartHelper.Callback(Object state)
---
Failed FailingUntilStep [10 s]
Error Message:
"fail" timed out
Stack Trace:
at osu.Framework.Testing.Drawables.Steps.UntilStepButton.<>c__DisplayClass11_0.<.ctor>b__0() in /Users/smgi/Repos/osu-framework/osu.Framework/Testing/Drawables/Steps/UntilStepButton.cs:line 66
at osu.Framework.Testing.Drawables.Steps.StepButton.PerformStep(Boolean userTriggered) in /Users/smgi/Repos/osu-framework/osu.Framework/Testing/Drawables/Steps/StepButton.cs:line 124
at osu.Framework.Testing.TestScene.runNextStep(Action onCompletion, Action`1 onError, Func`2 stopCondition) in /Users/smgi/Repos/osu-framework/osu.Framework/Testing/TestScene.cs:line 235
--- End of stack trace from previous location ---
at osu.Framework.Testing.TestSceneTestRunner.TestRunner.RunTestBlocking(TestScene test) in /Users/smgi/Repos/osu-framework/osu.Framework/Testing/TestSceneTestRunner.cs:line 89
at osu.Framework.Testing.TestSceneTestRunner.RunTestBlocking(TestScene test) in /Users/smgi/Repos/osu-framework/osu.Framework/Testing/TestSceneTestRunner.cs:line 32
at osu.Framework.Testing.TestScene.UseTestSceneRunnerAttribute.AfterTest(ITest test) in /Users/smgi/Repos/osu-framework/osu.Framework/Testing/TestScene.cs:line 564
at NUnit.Framework.Internal.Commands.TestActionCommand.<>c__DisplayClass0_0.<.ctor>b__1(TestExecutionContext context)
at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__1()
at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)
I got annoyed with totally useless test failure messages and having to follow the steps to the failures.
A few important changes here:
StepButton
constructors except where required (UntilStepButton
,RepeatStepButton
) to preserve behaviour. They now use properties includingrequired
and{ get; init; }
keywords to indicate expected uses.ToggleStepButton
missing theisSetupStep
param. It feels like the contracts were too loose being optional ctor params.UntilStepButton
now has a callstack similar toAssertButton
.AddStep
andAddLabelStep
no longer return the step button. It's not used in o!f and osu!, and I'm not sure what practice use it could ever have.AddUntilStep
) calls theAddStep(StepButton)
method which does the schedule + add to container. In particular, this is important for generating correct callstacks as they're currently generated inside the lambda and lose context. This is why our traces are terrible.If deemed necessary, I can split this PR up into two. I kinda went down the rabbit hole to see where it ends, so it combines the refactoring.
Demo:
Before:
After: