Fix GL core tests that depended on test order and JIT timing (#2298)

build.gradle.kts

@@ -175,8 +175,14 @@ val glsmRedirectAgent: Configuration by configurations.creating {
     isCanBeResolved = true
 }
 
+val glsmMutableStaticsAgent: Configuration by configurations.creating {
+    isCanBeConsumed = false
+    isCanBeResolved = true
+}
+
 dependencies {
     glsmRedirectAgent(project(":glsm", "redirectAgent"))
+    glsmMutableStaticsAgent(project(":glsm", "mutableStaticsAgent"))
     archScanAsm(libs.asm.tree.test)
     archScanAsm(libs.asm.commons.test)
 }
@@ -189,6 +195,7 @@ tasks.test {
     useJUnitPlatform { excludeTags = setOf("gl-core") }
     configureAngelicaJava8()
     classpath = archScanAsm + classpath
+    javaAgent(glsmMutableStaticsAgent)
     jvmArgs("-Dceleritas.lwjglService=org.taumc.celeritas.lwjgl.HeadlessTestLWJGLService")
     dependsOn(":glsm:classes", ":lwjgl3-backend:classes", ":sdl-gpu:classes")
 }

glsm/build.gradle.kts

@@ -158,6 +158,7 @@ dependencies {
     testFixturesApi(platform(libs.junit.bom))
     testFixturesApi(libs.junit.jupiter.api)
     testFixturesApi(libs.lwjgl2)
+    testFixturesCompileOnly(libs.asm.tree.test)
 
     // Test
     testImplementation(sourceSets["stubs"].output)
@@ -257,6 +258,13 @@ fun Test.configureGlsmJava8() {
     }
 }
 
+val mutableStaticsAgentJar by tasks.registering(Jar::class) {
+    archiveClassifier = "mutable-statics-agent"
+    manifest {
+        attributes("Premain-Class" to "com.gtnewhorizons.angelica.glsm.testutil.MutableStatics")
+    }
+}
+
 tasks.test {
     useJUnitPlatform {
         excludeTags = GL_TASK_TAGS.values.toSet() + "lwjgl3"
@@ -265,6 +273,7 @@ tasks.test {
         excludeTestsMatching("com.gtnewhorizons.angelica.glsm.shader.SpirvShaderTranslator*Test")
     }
     configureGlsmJava8()
+    javaAgent(files(mutableStaticsAgentJar))
 }
 
 val redirectAgentJar by tasks.registering(Jar::class) {
@@ -279,8 +288,14 @@ val redirectAgent by configurations.creating {
     isCanBeResolved = false
 }
 
+val mutableStaticsAgent by configurations.creating {
+    isCanBeConsumed = true
+    isCanBeResolved = false
+}
+
 artifacts {
     add(redirectAgent.name, redirectAgentJar)
+    add(mutableStaticsAgent.name, mutableStaticsAgentJar)
 }
 
 val glTestTasks = GL_TASK_TAGS.map { (taskName, tag) ->

glsm/src/test/java/com/gtnewhorizons/angelica/glsm/GLSM_SharedDrawableContext_GLTest.java

@@ -202,7 +202,6 @@ public class GLSM_SharedDrawableContext_GLTest {
 
         DrawableContexts.run(sharedDrawable, "SharedDrawableContext-Reentry-Thread", () -> {
             final ShaderManager sm = ShaderManager.getInstance();
-            final boolean wasEnabled = ShaderManager.isEnabled();
             ShaderManager.enable();
             sm.activate();
             try {
@@ -228,7 +227,6 @@ public class GLSM_SharedDrawableContext_GLTest {
                 GLStateManager.glDeleteBuffers(vbo);
             } finally {
                 sm.deactivate();
-                if (!wasEnabled) ShaderManager.disable();
             }
         });
     }
@@ -273,7 +271,6 @@ public class GLSM_SharedDrawableContext_GLTest {
     void indexedListCompiledOnSharedDrawableReplaysOnDisplay() throws Exception {
         final int list = compileIndexedListOnSharedDrawable();
         final ShaderManager sm = ShaderManager.getInstance();
-        final boolean wasEnabled = ShaderManager.isEnabled();
         ShaderManager.enable();
         sm.activate();
         try {
@@ -283,7 +280,6 @@ public class GLSM_SharedDrawableContext_GLTest {
             assertEquals(indexedBatch().getSharedEBO(), GL11.glGetInteger(GL15.GL_ELEMENT_ARRAY_BUFFER_BINDING));
         } finally {
             sm.deactivate();
-            if (!wasEnabled) ShaderManager.disable();
             GLStateManager.glBindVertexArray(0);
         }
     }

glsm/src/test/java/com/gtnewhorizons/angelica/glsm/ffp/FFPVertexLightingGLTest.java

@@ -73,7 +73,6 @@ class FFPVertexLightingGLTest {
         GLStateManager.glDisable(GL11.GL_COLOR_MATERIAL);
         final ShaderManager sm = ShaderManager.getInstance();
         if (sm.isActive()) sm.deactivate();
-        ShaderManager.disable();
         GLStateManager.glBindVertexArray(0);
         if (vbo != 0) {
             GLStateManager.glDeleteBuffers(vbo);

glsm/src/testFixtures/java/com/gtnewhorizons/angelica/glsm/GLSMCoreExtension.java

@@ -16,6 +16,7 @@ import org.lwjgl.opengl.GL32;
 import org.lwjgl.opengl.PixelFormat;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.extension.ExtensionContext.Namespace.GLOBAL;
 
 /**
@@ -82,8 +83,11 @@ public class GLSMCoreExtension implements BeforeAllCallback, BeforeEachCallback,
     public void afterEach(ExtensionContext context) {
         final int error = GL11.glGetError();
         while (GL11.glGetError() != GL11.GL_NO_ERROR) {}
+        final boolean ffpDisabled = !ShaderManager.isEnabled();
+        if (ffpDisabled) ShaderManager.enable();
         GlsmRedirectAgent.assertNoFailures();
         assertEquals(GL11.GL_NO_ERROR, error, () -> "GL Error: 0x" + Integer.toHexString(error));
+        if (ffpDisabled) fail("test left FFP disabled");
     }
 
     @Override

glsm/src/testFixtures/java/com/gtnewhorizons/angelica/glsm/GlsmRedirectAgent.java

@@ -2,6 +2,7 @@ package com.gtnewhorizons.angelica.glsm;
 
 import com.gtnewhorizons.angelica.glsm.loading.ClassDump;
 import com.gtnewhorizons.angelica.glsm.loading.ModRedirector;
+import com.gtnewhorizons.angelica.glsm.testutil.MutableStatics;
 
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
@@ -27,6 +28,7 @@ public final class GlsmRedirectAgent {
     }
 
     public static void install(Instrumentation instrumentation) {
+        MutableStatics.install(instrumentation);
         final ModRedirector redirector = new ModRedirector(ClassDump.DISABLED);
         addTransformer(instrumentation, (className, classfile) -> {
             if (!className.startsWith(GTNHLIB)) return null;

glsm/src/testFixtures/java/com/gtnewhorizons/angelica/glsm/ffp/FfpFixture.java

@@ -237,7 +237,6 @@ public final class FfpFixture {
         GLStateManager.setFfpInstancing(Instancing.NONE);
         final ShaderManager sm = ShaderManager.getInstance();
         if (sm.isActive()) sm.deactivate();
-        ShaderManager.disable();
         GLStateManager.glBindVertexArray(0);
 
         GLStateManager.glActiveTexture(GL13.GL_TEXTURE1);

glsm/src/testFixtures/java/com/gtnewhorizons/angelica/glsm/testutil/MutableStatics.java

@@ -0,0 +1,61 @@
+package com.gtnewhorizons.angelica.glsm.testutil;
+
+import com.gtnewhorizons.angelica.glsm.GlsmRedirectAgent;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.FieldNode;
+
+import java.lang.instrument.Instrumentation;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/* Test agent: strips ACC_FINAL from listed static finals at load to avoid JIT inlining them when tests want to mutate them */
+public final class MutableStatics {
+
+    private static final Map<String, Set<String>> FIELDS_BY_OWNER = new HashMap<>();
+
+    static {
+        add("com/gtnewhorizons/angelica/glsm/backend/BackendManager", "RENDER_BACKEND");
+        add("com/gtnewhorizons/angelica/glsm/GLStateManager", "MainThread");
+        add("com/gtnewhorizons/angelica/config/SystemProperties", "UNMAPPED_GL", "SDL_DISABLE_IN_PASS_CLEAR");
+        add("net/coderbot/iris/Iris", "enabled");
+    }
+
+    private MutableStatics() {}
+
+    private static void add(String owner, String... fields) {
+        FIELDS_BY_OWNER.put(owner, new HashSet<>(Arrays.asList(fields)));
+    }
+
+    public static void premain(String args, Instrumentation instrumentation) {
+        install(instrumentation);
+    }
+
+    public static void install(Instrumentation instrumentation) {
+        GlsmRedirectAgent.addTransformer(instrumentation, (className, classfile) -> {
+            final Set<String> fields = FIELDS_BY_OWNER.get(className);
+            return fields == null ? null : stripFinal(classfile, fields);
+        });
+    }
+
+    private static byte[] stripFinal(byte[] classfile, Set<String> fields) {
+        final ClassNode node = new ClassNode();
+        new ClassReader(classfile).accept(node, 0);
+        int patched = 0;
+        for (FieldNode field : node.fields) {
+            if (fields.contains(field.name)) {
+                field.access &= ~Opcodes.ACC_FINAL;
+                patched++;
+            }
+        }
+        if (patched != fields.size()) throw new IllegalStateException(node.name + " lacks one of " + fields);
+        final ClassWriter writer = new ClassWriter(0);
+        node.accept(writer);
+        return writer.toByteArray();
+    }
+}

glsm/src/testFixtures/java/com/gtnewhorizons/angelica/glsm/testutil/Reflect.java

@@ -1,12 +1,12 @@
 package com.gtnewhorizons.angelica.glsm.testutil;
 
-import com.gtnewhorizon.gtnhlib.reflect.Fields;
-import com.gtnewhorizon.gtnhlib.reflect.Fields.LookupType;
+import com.gtnewhorizons.angelica.glsm.GlsmRedirectAgent;
 import sun.misc.Unsafe;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 
 public final class Reflect {
 
@@ -59,7 +59,13 @@ public final class Reflect {
     }
 
     public static <F> void setStaticFinal(Class<?> owner, String name, Class<F> type, F value) {
-        Fields.ofClass(owner).getField(LookupType.DECLARED_IN_HIERARCHY, name, type).setValue(null, value);
+        final Field f = field(owner, name);
+        if (f.getType() != type) throw new AssertionError(owner.getName() + "." + name + " is " + f.getType().getName() + ", not " + type.getName());
+        if (Modifier.isFinal(f.getModifiers())) {
+            GlsmRedirectAgent.assertNoFailures();
+            throw new AssertionError(owner.getName() + "." + name + " is still final; add it to MutableStatics");
+        }
+        setStatic(owner, name, value);
     }
 
     public static void setDeclared(Class<?> owner, Object target, String name, Object value) {

sdl-gpu/src/test/java/com/gtnewhorizons/angelica/sdlgpu/glsm/GlsmSdlAttachmentClearTest.java

@@ -2,6 +2,7 @@ package com.gtnewhorizons.angelica.sdlgpu.glsm;
 
 import com.gtnewhorizons.angelica.config.SystemProperties;
 import com.gtnewhorizons.angelica.glsm.GLStateManager;
+import com.gtnewhorizons.angelica.glsm.testutil.Reflect;
 import com.gtnewhorizons.angelica.sdlgpu.frame.FrameManager;
 import com.gtnewhorizons.angelica.sdlgpu.resource.ResourceManager;
 import org.junit.jupiter.api.Tag;
@@ -12,11 +13,9 @@ import org.lwjgl.BufferUtils;
 import org.lwjgl.opengl.GL11;
 import org.lwjgl.opengl.GL30;
 
-import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
 import java.nio.FloatBuffer;
 import java.nio.IntBuffer;
-import sun.misc.Unsafe;
 
 import static com.gtnewhorizons.angelica.sdlgpu.glsm.GlsmSdlHeadlessRig.BLACK;
 import static com.gtnewhorizons.angelica.sdlgpu.glsm.GlsmSdlHeadlessRig.RED;
@@ -34,17 +33,8 @@ class GlsmSdlAttachmentClearTest {
         return GlsmSdlHeadlessRig.frameManager().frame();
     }
 
-    // Reflect.setStaticFinal rejects primitive fields.
     private static void setDisableInPassClear(boolean value) {
-        try {
-            final Field theUnsafe = Unsafe.class.getDeclaredField("theUnsafe");
-            theUnsafe.setAccessible(true);
-            final Unsafe unsafe = (Unsafe) theUnsafe.get(null);
-            final Field field = SystemProperties.class.getDeclaredField("SDL_DISABLE_IN_PASS_CLEAR");
-            unsafe.putBoolean(unsafe.staticFieldBase(field), unsafe.staticFieldOffset(field), value);
-        } catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
+        Reflect.setStaticFinal(SystemProperties.class, "SDL_DISABLE_IN_PASS_CLEAR", boolean.class, value);
     }
 
     /** Clears colour to black and depth to 0.9, then draws a quad so a render pass is open. */

src/test/java/com/gtnewhorizons/angelica/rendering/tesr/RetainedTesrGroupsGLTest.java

@@ -151,7 +151,6 @@ class RetainedTesrGroupsGLTest {
         TesrShaderPrograms.invalidateAll();
         final ShaderManager ffp = ShaderManager.getInstance();
         if (ffp.isActive()) ffp.deactivate();
-        ShaderManager.disable();
         if (baseTexture != 0) GLStateManager.glDeleteTextures(baseTexture);
         if (glintTexture != 0) GLStateManager.glDeleteTextures(glintTexture);
         GLStateManager.glActiveTexture(GL13.GL_TEXTURE0);

src/test/java/com/gtnewhorizons/angelica/rendering/tesr/TesrLayerStateTest.java

@@ -93,7 +93,6 @@ class TesrLayerStateTest {
         GLStateManager.glMatrixMode(GL11.GL_MODELVIEW);
         final ShaderManager ffp = ShaderManager.getInstance();
         if (ffp.isActive()) ffp.deactivate();
-        ShaderManager.disable();
 
         GLStateManager.disableBlend();
         GLStateManager.disableAlphaTest();
@@ -228,7 +227,6 @@ class TesrLayerStateTest {
                     () -> {
                         assertEquals(0, GLStateManager.getActiveProgram(), "program during batch draw");
                         assertEquals(0, GLStateManager.getDrawFramebuffer(), "framebuffer during batch draw");
-                        assertEquals(0, GL11.glGetInteger(GL20.GL_CURRENT_PROGRAM), "driver program during batch draw");
                         assertEquals(0, GL11.glGetInteger(GL30.GL_DRAW_FRAMEBUFFER_BINDING), "driver draw framebuffer during batch draw");
                     },
                     () -> {

src/test/java/com/gtnewhorizons/angelica/rendering/tesr/TesrShaderProgramsGLTest.java

@@ -127,7 +127,6 @@ class TesrShaderProgramsGLTest {
         GLStateManager.disableFog();
         final ShaderManager ffp = ShaderManager.getInstance();
         if (ffp.isActive()) ffp.deactivate();
-        ShaderManager.disable();
     }
 
     private TesrShaderPrograms.Entry entry(String vertexBody, String fragmentBody, TesrShaderUniforms uniforms) {

src/test/java/net/coderbot/iris/gl/program/ProgramBindingRegressionTest.java

@@ -138,6 +138,8 @@ class ProgramBindingRegressionTest {
         RenderBackend original = BackendManager.RENDER_BACKEND;
         RenderBackend backend = mock(RenderBackend.class, delegatesTo(original));
         Reflect.setStaticFinal(BackendManager.class, "RENDER_BACKEND", RenderBackend.class, backend);
+        boolean idDeleted = false;
+        boolean otherDeleted = false;
         try {
             for (int i = 0; i < 5; i++) GLStateManager.glCallList(list);
             verify(backend, times(1)).isProgram(id);
@@ -145,26 +147,28 @@ class ProgramBindingRegressionTest {
             assertEquals(id, GL11.glGetInteger(GL20.GL_CURRENT_PROGRAM));
 
             GLStateManager.glDeleteProgram(id);
+            idDeleted = true;
             GLStateManager.glCallList(list); // Still valid while bound, but must not be cached.
             assertEquals(id, GLStateManager.getActiveProgram());
             assertEquals(id, GL11.glGetInteger(GL20.GL_CURRENT_PROGRAM));
             GLStateManager.glUseProgram(other); // Completes deferred deletion.
             GLStateManager.glCallList(list);
             assertEquals(0, GLStateManager.getActiveProgram());
-            assertEquals(0, GL11.glGetInteger(GL20.GL_CURRENT_PROGRAM));
             verify(backend, times(4)).isProgram(id);
             GLStateManager.glCallList(list);
             verify(backend, times(4)).isProgram(id);
 
             GLStateManager.glDeleteProgram(other);
+            otherDeleted = true;
             GLStateManager.glCallList(list);
             verify(backend, times(5)).isProgram(id);
         } finally {
             Reflect.setStaticFinal(BackendManager.class, "RENDER_BACKEND", RenderBackend.class, original);
             GLStateManager.glUseProgram(0);
             GLStateManager.glDeleteLists(list, 1);
-            if (GLStateManager.glIsProgram(id)) GLStateManager.glDeleteProgram(id);
-            if (GLStateManager.glIsProgram(other)) GLStateManager.glDeleteProgram(other);
+            // FFP's glUseProgram(0) is lazy, so other stays current; deleting it again is GL_INVALID_OPERATION on macOS
+            if (!idDeleted) GLStateManager.glDeleteProgram(id);
+            if (!otherDeleted) GLStateManager.glDeleteProgram(other);
         }
     }
 }