refactor: split public MainLoop methods from Engine-management code #73
Conversation
remove JUnit annotations now that this is no longer a superclass
…asses MainLoop has the methods tests tend to interact with, Engines manages the setup behind the scenes.
| long startGameTime = hostTime.getGameTimeInMs(); | ||
|
|
||
| while (f.get() && !timedOut) { | ||
| Thread.yield(); |
There was a problem hiding this comment.
Would it make sense to add some sort of throttled debug output here that could be used to show the time stats every, say, 10 seconds or so? To help identify when something goes wrong, like a weirdly long timeout, or maybe no progress being made (if there's something to ask for "status")
There was a problem hiding this comment.
We certainly have an ongoing need for something to give us more visibility in to what's going on when a test stalls, but that's not in scope for this PR.
| * or conversely run until a condition is true via {@link #runUntil(Supplier)} <br>{@code runUntil(()-> false);} | ||
| * | ||
| * <h2>Specifying Dependencies</h2> | ||
| * By default the environment will load only the engine itself. FIXME |
There was a problem hiding this comment.
Maybe these two FIXMEs can be moved around a bit so there's actual descriptive text behind them?
There was a problem hiding this comment.
I've pushed a big update to the javadoc.
Remove references to JUnit 4 methods.
| * Sets up a Terasology environment for use with your {@index JUnit} 5 test. | ||
| * <p> | ||
| * Supports Terasology's DI as in usual Systems. You can inject Managers via {@link In} annotation, constructor's or | ||
| * test's parameters. Also you can inject {@link ModuleTestingHelper} itself. | ||
| * Supports Terasology's DI as in usual Systems. You can inject Managers via {@link In} annotation, constructors or | ||
| * test parameters. Also you can inject {@link MainLoop} or {@link Engines} to interact with the environment's engine. | ||
| * <p> | ||
| * Every class annotated with this will create a single {@link ModuleTestingHelper} and use it during execution of | ||
| * Example: | ||
| * <pre><code> | ||
| * import org.junit.jupiter.api.extension.ExtendWith; | ||
| * import org.junit.jupiter.api.Test; | ||
| * import org.terasology.engine.registry.In; | ||
| * | ||
| * @{@link org.junit.jupiter.api.extension.ExtendWith}(MTEExtension.class) | ||
| * @{@link Dependencies}("MyModule") | ||
| * @{@link UseWorldGenerator}("Pathfinding:pathfinder") | ||
| * public class ExampleTest { | ||
| * | ||
| * @In | ||
| * EntityManager entityManager; | ||
| * | ||
| * @In | ||
| * {@link MainLoop} mainLoop; | ||
| * | ||
| * @Test | ||
| * public void testSomething() { | ||
| * // … | ||
| * } | ||
| * | ||
| * // Injection is also applied to the parameters of individual tests: | ||
| * @Test | ||
| * public void testSomething({@link Engines} engines, WorldProvider worldProvider) { | ||
| * // … | ||
| * } | ||
| * } | ||
| * </code></pre> | ||
| * <p> | ||
| * You can configure the environment with these additional annotations: | ||
| * <dl> | ||
| * <dt>{@link Dependencies @Dependencies}</dt> | ||
| * <dd>Specify which modules to include in the environment. Put the name of your module under test here. | ||
| * Any dependencies these modules declare in <code>module.txt</code> will be pulled in as well.</dd> | ||
| * <dt>{@link UseWorldGenerator @UseWorldGenerator}</dt> | ||
| * <dd>The URN of the world generator to use. Defaults to {@link org.terasology.moduletestingenvironment.fixtures.DummyWorldGenerator}, | ||
| * a flat world.</dd> | ||
| * </dl> |
There was a problem hiding this comment.
I really like this! 💚
However, I wonder whether this should be added to the README or module docs - not sure whether in addition to or instead of having it here 🤔
There was a problem hiding this comment.
It's definitely the sort of thing that anyone interested in using MTE will need to know. As for how to organize the docs, I haven't figured that out yet. Neither javadoc nor GitHub-Markdown offer good ways to include source listings like ExampleTest.java.
With the last usage of the deprecated ModuleTestingEnvironment superclass removed from Omega, we're free to remove some of the junit-specific things from ModuleTestingHelper's implementation and look at how to split those dozens of methods up into a few classes with separate responsibilities.
This PR puts
createHostand related methods in a new class namedEngines. It's mostly used for the work thatMTEExtensiondoes behind the scenes, but those tests that usecreateClientto start additional engines may use it directly.runWhileand related methods are in the classMainLoop. These are the methods that test authors will be interested in. ThemakeRelevantmethods are currently here as well.ModuleTestingHelperstill exists for backwards compatibility, but I'd like to push tests to use the smaller MainLoop interface as much as possible.Notes for Reviewers
There are a lot of lines, but I think it's okay to assume that all the methods in
EnginesandMainLoopwere moved out ofModuleTestingEnvironmentas-is.MTEExtensiondoes have some changes to allow the extension to provide the new classes to tests.This should be reviewable/runnable per-commit.
To Do
Docs. There's lots of old stuff specific to JUnit 4 that can go away, but we should be sure that the JUnit 5 replacements for those things have matching documentation.javadoc