Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fabric-loader-junit to allow unit testing of transformed classes. #767

Merged
merged 18 commits into from
Feb 18, 2023

Conversation

modmuss50
Copy link
Member

@modmuss50 modmuss50 commented Feb 4, 2023

PR to support testing mods with JUnit.

Screenshot 2023-02-04 at 16 34 55

This PR publishes a new fabric-loader-junit maven artifact. This can be setup like so:

dependencies {
	testImplementation "net.fabricmc:fabric-loader-junit:${project.loader_version}"
}

test {
	useJUnitPlatform()
}

Eclipse / Visual Studio code are not currently supported, users of those IDEs must run the tests via Gradle. Gradle 8 MUST also be used to support this, hence why this PR updates loader to use Gradle 8.0-rc2.

Previous discussion: FabricMC/fabric-loom#742
Doucmentation: https://junit.org/junit5/docs/snapshot/user-guide/#launcher-api-launcher-interceptors-custom
Junit PR adding this: junit-team/junit5#3091

@modmuss50 modmuss50 changed the title Start looking at junit support Add fabricloader-junit to allow unit testing of transformed classes. Feb 5, 2023
@modmuss50 modmuss50 marked this pull request as ready for review February 5, 2023 21:30
@modmuss50 modmuss50 changed the title Add fabricloader-junit to allow unit testing of transformed classes. Add fabric-loader-junit to allow unit testing of transformed classes. Feb 5, 2023
@modmuss50
Copy link
Member Author

This should now be ready, Gradle 8 has now been released so im happy to merge this pending no other requested changes 👍

@modmuss50
Copy link
Member Author

Going to merge this, I think for now its fair to call this "experimental" as its using a lot of new Junit and Gradle tech with limitations. As this is an advanced dev only feature I am less worried about having to maintain this far into the future if Junit or anything else changes in the future.

@modmuss50 modmuss50 merged commit 8765403 into FabricMC:master Feb 18, 2023
@md5
Copy link

md5 commented Feb 26, 2023

This is great timing! I was just updating a mod that I wrote against 1.17 and started getting this error in my JUnit tests:

java.lang.IllegalAccessError: class net.minecraft.registry.SimpleRegistry tried to access method 'void net.minecraft.registry.entry.RegistryEntry$Reference.setRegistryKey(net.minecraft.registry.RegistryKey)' (net.minecraft.registry.SimpleRegistry and net.minecraft.registry.entry.RegistryEntry$Reference are in unnamed module of loader 'app')

It turns out that SimpleRegistry and RegistryEntry$Reference are actually in the same package in the official mappings, so package private access to setRegistryKey works there, but in the YARN mappings they are in different packages, resulting in the IllegalAccessError. Since this breaking change appears to have been made in FabricMC/yarn#3396, I made a comment over there.

Adding fabric-loader-junit seems to have worked around that problem, but I started getting a different error now:

org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not execute test class 'io.appropriate.minecraft.mods.durability.DurabilityCheckerTests'.
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:54)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at [email protected]/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at [email protected]/java.lang.reflect.Method.invoke(Method.java:568)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at jdk.proxy1/jdk.proxy1.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: java.lang.RuntimeException: Mixin transformation of io.appropriate.minecraft.mods.durability.DurabilityCheckerTests failed
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.getPostMixinClassByteArray(KnotClassDelegate.java:427)
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.tryLoadClass(KnotClassDelegate.java:323)
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.loadClass(KnotClassDelegate.java:218)
	at net.fabricmc.loader.impl.launch.knot.KnotClassLoader.loadClass(KnotClassLoader.java:112)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:467)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.loadClass(JUnitPlatformTestClassProcessor.java:135)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.access$100(JUnitPlatformTestClassProcessor.java:58)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.execute(JUnitPlatformTestClassProcessor.java:100)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.execute(JUnitPlatformTestClassProcessor.java:90)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:60)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:52)
	... 18 more
Caused by: java.lang.ExceptionInInitializerError
	at net.fabricmc.loader.api.FabricLoader.getInstance(FabricLoader.java:41)
	at net.fabricmc.fabric.impl.client.indigo.IndigoMixinConfigPlugin.loadIfNeeded(IndigoMixinConfigPlugin.java:45)
	at net.fabricmc.fabric.impl.client.indigo.IndigoMixinConfigPlugin.shouldApplyIndigo(IndigoMixinConfigPlugin.java:60)
	at net.fabricmc.fabric.impl.client.indigo.IndigoMixinConfigPlugin.shouldApplyMixin(IndigoMixinConfigPlugin.java:79)
	at org.spongepowered.asm.mixin.transformer.PluginHandle.shouldApplyMixin(PluginHandle.java:132)
	at org.spongepowered.asm.mixin.transformer.MixinInfo.shouldApplyMixin(MixinInfo.java:988)
	at org.spongepowered.asm.mixin.transformer.MixinInfo.readDeclaredTargets(MixinInfo.java:953)
	at org.spongepowered.asm.mixin.transformer.MixinInfo.<init>(MixinInfo.java:882)
	at org.spongepowered.asm.mixin.transformer.MixinConfig.prepareMixins(MixinConfig.java:852)
	at org.spongepowered.asm.mixin.transformer.MixinConfig.prepare(MixinConfig.java:781)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.prepareConfigs(MixinProcessor.java:540)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.select(MixinProcessor.java:462)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.checkSelect(MixinProcessor.java:438)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.applyMixins(MixinProcessor.java:290)
	at org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClass(MixinTransformer.java:234)
	at org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClassBytes(MixinTransformer.java:202)
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.getPostMixinClassByteArray(KnotClassDelegate.java:422)
	... 30 more
Caused by: java.lang.IllegalStateException: trying to load net.fabricmc.loader.impl.FabricLoaderImpl from target class loader
	at net.fabricmc.loader.impl.util.LoaderUtil.verifyNotInTargetCl(LoaderUtil.java:51)
	at net.fabricmc.loader.impl.FabricLoaderImpl.<clinit>(FabricLoaderImpl.java:539)
	... 47 more

I'm hoping that someone more familiar with the internals will have a clearer idea than I do of what the cause may be. My PR with the updates to go from 1.17 to 1.19 is at appropriate/durability-alert-mod#2, but it doesn't seem like the changes I'm making there are causing this since the tests work for the 1.17 version of the mod.

@md5
Copy link

md5 commented Feb 26, 2023

It seems like perhaps FabricLoader.getInstance() needs to be called in launcherSessionOpened before setting the current thread's context ClassLoader or in the constructor of FabricLoaderLauncherSessionListener.

@modmuss50
Copy link
Member Author

Hi, @md5 I think this might be caused by having fabric-loader on the classpath twice. Due to a change made to skip remapping loader its quite easy for a dep to pull in an older version of loader and remap it. I see you have alteast 2 mods that you depend on, a workaround for now is to exclude the fabric-loader transitive dep from them.

@md5
Copy link

md5 commented Feb 26, 2023

Hi, @md5 I think this might be caused by having fabric-loader on the classpath twice. Due to a change made to skip remapping loader its quite easy for a dep to pull in an older version of loader and remap it. I see you have alteast 2 mods that you depend on, a workaround for now is to exclude the fabric-loader transitive dep from them.

Thanks @modmuss50! appropriate/durability-alert-mod@47178fa fixed things up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants