Get testing working, add tests for blockstate cache rebuilds

common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java

@@ -25,6 +25,11 @@ public abstract class BlockStateBaseMixin implements IBlockState {
         cacheInvalid = true;
     }
 
+    @Override
+    public boolean isCacheInvalid() {
+        return cacheInvalid;
+    }
+
     private BlockBehaviour.BlockStateBase.Cache generateCache(BlockBehaviour.BlockStateBase base) {
         if(cacheInvalid) {
             // Ensure that only one block's cache is built at a time

common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java

@@ -316,19 +316,21 @@ public class ModernFixEarlyConfig {
     public static ModernFixEarlyConfig load(File file) {
         ModernFixEarlyConfig config = new ModernFixEarlyConfig(file);
         Properties props = new Properties();
-        if(file.exists()) {
+        if(!Boolean.getBoolean("modernfix.ignoreConfigForTesting")) {
-            try (FileInputStream fin = new FileInputStream(file)){
+            if(file.exists()) {
-                props.load(fin);
+                try (FileInputStream fin = new FileInputStream(file)){
-            } catch (IOException e) {
+                    props.load(fin);
-                throw new RuntimeException("Could not load config file", e);
+                } catch (IOException e) {
+                    throw new RuntimeException("Could not load config file", e);
+                }
+                config.readProperties(props);
             }
-            config.readProperties(props);
-        }
 
-        try {
+            try {
-            config.save();
+                config.save();
-        } catch (IOException e) {
+            } catch (IOException e) {
-            LOGGER.warn("Could not write configuration file", e);
+                LOGGER.warn("Could not write configuration file", e);
+            }
         }
 
         return config;

common/src/main/java/org/embeddedt/modernfix/duck/IBlockState.java

@@ -3,4 +3,5 @@ package org.embeddedt.modernfix.duck;
 
 public interface IBlockState {
     void clearCache();
+    boolean isCacheInvalid();
 }

fabric/build.gradle

@@ -1,5 +1,6 @@
 plugins {
     id "com.github.johnrengelman.shadow" version "7.1.2"
+    id 'com.adarshr.test-logger' version '3.2.0'
 }
 
 architectury {
@@ -44,8 +45,8 @@ dependencies {
     // Remove the next line if you don't want to depend on the API
     // modApi "me.shedaniel:architectury-fabric:${rootProject.architectury_version}"
 
-    testImplementation(common(project(path: ":common", configuration: "namedElements"))) { transitive false }
+    common(project(path: ":common", configuration: "namedElements")) { transitive false }
-    shadowCommon(project(path: ":common", configuration: "transformProductionFabric")) { transitive false }
+    testImplementation(shadowCommon(project(path: ":common", configuration: "transformProductionFabric"))) { transitive false }
 
     testImplementation(platform("org.junit:junit-bom:${project.junit_version}"))
     testImplementation("org.junit.jupiter:junit-jupiter-api")
@@ -66,6 +67,7 @@ test {
         runDir.mkdir()
     }
     workingDir = runDir
+    systemProperty 'modernfix.ignoreConfigForTesting', 'true'
 
     // inject our custom agent to fix #817
     FileCollection agentFile = configurations.getByName("testAgent")

fabric/src/main/java/org/embeddedt/modernfix/ModernFixPreLaunchFabric.java

@@ -9,6 +9,10 @@ import org.embeddedt.modernfix.util.CommonModUtil;
 public class ModernFixPreLaunchFabric implements PreLaunchEntrypoint {
     @Override
     public void onPreLaunch() {
+        if(ModernFixMixinPlugin.instance == null) {
+            System.err.println("Mixin plugin not loaded yet");
+            return;
+        }
         if(ModernFixMixinPlugin.instance.isOptionEnabled("feature.spark_profile_launch.OnFabric")) {
             CommonModUtil.runWithoutCrash(() -> SparkLaunchProfiler.start("launch"), "Failed to start profiler");
         }

fabric/src/test/java/net/minecraft/world/level/block/state/BlockStateCacheTest.java

@@ -0,0 +1,58 @@
+package net.minecraft.world.level.block.state;
+
+import net.minecraft.core.BlockPos;
+import net.minecraft.world.level.EmptyBlockGetter;
+import net.minecraft.world.level.block.Blocks;
+import org.embeddedt.modernfix.duck.IBlockState;
+import org.embeddedt.modernfix.testing.util.BootstrapMinecraft;
+import org.junit.jupiter.api.*;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@BootstrapMinecraft
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+public class BlockStateCacheTest {
+    @BeforeEach
+    public void rebuildCache() {
+        Blocks.rebuildCache();
+    }
+
+    /**
+     * Initially, the cache should be invalid, and null.
+     */
+    @Test
+    @Order(1)
+    public void testCacheNullInitially() {
+        BlockState stoneBlock = Blocks.STONE.defaultBlockState();
+        assertTrue(((IBlockState)stoneBlock).isCacheInvalid());
+        assertNull(stoneBlock.cache);
+    }
+
+    /**
+     * When an API that needs the cache is called, it should be built and the invalid flag
+     * becomes false.
+     */
+    @Test
+    @Order(2)
+    public void testCacheBuiltByRequest() {
+        BlockState stoneBlock = Blocks.STONE.defaultBlockState();
+        stoneBlock.getCollisionShape(EmptyBlockGetter.INSTANCE, BlockPos.ZERO);
+        assertFalse(((IBlockState)stoneBlock).isCacheInvalid());
+        assertNotNull(stoneBlock.cache);
+    }
+
+    /**
+     * When a second rebuild occurs, the invalid flag should be set to true, but the old cache
+     * is not set to null, in order to prevent NPEs if a second thread is accessing the cache
+     * when this takes place.
+     */
+    @Test
+    @Order(3)
+    public void testCacheInvalidatedByLateRebuild() {
+        BlockState stoneBlock = Blocks.STONE.defaultBlockState();
+        stoneBlock.getCollisionShape(EmptyBlockGetter.INSTANCE, BlockPos.ZERO);
+        Blocks.rebuildCache();
+        assertTrue(((IBlockState)stoneBlock).isCacheInvalid());
+        assertNotNull(stoneBlock.cache);
+    }
+}

fabric/src/test/java/org/embeddedt/modernfix/blocks/BlockStateCacheTest.java

@@ -1,28 +0,0 @@
-/*
-package org.embeddedt.modernfix.blocks;
-
-import net.minecraft.DetectedVersion;
-import net.minecraft.core.Registry;
-import net.minecraft.resources.ResourceLocation;
-import net.minecraft.server.Bootstrap;
-import net.minecraft.world.item.Items;
-import org.junit.Test;
-import org.junit.jupiter.api.BeforeAll;
-
-import static org.junit.jupiter.api.Assertions.*;
-
-public class BlockStateCacheTest {
-    @BeforeAll
-    public static void setup() {
-        DetectedVersion.tryDetectVersion();
-        Bootstrap.bootStrap();
-    }
-
-    @Test
-    public void testItemRegistry() {
-        ResourceLocation location = Registry.ITEM.getKey(Items.DIAMOND);
-        assertEquals(location.toString(), "minecraft:diamond");
-    }
-}
-
- */

fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraft.java

@@ -0,0 +1,14 @@
+package org.embeddedt.modernfix.testing.util;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+@ExtendWith({ BootstrapMinecraftExtension.class })
+public @interface BootstrapMinecraft {
+}

fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java

@@ -0,0 +1,21 @@
+package org.embeddedt.modernfix.testing.util;
+
+import net.minecraft.DetectedVersion;
+import net.minecraft.server.Bootstrap;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.Extension;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class BootstrapMinecraftExtension implements Extension, BeforeAllCallback, AfterAllCallback {
+    @Override
+    public void beforeAll(ExtensionContext context) throws Exception {
+        DetectedVersion.tryDetectVersion();
+        Bootstrap.bootStrap();
+    }
+
+    @Override
+    public void afterAll(ExtensionContext context) throws Exception {
+
+    }
+}