Writing Java tests for Fabric

[SquidDev]

While the gametest framework is a fantastic utility for testing Minecraft mods, there's often times when it's desirable to write unit tests more isolated integration tests¹ instead. However, as mixins aren't applied inside a test environment, there's only so much you can do before hitting LinkageError or other fun runtime issues!

Several mods have solved this by injecting Fabric loader as part of the test initialisation process (for example multiconnect and AE2). However, given this is quite an involved process (and is quite heavily tied to Fabric Loader internals), it feels like it would be good to have an official solution instead.

This obviously isn't something Loom can solve in isolation (it would, I assume require some changes to Fabric Loader and/or DLI), but given it's dev-tooling related, this seemed the best place.

---

This is the current solution I'm using. It hooks into the test lifecycle early on using JUnit's automatic extension registration², then installs a Java agent, acquiring an Instrumentation interface, which is then used for bytecode rewriting. To be clear, this isn't good code, but hopefully shows that the underlying principle is viable!

To build something actually usable, I'd probably propose the following changes:

• Add a new launcher class to Fabric Loader which loads mods and installs a ClassFileTransformer into a provided Instrumentation, but does not actually launch the game.

• Define a JUnit extension somewhere (in DLI would sorta make sense, but could be a new project), which installs a Java agent and sets up Fabric Loader.

  The pain point here is that, unlike DLI, the extension shouldn't rely on any system properties or arguments (as those won't be present when running tests from an IDE). Short of hard-coding .gradle/loom-cache/launch.cfg into the extension, I'm not sure of a good way to detect the additional launch properties needed.

• Add some method to Loom (loom.enableTestSupport()) which adds the test extension to the test runtime classpath.

I'm happy to take a look at implementing this, just wanted to check whether this was desirable and/or sensible.

Footnotes

1. Player objects me calling these unit tests. More precisely, I want to write smaller-scale tests which only depend on a small subset of Minecraft's functionality, and so don't require spinning up a server. ↩

2. This does require setting a system property, which is annoying. There's probably other ways to nastily load early on, such as defining a TestEngine, ↩

[modmuss50]

This is definally something that I would like to support as its very useful to have. The main issue with your solution is that it is accessing loader internals. @sfPlayer1 would be able to provide more input on this, but one possible idea would be to include the junit extension in loader, or atleast have an API that could be used by something on the loom side of things.

Loom generates the launch.cfg file, and could possibly also put this (or a path to it) in a jar on the test classpath? There are a lot of solutions to this I think.

Yes, loom should have an option to enable/disable this. Can just be a property in the extension. I see no reason not to enable it by default?

Seems like the main thing missing is a stable API to startup loader and transform classes.

[ChristopherHaws]

Would this include the ability to write unit tests for the client classes when using splitEnvironmentSourceSets? Right now I can only reference items in the main module and not the client module.

[SquidDev]

Oh, that's fairly easy to do already - just add dependencies { testImplementation(sourceSets.client.output) } to your Gradle script. But possibly should be adding that to Loom too.

[ChristopherHaws]

@SquidDev Hmm, I just tried doing this with my code and I am now able to access classes inside the client module, but I cant access client Minecraft classes (for example, my tests use net.minecraft.util.Formatting)

[SquidDev]

Oh, you might also need to add the following:

configurations {
  test.compileClasspath += client.compileClasspath
  test.runtimeClasspath += client.runtimeClasspath
}

Sorry, I've got a load of extra configuration which sits on top of Loom, so forgotten what is/isn't needed!

[ChristopherHaws]

@SquidDev That didn't seem to work, in fact my gradle file wouldn't compile. After some digging (I am new to gradle coming from .NET's MSBuild) I was able to get it to compile using:

sourceSets {
    test {
        compileClasspath += sourceSets.client.compileClasspath
        runtimeClasspath += sourceSets.client.runtimeClasspath
    }
}

Does this look right to you?

[zml2008]

There's been some discussion of this on the JUnit side -- the existing attempts/discussion I've seen linked are:

• https://github.com/fridujo/classpath-junit-extension/blob/master/src/main/java/com/github/fridujo/classpath/junit/extension/jupiter/AbstractClasspathExtension.java
• Introduce extension API for providing a custom ClassLoader (e.g., for Powermock) junit-team/junit5#201
• Introduce extension API for customizing the ClassLoader in Jupiter junit-team/junit5#3028

[SquidDev]

One issue with using Instrumentation that I'd failed to consider, is that it only supports changing existing classes, not defining new ones. This is mostly needed when a mixin contains an inner class (e.g. FooMixin$Inner), as Mixin now needs to define a corresponding inner class (e.g. Foo$Inner) It is possible to do this with more ByteBuddy hacks, but probably means that this (ab)using Instrumentation isn't a good long-term solution.

When Player and I discussed this on IRC, he suggested running Fabric-specific tests in their own classloader. As @zml2008 has mentioned, this isn't really feasible with JUnit right now. The best way (which is what classpath-junit-extension does) is to use a InvocationInterceptor to re-dispatch the test body inside a different classloader. However, the test class is still loaded and instantiated with the default classloader, which means you have to be very careful about not prematurely loading Minecraft classes. I don't know how much of an issue this would be in practice, but it doesn't fill me with enthusiasm either.

[modmuss50]

I had a quick play with the TestNg library instead of junit.

public class ExampleMod implements ModInitializer {
	@Override
	public void onInitialize() {
		TestNG testNG = new TestNG();
		testNG.setTestClasses(new Class[]{TestClass.class});
		testNG.run();
	}

	private class TestClass {
		@Test
		public void example() {
			System.out.println("Example test");
			System.out.println(Blocks.JUKEBOX.getName().toString());
		}
	}
}

The above very simple example works as expected, would need a bit more boilerplate to get the errors out of it and set the exit code. The downside is this still does not provide nice IDE intergration.

[shartte]

I think I solved this for AE2 like this:

(during setup, where Instrumentation is available):

        Path tempDir;
        try {
            tempDir = Files.createTempDirectory("ae2test");
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
        Runtime.getRuntime().addShutdownHook(new Thread(() -> {
            try {
                Files.delete(tempDir);
            } catch (IOException e) {
                e.printStackTrace();
            }
        }));
        var classInjector = ClassInjector.UsingInstrumentation
                .of(tempDir.toFile(), ClassInjector.UsingInstrumentation.Target.SYSTEM, instrumentation);

In the Instrumentation class file transformer, grab the private map of synthetic classes that Mixin maintains:

class AccessWidenerTransformer implements ClassFileTransformer {

    [...]

    private final ClassInjector classInjector;
    private final Map<String, ?> syntheticClasses;
    private final Set<String> injectedClasses = new HashSet<>();

    public AccessWidenerTransformer(AccessWidener accessWidener, IMixinTransformer mixinTransformer, ClassInjector classInjector) {
        this.accessWidener = accessWidener;
        this.mixinTransformer = mixinTransformer;
        this.classInjector = classInjector;
        this.syntheticClasses = getSyntheticClassMap(mixinTransformer);
    }

    /**
     * Gets the private map of synthetic classes from the Mixin transformer.
     * Since we only get callbacks in this class to transform classes that *actually* exist,
     * we need to be proactive in injecting those synthetic classes and need access
     * to the list of such classes that Mixin generated.
     */
    @SuppressWarnings("unchecked")
    private Map<String, ?> getSyntheticClassMap(IMixinTransformer mixinTransformer) {
        final Map<String, ?> syntheticClasses;
        try {
            var syntheticClassRegistryField = mixinTransformer.getClass().getDeclaredField("syntheticClassRegistry");
            syntheticClassRegistryField.setAccessible(true);
            var syntheticClassRegistry = syntheticClassRegistryField.get(mixinTransformer);
            var classesField = syntheticClassRegistry.getClass().getDeclaredField("classes");
            classesField.setAccessible(true);
            syntheticClasses = (Map<String, ?>) classesField.get(syntheticClassRegistry);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
        return syntheticClasses;
    }

And then, everytime after Mixin transforms a class, ensure any new synthetic class gets injected:

    @Override
    public byte[] transform(Module module, ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) {
        var niceName = className.replace('/', '.');
        if (accessWidener.getTargets().contains(niceName)) {
            ClassReader classReader = new ClassReader(classfileBuffer);
            ClassWriter classWriter = new ClassWriter(0);
            ClassVisitor visitor = classWriter;
            visitor = AccessWidenerClassVisitor.createClassVisitor(FabricLoaderImpl.ASM_VERSION, visitor, FabricLoaderImpl.INSTANCE.getAccessWidener());
            classReader.accept(visitor, 0);
            classfileBuffer = classWriter.toByteArray();
        }

        if (className.startsWith("net/minecraft") || className.contains("mixin")) {
            var environment = MixinEnvironment.getCurrentEnvironment();

            var transformResult = mixinTransformer.transformClass(environment, niceName, classfileBuffer);

            // Handle any synthetic anonymous classes that Mixins may have generated
            // We need to inject those into the class-loader using ByteBuddy since
            // the instrumentation API does not support this natively.
            Set<String> classesToInject = Sets.difference(syntheticClasses.keySet(), injectedClasses);
            if (!classesToInject.isEmpty()) {
                var newClasses = new HashMap<String, byte[]>();
                for (var syntheticClass : new HashSet<>(classesToInject)) {
                    injectedClasses.add(syntheticClass);
                    newClasses.put(
                            syntheticClass.replace('/', '.'),
                            mixinTransformer.generateClass(environment, syntheticClass)
                    );
                }
                classInjector.injectRaw(newClasses);
            }

            return transformResult;
        }
        return null;
    }

[SquidDev]

Yep, that's pretty much what I did in CC:T (I use ClassInjector.UsingReflection rather than UsingInstrumentation). It's definitely workable, just feels sufficiently ugly I wouldn't want to build an "official" solution on top of it :p.

[modmuss50]

JUnit recently merged a PR that adds the LauncherInterceptor API. I have had a quick got at getting this to work here: FabricMC/fabric-loader#767 with limited but promising success. My main issue atm is that junit is unable to find any tests and im not quite sure why.

[modmuss50]

This has been merged here: FabricMC/fabric-loader#767

I look forward to seeing what you are able to do with it, if you have any issues or suggestions please post them on the loader issue tracker. 👍

[SquidDev]

Amazing, thank you so much! ❤️ I'm afraid I'm currently stuck on Gradle 7.5.x (yay, multi-loader project), but will report back when I'm able to upgrade :).