From ff39e9022ba5d3d12f971757c4c0f5f643915395 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 6 Jul 2023 19:26:44 -0400 Subject: [PATCH] Get testing working, add tests for blockstate cache rebuilds --- .../BlockStateBaseMixin.java | 5 ++ .../core/config/ModernFixEarlyConfig.java | 24 ++++---- .../embeddedt/modernfix/duck/IBlockState.java | 1 + fabric/build.gradle | 6 +- .../modernfix/ModernFixPreLaunchFabric.java | 4 ++ .../block/state/BlockStateCacheTest.java | 58 +++++++++++++++++++ .../modernfix/blocks/BlockStateCacheTest.java | 28 --------- .../testing/util/BootstrapMinecraft.java | 14 +++++ .../util/BootstrapMinecraftExtension.java | 21 +++++++ 9 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 fabric/src/test/java/net/minecraft/world/level/block/state/BlockStateCacheTest.java delete mode 100644 fabric/src/test/java/org/embeddedt/modernfix/blocks/BlockStateCacheTest.java create mode 100644 fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraft.java create mode 100644 fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java index 576159a3..db024b63 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java +++ b/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 diff --git a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index fa0783ea..ad237a34 100644 --- a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/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()) { - try (FileInputStream fin = new FileInputStream(file)){ - props.load(fin); - } catch (IOException e) { - throw new RuntimeException("Could not load config file", e); + if(!Boolean.getBoolean("modernfix.ignoreConfigForTesting")) { + if(file.exists()) { + try (FileInputStream fin = new FileInputStream(file)){ + props.load(fin); + } catch (IOException e) { + throw new RuntimeException("Could not load config file", e); + } + config.readProperties(props); } - config.readProperties(props); - } - try { - config.save(); - } catch (IOException e) { - LOGGER.warn("Could not write configuration file", e); + try { + config.save(); + } catch (IOException e) { + LOGGER.warn("Could not write configuration file", e); + } } return config; diff --git a/common/src/main/java/org/embeddedt/modernfix/duck/IBlockState.java b/common/src/main/java/org/embeddedt/modernfix/duck/IBlockState.java index d396f128..ea38b50e 100644 --- a/common/src/main/java/org/embeddedt/modernfix/duck/IBlockState.java +++ b/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(); } diff --git a/fabric/build.gradle b/fabric/build.gradle index 1b813a9e..a17fb87f 100644 --- a/fabric/build.gradle +++ b/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 } - shadowCommon(project(path: ":common", configuration: "transformProductionFabric")) { transitive false } + common(project(path: ":common", configuration: "namedElements")) { 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") diff --git a/fabric/src/main/java/org/embeddedt/modernfix/ModernFixPreLaunchFabric.java b/fabric/src/main/java/org/embeddedt/modernfix/ModernFixPreLaunchFabric.java index 8251d6d7..068cad51 100644 --- a/fabric/src/main/java/org/embeddedt/modernfix/ModernFixPreLaunchFabric.java +++ b/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"); } diff --git a/fabric/src/test/java/net/minecraft/world/level/block/state/BlockStateCacheTest.java b/fabric/src/test/java/net/minecraft/world/level/block/state/BlockStateCacheTest.java new file mode 100644 index 00000000..c6884fe2 --- /dev/null +++ b/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); + } +} diff --git a/fabric/src/test/java/org/embeddedt/modernfix/blocks/BlockStateCacheTest.java b/fabric/src/test/java/org/embeddedt/modernfix/blocks/BlockStateCacheTest.java deleted file mode 100644 index f45fb0c7..00000000 --- a/fabric/src/test/java/org/embeddedt/modernfix/blocks/BlockStateCacheTest.java +++ /dev/null @@ -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"); - } -} - - */ \ No newline at end of file diff --git a/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraft.java b/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraft.java new file mode 100644 index 00000000..a1710398 --- /dev/null +++ b/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 { +} diff --git a/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java b/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java new file mode 100644 index 00000000..0971888f --- /dev/null +++ b/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 { + + } +}