Make sure each TestCase runs all tests under one GameHost #1524
Conversation
Aligns headless nUnit testing to how we expect tests to behave in VisualTests.
| public override void RunTest() | ||
| { | ||
| using (var host = new HeadlessGameHost()) | ||
| host.Run(new FrameworkTestCaseTestRunner(this)); |
smoogipoo
Apr 17, 2018
Contributor
This needs to be reimplemented
This needs to be reimplemented
peppy
Apr 17, 2018
Author
Member
it is
it is
| @@ -42,6 +42,11 @@ public abstract class GameHost : IIpcHost, IDisposable | |||
|
|
|||
| private FrameworkDebugConfigManager debugConfig; | |||
|
|
|||
| internal void Run(object p) | |||
| { | |||
| throw new NotImplementedException(); | |||
smoogipoo
Apr 17, 2018
Contributor
Hm?
Hm?
peppy
Apr 17, 2018
Author
Member
wtf
wtf
| } | ||
| catch | ||
| { | ||
| } | ||
|
|
||
| host.Dispose(); |
smoogipoo
Apr 17, 2018
Contributor
Why is this post-storage deletion?
Why is this post-storage deletion?
| [TearDown] | ||
| public void RunTests() | ||
| { | ||
| var task = runner.Schedule(() => runner.RunTest(this)); |
smoogipoo
Apr 17, 2018
•
Contributor
How about doing all of this (including blocking) internally to RunTest? Removes this cross-object dependency.
How about doing all of this (including blocking) internally to RunTest? Removes this cross-object dependency.
| }, time_between_tests); | ||
| }, e => | ||
|
|
||
| { | ||
| // Other tests may run even if this one failed, so the TestCase still needs to be removed | ||
| Remove(test); |
smoogipoo
Apr 17, 2018
Contributor
This should also invoke onCompletion
This should also invoke onCompletion
| }, time_between_tests); | ||
| }, e => | ||
|
|
smoogipoo
Apr 17, 2018
Contributor
Extra newline
Extra newline
| /// Blocks execution until TestCase runs to completion. | ||
| /// </summary> | ||
| /// <param name="testCase">The TestCase to run.</param> | ||
| public void RunTestFromOtherThread(TestCase testCase) |
smoogipoo
Apr 17, 2018
Contributor
How about just RunTestBlocking instead? RunTestFromOtherThread implies it's running on another thread to me.
Or... What's the argument against just RunTest as it was previously?
How about just RunTestBlocking instead? RunTestFromOtherThread implies it's running on another thread to me.
Or... What's the argument against just RunTest as it was previously?
| @@ -74,7 +82,9 @@ public virtual void RunTest() | |||
| /// This test must run before any other tests, as it relies on <see cref="StepsContainer"/> not being cleared and not having any elements. | |||
| /// </summary> | |||
| [Test, Order(int.MinValue)] | |||
| public void TestConstructor() { mainTest = false; } | |||
| public void TestConstructor() | |||
| { | |||
smoogipoo
Apr 17, 2018
Contributor
Are you sure this is still running tests in the ctor? The reason for this having a flag was because all steps in ctor are added prior to SetUp methods being invoked.
Are you sure this is still running tests in the ctor? The reason for this having a flag was because all steps in ctor are added prior to SetUp methods being invoked.
smoogipoo
Apr 17, 2018
Contributor
I'm sure there's a way through TestContext to get the currently running test, so that should be used instead of a flag, tho.
I'm sure there's a way through TestContext to get the currently running test, so that should be used instead of a flag, tho.
|
I think you'll want to make an osu!-side PR before this is merged in. Some of it might have to change to accomodate for |
|
Yep, that's next thing on my list. |
|
Plz check the last commit and merge whenever. |
|
What's the reasoning for that change? |
|
Because |
|
Aha |
Aligns headless nUnit testing to how we expect tests to behave in VisualTests.