Terasology / ModuleTestingEnvironment

3 stars 15 forks source link

feat: load modules from classpath (for dependencies) #41

Closed keturn closed 3 years ago

keturn commented 3 years ago

If engine's ModuleManager doesn't load all modules on the classpath by default, how are MTE tests going to get their dependencies?

Companion to https://github.com/MovingBlocks/Terasology/pull/4543

keturn commented 3 years ago

Something along these lines?

except this doesn't actually work, as all the tests I've tried so far fail in a fiery death of NullPointerExceptions, but they do get past the dependency resolution stage.

keturn commented 3 years ago

current status: lots of things passing in IntelliJ but failing under gradle.

keturn commented 3 years ago

I think I'm starting to see what's happening with the mess I'm in.

The errors I'm running in to are when environment.getModuleProviding comes up empty: https://github.com/MovingBlocks/gestalt/blob/d8b35c056c5663d73497acdda7c01b9e32b0b439/gestalt-module/src/main/java/org/terasology/module/ModuleEnvironment.java#L298-L303

Asked about a type, it looks at that type's location, and then looks through all the environment's modules to see if any of them were registered with that location.

When we only have jars for everything, this is fine.

When we have sources and we're running under IntelliJ, it's also fine, because it puts modules/**/classes/ on the classpath and the modules are registered that way.

Problem comes up when we have sources and we run under gradle. Gradle puts jars on the classpath, but when ModuleManager scans the modules/ path, we end up with the classes directories. Even if we don't load register the jars on the classpath as modules, the being passed to getModuleProviding still come from those jars.

So.

Options:

🐘 I think it's possible do that thing with gradle, but there's no built-in option for it (at least as of gradle 6.8), so it'd be significant work.

🕶 preventing MTE from scanning modules/ is probably what makes the most sense. This is already configurable in PathManager, more or less. I doubt we can get away with setting the list of module paths as empty, because this thing that expects there to always be one https://github.com/MovingBlocks/Terasology/blob/220c52dcb9d5a7b68f9d23bff2f80c85daf94649/engine/src/main/java/org/terasology/engine/paths/PathManager.java#L349 but we can set that to be in MTE's temp directory.

keturn commented 3 years ago

Time for another Terasology Build Tune-Up Tuesday, with more fun facts about Gradle and IntelliJ!

As we covered earlier, gradle puts jars on the runtime classpath, not classes directories.

Except for the current subproject under test. The difference is probably because main and test are source sets within the same project, not a dependency between projects.

This becomes especially relevant when the build has things that are only triggered when building the jar, as is the case here: https://github.com/MovingBlocks/Terasology/blob/220c52dcb9d5a7b68f9d23bff2f80c85daf94649/buildSrc/src/main/kotlin/terasology-module.gradle.kts#L192-L197

That's one reason why your tests might not be finding module resources when invoked by gradle.

As for IntelliJ IDEA:

When IntelliJ IDEA imports a gradle project, it makes an IdeaModule (not to be confused with a Terasology module!) for the project, and sub-modules for the main and test source sets.

The complication for us comes in when we consider resources. IntelliJ IDEA won't run a gradle processResources task by default, but it will at least pick up a few things from the source set.

The assets of a Terasology module are stored in Foo/assets.

We can add Foo/assets as a resource directory to Foo.main. That seems to work, but a file assets/textures/foo.png ends up under the resource path textures/foo.png, not the intended assets/textures/foo.png. Its resource name is relative to the resource directory; the name of the resource directory itself is not included.

If we were using IntelliJ alone, we would solve this by setting the relative output path for assets as a resource root. But IntelliJ doesn't use gradle's output paths, so it doesn't pick up on that from the SourceSet's output properties.

In gradle, we can set Foo/ as the source with an include pattern to match assets (and deltas and overrides) without everything else. But that's too sophisticated for the gradle-IntelliJ translation. When IntelliJ sees that Foo.main is trying to claim Foo/ as a content root, it cries foul, complaining that Foo/ is already taken as a content root by its parent IdeaModule.

Long story short: Avoid build configuration headaches by keeping your resources in a resources directory.

keturn commented 3 years ago

In recent comments on #4543, I suggest giving up the current attempt to move ModuleLoadingStuff over here.

If we do that, does anything in this PR remain relevant?

It looks like there are some debugging nice-to-haves we can extract to a separate PR, and might as well start a fresh one for whatever toggle mechanism #4543 ends up offering us.

"Should we care if classpath modules are also on the module path?" is a question worth remembering, https://github.com/Terasology/ModuleTestingEnvironment/blob/0884c431316e3a3b203dd01617bd0286b9b72d48/src/main/java/org/terasology/moduletestingenvironment/ModuleLoadingStuff.java#L51-L53

but we can probably get away with not answering it until after gestalt-v7.

keturn commented 3 years ago

extracted some things to #42.